diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 0e386bbf9e..06af059873 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -100,6 +100,20 @@ tasks: platform: ubuntu1804 shell_commands: - ./test/clippy/clippy_failure_test.sh + rustfmt_examples: + name: Rustfmt on Examples + platform: ubuntu2004 + working_directory: examples + build_flags: + - "--aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect" + - "--output_groups=rustfmt_checks" + build_targets: + - //... + rustfmt_failure: + name: Negative Rustfmt Tests + platform: ubuntu2004 + run_targets: + - "//test/rustfmt:test_runner" ubuntu2004_clang: name: Ubuntu 20.04 with Clang platform: ubuntu2004 diff --git a/BUILD.bazel b/BUILD.bazel index feb93a2b9f..72110d5681 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -17,3 +17,10 @@ error_format( build_setting_default = "human", visibility = ["//visibility:public"], ) + +# This setting is used by the rustfmt rules. See https://bazelbuild.github.io/rules_rust/rust_fmt.html +label_flag( + name = "rustfmt.toml", + build_setting_default = "//tools/rustfmt:rustfmt.toml", + visibility = ["//visibility:public"], +) diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel index 64e2dd0e89..acb3adedfb 100644 --- a/docs/BUILD.bazel +++ b/docs/BUILD.bazel @@ -88,6 +88,14 @@ PAGES = dict([ "rust_doc_test", ], ), + page( + name = "rust_fmt", + header_template = ":rust_fmt.vm", + symbols = [ + "rustfmt_aspect", + "rustfmt_test", + ], + ), page( name = "rust_proto", symbols = [ diff --git a/docs/flatten.md b/docs/flatten.md index 6a0bca539f..90075793e2 100644 --- a/docs/flatten.md +++ b/docs/flatten.md @@ -33,6 +33,8 @@ * [rust_wasm_bindgen](#rust_wasm_bindgen) * [rust_wasm_bindgen_repositories](#rust_wasm_bindgen_repositories) * [rust_wasm_bindgen_toolchain](#rust_wasm_bindgen_toolchain) +* [rustfmt_aspect](#rustfmt_aspect) +* [rustfmt_test](#rustfmt_test) @@ -1281,6 +1283,25 @@ For additional information, see the [Bazel toolchains documentation][toolchains] | bindgen | The label of a wasm-bindgen-cli executable. | Label | optional | None | + + +## rustfmt_test + +
+rustfmt_test(name, targets)
+
+ +A test rule for performing `rustfmt --check` on a set of targets + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | +| targets | Rust targets to run rustfmt --check on. | List of labels | optional | [] | + + ## cargo_build_script @@ -1757,3 +1778,40 @@ $ bazel build --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect | name | A unique name for this target. | Name | required | | + + +## rustfmt_aspect + +
+rustfmt_aspect(name)
+
+ +This aspect is used to gather information about a crate for use in rustfmt and perform rustfmt checks + +Output Groups: + +- `rustfmt_manifest`: A manifest used by rustfmt binaries to provide crate specific settings. +- `rustfmt_checks`: Executes `rustfmt --check` on the specified target. + +The build setting `@rules_rust//:rustfmt.toml` is used to control the Rustfmt [configuration settings][cs] +used at runtime. + +[cs]: https://rust-lang.github.io/rustfmt/ + +This aspect is executed on any target which provides the `CrateInfo` provider. However +users may tag a target with `norustfmt` to have it skipped. Additionally, generated +source files are also ignored by this aspect. + + +**ASPECT ATTRIBUTES** + + + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | + + diff --git a/docs/index.md b/docs/index.md index 6658de5b45..ad388c0e5e 100644 --- a/docs/index.md +++ b/docs/index.md @@ -42,6 +42,7 @@ supported in certain environments. - [defs](defs.md): standard rust rules for building and testing libraries and binaries. - [rust_doc](rust_doc.md): rules for generating and testing rust documentation. - [rust_clippy](rust_clippy.md): rules for running [clippy](https://github.com/rust-lang/rust-clippy#readme). +- [rust_fmt](rust_fmt.md): rules for running [rustfmt](https://github.com/rust-lang/rustfmt#readme). - [rust_proto](rust_proto.md): rules for generating [protobuf](https://developers.google.com/protocol-buffers). and [gRPC](https://grpc.io) stubs. - [rust_bindgen](rust_bindgen.md): rules for generating C++ bindings. diff --git a/docs/rust_fmt.md b/docs/rust_fmt.md new file mode 100644 index 0000000000..fed766ca0b --- /dev/null +++ b/docs/rust_fmt.md @@ -0,0 +1,104 @@ + +# Rust Fmt + +* [rustfmt_aspect](#rustfmt_aspect) +* [rustfmt_test](#rustfmt_test) + + +## Overview + + +[Rustfmt][rustfmt] is a tool for formatting Rust code according to style guidelines. +By default, Rustfmt uses a style which conforms to the [Rust style guide][rsg] that +has been formalized through the [style RFC process][rfcp]. A complete list of all +configuration options can be found in the [Rustfmt GitHub Pages][rgp]. + + + +### Setup + + +Formatting your Rust targets' source code requires no setup outside of loading `rules_rust` +in your workspace. Simply run `bazel run @rules_rust//tools/rustfmt` to format source code. + +In addition to this formatter, a check can be added to your build phase using the [rustfmt_aspect](#rustfmt-aspect) +aspect. Simply add the following to a `.bazelrc` file to enable this check. + +```text +build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect +build --output_groups=+rustfmt_checks +``` + +It's recommended to only enable this aspect in your CI environment so formatting issues do not +impact user's ability to rapidly iterate on changes. + +The `rustfmt_aspect` also uses a `--@rules_rust//:rustfmt.toml` setting which determines the +[configuration file][rgp] used by the formatter (`@rules_rust//tools/rustfmt`) and the aspect +(`rustfmt_aspect`). This flag can be added to your `.bazelrc` file to ensure a consistent config +file is used whenever `rustfmt` is run: + +```text +build --@rules_rust//:rustfmt.toml=//:rustfmt.toml +``` + +[rustfmt]: https://github.com/rust-lang/rustfmt#readme +[rsg]: https://github.com/rust-lang-nursery/fmt-rfcs/blob/master/guide/guide.md +[rfcp]: https://github.com/rust-lang-nursery/fmt-rfcs +[rgp]: https://rust-lang.github.io/rustfmt/ + + + +## rustfmt_test + +
+rustfmt_test(name, targets)
+
+ +A test rule for performing `rustfmt --check` on a set of targets + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | +| targets | Rust targets to run rustfmt --check on. | List of labels | optional | [] | + + + + +## rustfmt_aspect + +
+rustfmt_aspect(name)
+
+ +This aspect is used to gather information about a crate for use in rustfmt and perform rustfmt checks + +Output Groups: + +- `rustfmt_manifest`: A manifest used by rustfmt binaries to provide crate specific settings. +- `rustfmt_checks`: Executes `rustfmt --check` on the specified target. + +The build setting `@rules_rust//:rustfmt.toml` is used to control the Rustfmt [configuration settings][cs] +used at runtime. + +[cs]: https://rust-lang.github.io/rustfmt/ + +This aspect is executed on any target which provides the `CrateInfo` provider. However +users may tag a target with `norustfmt` to have it skipped. Additionally, generated +source files are also ignored by this aspect. + + +**ASPECT ATTRIBUTES** + + + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | + + diff --git a/docs/rust_fmt.vm b/docs/rust_fmt.vm new file mode 100644 index 0000000000..a21cdf3219 --- /dev/null +++ b/docs/rust_fmt.vm @@ -0,0 +1,41 @@ +#[[ +## Overview +]]# + +[Rustfmt][rustfmt] is a tool for formatting Rust code according to style guidelines. +By default, Rustfmt uses a style which conforms to the [Rust style guide][rsg] that +has been formalized through the [style RFC process][rfcp]. A complete list of all +configuration options can be found in the [Rustfmt GitHub Pages][rgp]. + + +#[[ +### Setup +]]# + +Formatting your Rust targets' source code requires no setup outside of loading `rules_rust` +in your workspace. Simply run `bazel run @rules_rust//tools/rustfmt` to format source code. + +In addition to this formatter, a check can be added to your build phase using the [rustfmt_aspect](#rustfmt-aspect) +aspect. Simply add the following to a `.bazelrc` file to enable this check. + +```text +build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect +build --output_groups=+rustfmt_checks +``` + +It's recommended to only enable this aspect in your CI environment so formatting issues do not +impact user's ability to rapidly iterate on changes. + +The `rustfmt_aspect` also uses a `--@rules_rust//:rustfmt.toml` setting which determines the +[configuration file][rgp] used by the formatter (`@rules_rust//tools/rustfmt`) and the aspect +(`rustfmt_aspect`). This flag can be added to your `.bazelrc` file to ensure a consistent config +file is used whenever `rustfmt` is run: + +```text +build --@rules_rust//:rustfmt.toml=//:rustfmt.toml +``` + +[rustfmt]: https://github.com/rust-lang/rustfmt#readme +[rsg]: https://github.com/rust-lang-nursery/fmt-rfcs/blob/master/guide/guide.md +[rfcp]: https://github.com/rust-lang-nursery/fmt-rfcs +[rgp]: https://rust-lang.github.io/rustfmt/ diff --git a/docs/symbols.bzl b/docs/symbols.bzl index 67351cc945..fc9a7a2174 100644 --- a/docs/symbols.bzl +++ b/docs/symbols.bzl @@ -50,6 +50,8 @@ load( _rust_static_library = "rust_static_library", _rust_test = "rust_test", _rust_test_suite = "rust_test_suite", + _rustfmt_aspect = "rustfmt_aspect", + _rustfmt_test = "rustfmt_test", ) load( "@rules_rust//rust:repositories.bzl", @@ -113,3 +115,6 @@ rust_analyzer_aspect = _rust_analyzer_aspect crate_universe = _crate_universe crate = _crate + +rustfmt_aspect = _rustfmt_aspect +rustfmt_test = _rustfmt_test diff --git a/rust/defs.bzl b/rust/defs.bzl index e1c2458e94..112086d157 100644 --- a/rust/defs.bzl +++ b/rust/defs.bzl @@ -49,6 +49,11 @@ load( "//rust/private:rustdoc_test.bzl", _rust_doc_test = "rust_doc_test", ) +load( + "//rust/private:rustfmt.bzl", + _rustfmt_aspect = "rustfmt_aspect", + _rustfmt_test = "rustfmt_test", +) rust_library = _rust_library # See @rules_rust//rust/private:rust.bzl for a complete description. @@ -96,7 +101,13 @@ rust_common = _rust_common # See @rules_rust//rust/private:common.bzl for a complete description. rust_analyzer_aspect = _rust_analyzer_aspect -# See @rules_rust//rust:private/rust_analyzer.bzl for a complete description. +# See @rules_rust//rust/private:rust_analyzer.bzl for a complete description. rust_analyzer = _rust_analyzer -# See @rules_rust//rust:private/rust_analyzer.bzl for a complete description. +# See @rules_rust//rust/private:rust_analyzer.bzl for a complete description. + +rustfmt_aspect = _rustfmt_aspect +# See @rules_rust//rust/private:rustfmt.bzl for a complete description. + +rustfmt_test = _rustfmt_test +# See @rules_rust//rust/private:rustfmt.bzl for a complete description. diff --git a/rust/private/rustfmt.bzl b/rust/private/rustfmt.bzl new file mode 100644 index 0000000000..43051e5cc3 --- /dev/null +++ b/rust/private/rustfmt.bzl @@ -0,0 +1,175 @@ +"""A module defining rustfmt rules""" + +load(":common.bzl", "rust_common") +load(":utils.bzl", "find_toolchain") + +def _find_rustfmtable_srcs(target, aspect_ctx = None): + """Parse a target for rustfmt formattable sources. + + Args: + target (Target): The target the aspect is running on. + aspect_ctx (ctx, optional): The aspect's context object. + + Returns: + list: A list of formattable sources (`File`). + """ + if rust_common.crate_info not in target: + return [] + + # Targets annotated with `norustfmt` will not be formatted + if aspect_ctx and "norustfmt" in aspect_ctx.rule.attr.tags: + return [] + + crate_info = target[rust_common.crate_info] + + # Filter out any generated files + srcs = [src for src in crate_info.srcs.to_list() if src.is_source] + + return srcs + +def _generate_manifest(edition, srcs, ctx): + # Gather the source paths to non-generated files + src_paths = [src.path for src in srcs] + + # Write the rustfmt manifest + manifest = ctx.actions.declare_file(ctx.label.name + ".rustfmt") + ctx.actions.write( + output = manifest, + content = "\n".join(src_paths + [ + edition, + ]), + ) + + return manifest + +def _perform_check(edition, srcs, ctx): + toolchain = find_toolchain(ctx) + + marker = ctx.actions.declare_file(ctx.label.name + ".rustfmt.ok") + + args = ctx.actions.args() + args.add("--touch-file") + args.add(marker) + args.add("--") + args.add(toolchain.rustfmt) + args.add("--edition") + args.add(edition) + args.add("--check") + args.add_all(srcs) + + ctx.actions.run( + executable = ctx.executable._process_wrapper, + inputs = srcs, + outputs = [marker], + tools = [toolchain.rustfmt], + arguments = [args], + mnemonic = "Rustfmt", + ) + + return marker + +def _rustfmt_aspect_impl(target, ctx): + srcs = _find_rustfmtable_srcs(target, ctx) + + # If there are no formattable sources, do nothing. + if not srcs: + return [] + + # Parse the edition to use for formatting from the target + edition = target[rust_common.crate_info].edition + + manifest = _generate_manifest(edition, srcs, ctx) + marker = _perform_check(edition, srcs, ctx) + + return [ + OutputGroupInfo( + rustfmt_manifest = depset([manifest]), + rustfmt_checks = depset([marker]), + ), + ] + +rustfmt_aspect = aspect( + implementation = _rustfmt_aspect_impl, + doc = """\ +This aspect is used to gather information about a crate for use in rustfmt and perform rustfmt checks + +Output Groups: + +- `rustfmt_manifest`: A manifest used by rustfmt binaries to provide crate specific settings. +- `rustfmt_checks`: Executes `rustfmt --check` on the specified target. + +The build setting `@rules_rust//:rustfmt.toml` is used to control the Rustfmt [configuration settings][cs] +used at runtime. + +[cs]: https://rust-lang.github.io/rustfmt/ + +This aspect is executed on any target which provides the `CrateInfo` provider. However +users may tag a target with `norustfmt` to have it skipped. Additionally, generated +source files are also ignored by this aspect. +""", + attrs = { + "_process_wrapper": attr.label( + doc = "A process wrapper for running rustfmt on all platforms", + cfg = "exec", + executable = True, + default = Label("//util/process_wrapper"), + ), + }, + incompatible_use_toolchain_transition = True, + fragments = ["cpp"], + host_fragments = ["cpp"], + toolchains = [ + str(Label("//rust:toolchain")), + ], +) + +def _rustfmt_test_impl(ctx): + # The executable of a test target must be the output of an action in + # the rule implementation. This file is simply a symlink to the real + # rustfmt test runner. + runner = ctx.actions.declare_file("{}{}".format( + ctx.label.name, + ctx.executable._runner.extension, + )) + + ctx.actions.symlink( + output = runner, + target_file = ctx.executable._runner, + is_executable = True, + ) + + manifests = [target[OutputGroupInfo].rustfmt_manifest for target in ctx.attr.targets] + srcs = [depset(_find_rustfmtable_srcs(target)) for target in ctx.attr.targets] + + runfiles = ctx.runfiles( + transitive_files = depset(transitive = manifests + srcs), + ) + + runfiles = runfiles.merge( + ctx.attr._runner[DefaultInfo].default_runfiles, + ) + + return [DefaultInfo( + files = depset([runner]), + runfiles = runfiles, + executable = runner, + )] + +rustfmt_test = rule( + implementation = _rustfmt_test_impl, + doc = "A test rule for performing `rustfmt --check` on a set of targets", + attrs = { + "targets": attr.label_list( + doc = "Rust targets to run `rustfmt --check` on.", + providers = [rust_common.crate_info], + aspects = [rustfmt_aspect], + ), + "_runner": attr.label( + doc = "The rustfmt test runner", + cfg = "exec", + executable = True, + default = Label("//tools/rustfmt:rustfmt_test"), + ), + }, + test = True, +) diff --git a/test/rustfmt/BUILD.bazel b/test/rustfmt/BUILD.bazel index 2e4b67bf3e..5ab4876efd 100644 --- a/test/rustfmt/BUILD.bazel +++ b/test/rustfmt/BUILD.bazel @@ -1,17 +1,56 @@ -load("@rules_rust//test/rustfmt:rustfmt_generator.bzl", "rustfmt_generator") - -rustfmt_generator( - name = "formatted", - src = ":unformatted.rs", -) - -sh_test( - name = "rustfmt_test", - size = "small", - srcs = [":rustfmt_test.sh"], - data = [ - ":formatted.rs", - ":unformatted.rs", - ], - deps = ["@bazel_tools//tools/bash/runfiles"], +load("@rules_rust//rust:defs.bzl", "rust_binary", "rustfmt_test") + +exports_files([ + "test_rustfmt.toml", +]) + +rust_binary( + name = "formatted_2018", + srcs = ["srcs/2018/formatted.rs"], + edition = "2018", +) + +rustfmt_test( + name = "test_formatted_2018", + targets = [":formatted_2018"], +) + +rust_binary( + name = "unformatted_2018", + srcs = ["srcs/2018/unformatted.rs"], + edition = "2018", +) + +rustfmt_test( + name = "test_unformatted_2018", + tags = ["manual"], + targets = [":unformatted_2018"], +) + +rust_binary( + name = "formatted_2015", + srcs = ["srcs/2015/formatted.rs"], + edition = "2015", +) + +rustfmt_test( + name = "test_formatted_2015", + targets = [":formatted_2015"], +) + +rust_binary( + name = "unformatted_2015", + srcs = ["srcs/2015/unformatted.rs"], + edition = "2015", +) + +rustfmt_test( + name = "test_unformatted_2015", + tags = ["manual"], + targets = [":unformatted_2015"], +) + +sh_binary( + name = "test_runner", + srcs = ["rustfmt_failure_test.sh"], ) diff --git a/test/rustfmt/rustfmt_failure_test.sh b/test/rustfmt/rustfmt_failure_test.sh new file mode 100755 index 0000000000..0a658e8efb --- /dev/null +++ b/test/rustfmt/rustfmt_failure_test.sh @@ -0,0 +1,89 @@ +#!/bin/bash + +# Runs Bazel build commands over rustfmt rules, where some are expected +# to fail. +# +# Can be run from anywhere within the rules_rust workspace. + +set -euo pipefail + +if [[ -z "${BUILD_WORKSPACE_DIRECTORY:-}" ]]; then + echo "This script should be run under Bazel" + exit 1 +fi + +cd "${BUILD_WORKSPACE_DIRECTORY}" + +# Executes a bazel build command and handles the return value, exiting +# upon seeing an error. +# +# Takes two arguments: +# ${1}: The expected return code. +# ${2}: The target within "//test/rustfmt" to be tested. +function check_build_result() { + local ret=0 + echo -n "Testing ${2}... " + (bazel test //test/rustfmt:"${2}" &> /dev/null) || ret="$?" && true + if [[ "${ret}" -ne "${1}" ]]; then + echo "FAIL: Unexpected return code [saw: ${ret}, want: ${1}] building target //test/rustfmt:${2}" + echo " Run \"bazel test //test/rustfmt:${2}\" to see the output" + exit 1 + else + echo "OK" + fi +} + +function test_all() { + local -r TEST_OK=0 + local -r TEST_FAILED=3 + + check_build_result $TEST_FAILED test_unformatted_2015 + check_build_result $TEST_FAILED test_unformatted_2018 + check_build_result $TEST_OK test_formatted_2015 + check_build_result $TEST_OK test_formatted_2018 +} + +function test_apply() { + local -r TEST_OK=0 + local -r TEST_FAILED=3 + + temp_dir="$(mktemp -d -t ci-XXXXXXXXXX)" + new_workspace="${temp_dir}/rules_rust_test_rustfmt" + + mkdir -p "${new_workspace}/test/rustfmt" && \ + cp -r test/rustfmt/* "${new_workspace}/test/rustfmt/" && \ + cat << EOF > "${new_workspace}/WORKSPACE.bazel" +workspace(name = "rules_rust_test_rustfmt") +local_repository( + name = "rules_rust", + path = "${BUILD_WORKSPACE_DIRECTORY}", +) +load("@rules_rust//rust:repositories.bzl", "rust_repositories") +rust_repositories() +EOF + + pushd "${new_workspace}" + + # Format a specific target + bazel run @rules_rust//tools/rustfmt -- //test/rustfmt:unformatted_2018 + + check_build_result $TEST_FAILED test_unformatted_2015 + check_build_result $TEST_OK test_unformatted_2018 + check_build_result $TEST_OK test_formatted_2015 + check_build_result $TEST_OK test_formatted_2018 + + # Format all targets + bazel run @rules_rust//tools/rustfmt --@rules_rust//:rustfmt.toml=//test/rustfmt:test_rustfmt.toml + + check_build_result $TEST_OK test_unformatted_2015 + check_build_result $TEST_OK test_unformatted_2018 + check_build_result $TEST_OK test_formatted_2015 + check_build_result $TEST_OK test_formatted_2018 + + popd + + rm -rf "${temp_dir}" +} + +test_all +test_apply diff --git a/test/rustfmt/rustfmt_generator.bzl b/test/rustfmt/rustfmt_generator.bzl index 930906ced0..9586fba93e 100644 --- a/test/rustfmt/rustfmt_generator.bzl +++ b/test/rustfmt/rustfmt_generator.bzl @@ -2,6 +2,9 @@ # buildifier: disable=bzl-visibility load("//rust/private:utils.bzl", "find_toolchain") +# buildifier: disable=print +print("WARNING: `rustfmt_generator` is deprecated. Instead, see https://bazelbuild.github.io/rules_rust/rustfmt.html") + def _rustfmt_generator_impl(ctx): toolchain = find_toolchain(ctx) rustfmt_bin = toolchain.rustfmt diff --git a/test/rustfmt/rustfmt_test.sh b/test/rustfmt/rustfmt_test.sh deleted file mode 100755 index a288b30cf7..0000000000 --- a/test/rustfmt/rustfmt_test.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/bash -set -euxo pipefail - -formatted="$(rlocation rules_rust/test/rustfmt/formatted.rs)" -unformatted="$(rlocation rules_rust/test/rustfmt/unformatted.rs)" - -# Ensure that the file was formatted -! diff "$unformatted" "$formatted" diff --git a/test/rustfmt/srcs/2015/formatted.rs b/test/rustfmt/srcs/2015/formatted.rs new file mode 100644 index 0000000000..a2040136c1 --- /dev/null +++ b/test/rustfmt/srcs/2015/formatted.rs @@ -0,0 +1,3 @@ +fn main() { + println!("2015"); +} diff --git a/test/rustfmt/srcs/2015/unformatted.rs b/test/rustfmt/srcs/2015/unformatted.rs new file mode 100644 index 0000000000..d6e473ff20 --- /dev/null +++ b/test/rustfmt/srcs/2015/unformatted.rs @@ -0,0 +1 @@ +fn main(){println!("2015");} diff --git a/test/rustfmt/srcs/2018/formatted.rs b/test/rustfmt/srcs/2018/formatted.rs new file mode 100644 index 0000000000..dff90f24a1 --- /dev/null +++ b/test/rustfmt/srcs/2018/formatted.rs @@ -0,0 +1,40 @@ +use std::future::Future; +use std::sync::Arc; +use std::task::{Context, Poll, Wake}; +use std::thread::{self, Thread}; + +/// A waker that wakes up the current thread when called. +struct ThreadWaker(Thread); + +impl Wake for ThreadWaker { + fn wake(self: Arc) { + self.0.unpark(); + } +} + +/// Run a future to completion on the current thread. +fn block_on(fut: impl Future) -> T { + // Pin the future so it can be polled. + let mut fut = Box::pin(fut); + + // Create a new context to be passed to the future. + let t = thread::current(); + let waker = Arc::new(ThreadWaker(t)).into(); + let mut cx = Context::from_waker(&waker); + + // Run the future to completion. + loop { + match fut.as_mut().poll(&mut cx) { + Poll::Ready(res) => return res, + Poll::Pending => thread::park(), + } + } +} + +async fn edition() -> i32 { + 2018 +} + +fn main() { + println!("{}", block_on(edition())); +} diff --git a/test/rustfmt/srcs/2018/unformatted.rs b/test/rustfmt/srcs/2018/unformatted.rs new file mode 100644 index 0000000000..454ac49255 --- /dev/null +++ b/test/rustfmt/srcs/2018/unformatted.rs @@ -0,0 +1,19 @@ +use std::future::Future; use std::sync::Arc; use std::task::{Context, Poll, Wake}; use std::thread::{self, Thread}; +/// A waker that wakes up the current thread when called. +struct ThreadWaker(Thread); +impl Wake for ThreadWaker {fn wake(self: Arc) {self.0.unpark();}} +/// Run a future to completion on the current thread. +fn block_on(fut: impl Future) -> T { +// Pin the future so it can be polled. +let mut fut = Box::pin(fut); +// Create a new context to be passed to the future. +let t = thread::current();let waker = Arc::new(ThreadWaker(t)).into(); +let mut cx = Context::from_waker(&waker); +// Run the future to completion. +loop {match fut.as_mut().poll(&mut cx) { +Poll::Ready(res) => return res, Poll::Pending => thread::park(), +} +} +} +async fn edition() -> i32 {2018} +fn main(){println!("{}", block_on(edition()));} diff --git a/test/rustfmt/test_rustfmt.toml b/test/rustfmt/test_rustfmt.toml new file mode 100644 index 0000000000..ac5d99f199 --- /dev/null +++ b/test/rustfmt/test_rustfmt.toml @@ -0,0 +1 @@ +control_brace_style = "AlwaysNextLine" diff --git a/test/rustfmt/unformatted.rs b/test/rustfmt/unformatted.rs deleted file mode 100644 index f36f291bf6..0000000000 --- a/test/rustfmt/unformatted.rs +++ /dev/null @@ -1 +0,0 @@ -fn example(){println!("test");} diff --git a/test/unit/crate_name/crate_name_test.bzl b/test/unit/crate_name/crate_name_test.bzl index 20708f8711..d8efb94e0d 100644 --- a/test/unit/crate_name/crate_name_test.bzl +++ b/test/unit/crate_name/crate_name_test.bzl @@ -126,14 +126,14 @@ def _crate_name_test(): rust_library( name = "invalid/default-crate-name", srcs = ["lib.rs"], - tags = ["manual"], + tags = ["manual", "norustfmt"], ) rust_library( name = "invalid-custom-crate-name", crate_name = "hyphens-not-allowed", srcs = ["lib.rs"], - tags = ["manual"], + tags = ["manual", "norustfmt"], ) default_crate_name_library_test( diff --git a/tools/runfiles/runfiles.rs b/tools/runfiles/runfiles.rs index f30088fdc3..8f3bf5d92b 100644 --- a/tools/runfiles/runfiles.rs +++ b/tools/runfiles/runfiles.rs @@ -66,7 +66,7 @@ impl Runfiles { } /// Returns the .runfiles directory for the currently executing binary. -fn find_runfiles_dir() -> io::Result { +pub fn find_runfiles_dir() -> io::Result { let exec_path = std::env::args().nth(0).expect("arg 0 was not set"); let mut binary_path = PathBuf::from(&exec_path); diff --git a/tools/rustfmt/BUILD.bazel b/tools/rustfmt/BUILD.bazel new file mode 100644 index 0000000000..2394456d11 --- /dev/null +++ b/tools/rustfmt/BUILD.bazel @@ -0,0 +1,60 @@ +load("//rust:defs.bzl", "rust_binary", "rust_library") + +package(default_visibility = ["//visibility:public"]) + +exports_files(["rustfmt.toml"]) + +alias( + name = "rustfmt_bin", + actual = select({ + "@rules_rust//rust/platform:aarch64-apple-darwin": "@rust_darwin_aarch64//:rustfmt_bin", + "@rules_rust//rust/platform:aarch64-unknown-linux-gnu": "@rust_linux_aarch64//:rustfmt_bin", + "@rules_rust//rust/platform:x86_64-apple-darwin": "@rust_darwin_x86_64//:rustfmt_bin", + "@rules_rust//rust/platform:x86_64-pc-windows-msvc": "@rust_windows_x86_64//:rustfmt_bin", + "@rules_rust//rust/platform:x86_64-unknown-linux-gnu": "@rust_linux_x86_64//:rustfmt_bin", + }), +) + +rust_library( + name = "rustfmt_lib", + srcs = glob( + ["srcs/**/*.rs"], + exclude = ["srcs/**/*main.rs"], + ), + data = [ + ":rustfmt_bin", + "//:rustfmt.toml", + ], + edition = "2018", + rustc_env = { + "RUSTFMT": "$(rootpath :rustfmt_bin)", + "RUSTFMT_CONFIG": "$(rootpath //:rustfmt.toml)", + }, +) + +rust_binary( + name = "rustfmt", + srcs = [ + "srcs/main.rs", + ], + data = [ + "//:rustfmt.toml", + ], + edition = "2018", + deps = [ + ":rustfmt_lib", + "//util/label", + ], +) + +rust_binary( + name = "rustfmt_test", + srcs = [ + "srcs/test_main.rs", + ], + edition = "2018", + deps = [ + ":rustfmt_lib", + "//tools/runfiles", + ], +) diff --git a/tools/rustfmt/rustfmt.toml b/tools/rustfmt/rustfmt.toml new file mode 100644 index 0000000000..44bdbf2485 --- /dev/null +++ b/tools/rustfmt/rustfmt.toml @@ -0,0 +1 @@ +# rustfmt options: https://rust-lang.github.io/rustfmt/ diff --git a/tools/rustfmt/srcs/lib.rs b/tools/rustfmt/srcs/lib.rs new file mode 100644 index 0000000000..35ccf78a94 --- /dev/null +++ b/tools/rustfmt/srcs/lib.rs @@ -0,0 +1,76 @@ +use std::env; +use std::fs; +use std::path::{Path, PathBuf}; + +/// The expected extension of rustfmt manifest files generated by `rustfmt_aspect`. +pub const RUSTFMT_MANIFEST_EXTENSION: &'static str = "rustfmt"; + +/// Generate an absolute path to a file without resolving symlinks +fn absolutify_existing>(path: &T) -> std::io::Result { + let absolute_path = if path.as_ref().is_absolute() { + path.as_ref().to_owned() + } else { + std::env::current_dir() + .expect("Failed to get working directory") + .join(path) + }; + std::fs::metadata(&absolute_path).map(|_| absolute_path) +} + +/// A struct containing details used for executing rustfmt. +#[derive(Debug)] +pub struct RustfmtConfig { + /// The rustfmt binary from the currently active toolchain + pub rustfmt: PathBuf, + + /// The rustfmt config file containing rustfmt settings. + /// https://rust-lang.github.io/rustfmt/ + pub config: PathBuf, +} + +/// Parse command line arguments and environment variables to +/// produce config data for running rustfmt. +pub fn parse_rustfmt_config() -> RustfmtConfig { + RustfmtConfig { + rustfmt: absolutify_existing(&env!("RUSTFMT")).expect("Unable to find rustfmt binary"), + config: absolutify_existing(&env!("RUSTFMT_CONFIG")) + .expect("Unable to find rustfmt config file"), + } +} + +/// A struct of target specific information for use in running `rustfmt`. +#[derive(Debug)] +pub struct RustfmtManifest { + /// The Rust edition of the Bazel target + pub edition: String, + + /// A list of all (non-generated) source files for formatting. + pub sources: Vec, +} + +/// Parse rustfmt flags from a manifest generated by builds using `rustfmt_aspect`. +pub fn parse_rustfmt_manifest(manifest: &Path) -> RustfmtManifest { + let content = fs::read_to_string(manifest).expect(&format!( + "Failed to read rustfmt manifest: {}", + manifest.display() + )); + + let mut lines: Vec = content + .split("\n") + .into_iter() + .filter(|s| !s.is_empty()) + .map(|s| s.to_owned()) + .collect(); + + let edition = lines + .pop() + .expect("There should always be at least 1 line in the manifest"); + edition + .parse::() + .expect("The edition should be a numeric value. eg `2018`."); + + RustfmtManifest { + edition: edition, + sources: lines, + } +} diff --git a/tools/rustfmt/srcs/main.rs b/tools/rustfmt/srcs/main.rs new file mode 100644 index 0000000000..74fea5d158 --- /dev/null +++ b/tools/rustfmt/srcs/main.rs @@ -0,0 +1,185 @@ +use std::env; +use std::path::PathBuf; +use std::process::{Command, Stdio}; +use std::str; + +use label; +use rustfmt_lib; + +fn main() { + // Gather all command line and environment settings + let options = parse_args(); + + // Gather a list of all formattable targets + let targets = query_rustfmt_targets(&options); + + // Run rustfmt on these targets + apply_rustfmt(&options, &targets); +} + +/// Perform a `bazel` query to determine a list of Bazel targets which are to be formatted. +fn query_rustfmt_targets(options: &Config) -> Vec { + // Determine what packages to query + let scope = match options.packages.is_empty() { + true => "//...:all".to_owned(), + false => { + // Check to see if all the provided packages are actually targets + let is_all_targets = options + .packages + .iter() + .all(|pkg| match label::analyze(pkg) { + Ok(tgt) => tgt.name != "all", + Err(_) => false, + }); + + // Early return if a list of targets and not packages were provided + if is_all_targets { + return options.packages.clone(); + } + + options.packages.join(" + ") + } + }; + + let query_args = vec![ + "query".to_owned(), + format!( + r#"kind('{types}', {scope}) except attr(tags, 'norustfmt', kind('{types}', {scope}))"#, + types = "^rust_", + scope = scope + ), + ]; + + let child = Command::new(&options.bazel) + .current_dir(&options.workspace) + .args(query_args) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .spawn() + .expect("Failed to spawn bazel query command"); + + let output = child + .wait_with_output() + .expect("Failed to wait on spawned command"); + + if !output.status.success() { + std::process::exit(output.status.code().unwrap_or(1)); + } + + str::from_utf8(&output.stdout) + .expect("Invalid stream from command") + .split("\n") + .filter(|line| !line.is_empty()) + .map(|line| line.to_string()) + .collect() +} + +/// Build a list of Bazel targets using the `rustfmt_aspect` to produce the +/// arguments to use when formatting the sources of those targets. +fn generate_rustfmt_target_manifests(options: &Config, targets: &Vec) { + let build_args = vec![ + "build", + "--aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect", + "--output_groups=rustfmt_manifest", + ]; + + let child = Command::new(&options.bazel) + .current_dir(&options.workspace) + .args(build_args) + .args(targets) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .spawn() + .expect("Failed to spawn command"); + + let output = child + .wait_with_output() + .expect("Failed to wait on spawned command"); + + if !output.status.success() { + std::process::exit(output.status.code().unwrap_or(1)); + } +} + +/// Run rustfmt on a set of Bazel targets +fn apply_rustfmt(options: &Config, targets: &Vec) { + // Ensure the targets are first built and a manifest containing `rustfmt` + // arguments are generated before formatting source files. + generate_rustfmt_target_manifests(&options, &targets); + + for target in targets.iter() { + // Replace any `:` characters and strip leading slashes + let target_path = target.replace(":", "/").trim_start_matches("/").to_owned(); + + // Find a manifest for the current target. Not all targets will have one + let manifest = options.workspace.join("bazel-bin").join(format!( + "{}.{}", + &target_path, + rustfmt_lib::RUSTFMT_MANIFEST_EXTENSION, + )); + + if !manifest.exists() { + continue; + } + + // Load the manifest containing rustfmt arguments + let rustfmt_config = rustfmt_lib::parse_rustfmt_manifest(&manifest); + + // Ignore any targets which do not have source files. This can + // occur in cases where all source files are generated. + if rustfmt_config.sources.is_empty() { + continue; + } + + // Run rustfmt + let status = Command::new(&options.rustfmt_config.rustfmt) + .current_dir(&options.workspace) + .arg("--edition") + .arg(rustfmt_config.edition) + .arg("--config-path") + .arg(&options.rustfmt_config.config) + .args(rustfmt_config.sources) + .status() + .expect("Failed to run rustfmt"); + + if !status.success() { + std::process::exit(status.code().unwrap_or(1)); + } + } +} + +/// A struct containing details used for executing rustfmt. +#[derive(Debug)] +struct Config { + /// The path of the Bazel workspace root. + pub workspace: PathBuf, + + /// The Bazel executable to use for builds and queries. + pub bazel: PathBuf, + + /// Information about the current rustfmt binary to run. + pub rustfmt_config: rustfmt_lib::RustfmtConfig, + + /// Optionally, users can pass a list of targets/packages/scopes + /// (eg `//my:target` or `//my/pkg/...`) to control the targets + /// to be formatted. If empty, all targets in the workspace will + /// be formatted. + pub packages: Vec, +} + +/// Parse command line arguments and environment variables to +/// produce config data for running rustfmt. +fn parse_args() -> Config { + Config{ + workspace: PathBuf::from( + env::var("BUILD_WORKSPACE_DIRECTORY") + .expect("The environment variable BUILD_WORKSPACE_DIRECTORY is required for finding the workspace root") + ), + bazel: PathBuf::from( + env::var("BAZEL_REAL") + .unwrap_or_else(|_| "bazel".to_owned()) + ), + rustfmt_config: rustfmt_lib::parse_rustfmt_config(), + packages: env::args().skip(1).collect(), + } +} diff --git a/tools/rustfmt/srcs/test_main.rs b/tools/rustfmt/srcs/test_main.rs new file mode 100644 index 0000000000..d1520d7a0f --- /dev/null +++ b/tools/rustfmt/srcs/test_main.rs @@ -0,0 +1,98 @@ +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command; + +use runfiles; +use rustfmt_lib; + +fn main() { + // Gather all and environment settings + let options = parse_args(); + + // Perform rustfmt for each manifest available + run_rustfmt(&options); +} + +/// Run rustfmt on a set of Bazel targets +fn run_rustfmt(options: &Config) { + // In order to ensure the test parses all sources, we separately + // track whether or not a failure has occured when checking formatting. + let mut is_failure: bool = false; + + for manifest in options.manifests.iter() { + // Ignore any targets which do not have source files. This can + // occur in cases where all source files are generated. + if manifest.sources.is_empty() { + continue; + } + + // Run rustfmt + let status = Command::new(&options.rustfmt_config.rustfmt) + .arg("--check") + .arg("--edition") + .arg(&manifest.edition) + .arg("--config-path") + .arg(&options.rustfmt_config.config) + .args(&manifest.sources) + .status() + .expect("Failed to run rustfmt"); + + if !status.success() { + is_failure = true; + } + } + + if is_failure { + std::process::exit(1); + } +} + +/// A struct containing details used for executing rustfmt. +#[derive(Debug)] +struct Config { + /// Information about the current rustfmt binary to run. + pub rustfmt_config: rustfmt_lib::RustfmtConfig, + + /// A list of manifests containing information about sources + /// to check using rustfmt. + pub manifests: Vec, +} + +/// Parse the runfiles of the current executable for manifests generated +/// but the `rustfmt_aspect` aspect. +fn find_manifests(dir: &Path, manifests: &mut Vec) { + if dir.is_dir() { + for entry in fs::read_dir(dir).expect("Failed to read directory contents") { + let entry = entry.expect("Failed to read directory entry"); + let path = entry.path(); + if path.is_dir() { + find_manifests(&path, manifests); + } else if let Some(ext) = path.extension() { + if ext == rustfmt_lib::RUSTFMT_MANIFEST_EXTENSION { + manifests.extend(vec![path]); + } + } + } + } +} + +/// Parse settings from the environment into a config struct +fn parse_args() -> Config { + let mut manifests: Vec = Vec::new(); + find_manifests( + &runfiles::find_runfiles_dir().expect("Failed to find runfiles directory"), + &mut manifests, + ); + + if manifests.is_empty() { + panic!("No manifests were found"); + } + + Config { + rustfmt_config: rustfmt_lib::parse_rustfmt_config(), + manifests: manifests + .iter() + .map(|manifest| rustfmt_lib::parse_rustfmt_manifest(&manifest)) + .collect(), + } +} diff --git a/util/label/label.rs b/util/label/label.rs index 158a570844..bd11a6e404 100644 --- a/util/label/label.rs +++ b/util/label/label.rs @@ -18,9 +18,9 @@ pub fn analyze<'s>(input: &'s str) -> Result> { #[derive(Debug, PartialEq)] pub struct Label<'s> { - repository_name: Option<&'s str>, - package_name: Option<&'s str>, - name: &'s str, + pub repository_name: Option<&'s str>, + pub package_name: Option<&'s str>, + pub name: &'s str, } type Result = core::result::Result;