From 17a050682db060e55d811d7aa341bceb70ddac57 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 3 May 2021 09:05:49 -0700 Subject: [PATCH 01/20] Added `rustfmt` rules and aspects --- .bazelci/presubmit.yml | 14 ++ docs/BUILD.bazel | 9 + docs/index.md | 1 + docs/symbols.bzl | 9 + rust/defs.bzl | 23 ++- rust/private/rustfmt.bzl | 215 +++++++++++++++++++++++ test/rustfmt/BUILD.bazel | 73 ++++++-- test/rustfmt/rustfmt_failure_test.sh | 89 ++++++++++ test/rustfmt/rustfmt_generator.bzl | 3 + test/rustfmt/rustfmt_test.sh | 8 - test/rustfmt/srcs/2015/formatted.rs | 3 + test/rustfmt/srcs/2015/unformatted.rs | 1 + test/rustfmt/srcs/2018/formatted.rs | 40 +++++ test/rustfmt/srcs/2018/unformatted.rs | 22 +++ test/rustfmt/unformatted.rs | 1 - test/unit/crate_name/crate_name_test.bzl | 4 +- tools/rustfmt/BUILD.bazel | 32 ++++ tools/rustfmt/rustfmt.toml | 1 + tools/rustfmt/srcs/main.rs | 207 ++++++++++++++++++++++ 19 files changed, 726 insertions(+), 29 deletions(-) create mode 100644 rust/private/rustfmt.bzl create mode 100755 test/rustfmt/rustfmt_failure_test.sh delete mode 100755 test/rustfmt/rustfmt_test.sh create mode 100644 test/rustfmt/srcs/2015/formatted.rs create mode 100644 test/rustfmt/srcs/2015/unformatted.rs create mode 100644 test/rustfmt/srcs/2018/formatted.rs create mode 100644 test/rustfmt/srcs/2018/unformatted.rs delete mode 100644 test/rustfmt/unformatted.rs create mode 100644 tools/rustfmt/BUILD.bazel create mode 100644 tools/rustfmt/rustfmt.toml create mode 100644 tools/rustfmt/srcs/main.rs diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 0e386bbf9e..6ac646e96a 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_check_aspect" + - "--output_groups=rustfmt" + 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/docs/BUILD.bazel b/docs/BUILD.bazel index 64e2dd0e89..f40f8e4101 100644 --- a/docs/BUILD.bazel +++ b/docs/BUILD.bazel @@ -88,6 +88,15 @@ PAGES = dict([ "rust_doc_test", ], ), + page( + name = "rust_fmt", + symbols = [ + "rustfmt", + "rustfmt_aspect", + "rustfmt_check", + "rustfmt_check_aspect", + ], + ), page( name = "rust_proto", symbols = [ 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/symbols.bzl b/docs/symbols.bzl index 67351cc945..ac0e03b786 100644 --- a/docs/symbols.bzl +++ b/docs/symbols.bzl @@ -50,6 +50,10 @@ load( _rust_static_library = "rust_static_library", _rust_test = "rust_test", _rust_test_suite = "rust_test_suite", + _rustfmt = "rustfmt", + _rustfmt_aspect = "rustfmt_aspect", + _rustfmt_check = "rustfmt_check", + _rustfmt_check_aspect = "rustfmt_check_aspect", ) load( "@rules_rust//rust:repositories.bzl", @@ -113,3 +117,8 @@ rust_analyzer_aspect = _rust_analyzer_aspect crate_universe = _crate_universe crate = _crate + +rustfmt = _rustfmt +rustfmt_aspect = _rustfmt_aspect +rustfmt_check = _rustfmt_check +rustfmt_check_aspect = _rustfmt_check_aspect diff --git a/rust/defs.bzl b/rust/defs.bzl index e1c2458e94..2ccacf73e8 100644 --- a/rust/defs.bzl +++ b/rust/defs.bzl @@ -49,6 +49,13 @@ load( "//rust/private:rustdoc_test.bzl", _rust_doc_test = "rust_doc_test", ) +load( + "//rust/private:rustfmt.bzl", + _rustfmt = "rustfmt", + _rustfmt_aspect = "rustfmt_aspect", + _rustfmt_check = "rustfmt_check", + _rustfmt_check_aspect = "rustfmt_check_aspect", +) rust_library = _rust_library # See @rules_rust//rust/private:rust.bzl for a complete description. @@ -96,7 +103,19 @@ 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 = _rustfmt +# See @rules_rust//rust/private:rustfmt.bzl for a complete description. + +rustfmt_aspect = _rustfmt_aspect +# See @rules_rust//rust/private:rustfmt.bzl for a complete description. + +rustfmt_check_aspect = _rustfmt_check_aspect +# See @rules_rust//rust/private:rustfmt.bzl for a complete description. + +rustfmt_check = _rustfmt_check +# 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..28b6f4ed55 --- /dev/null +++ b/rust/private/rustfmt.bzl @@ -0,0 +1,215 @@ +"""A module defining rustfmt rules""" + +load(":common.bzl", "rust_common") +load(":rust.bzl", "rust_binary") +load(":utils.bzl", "find_toolchain") + +def _rustfmt_aspect_impl(target, ctx): + if rust_common.crate_info not in target: + return [] + + # Targets annotated with `norustfmt` will not be formatted + if "norustfmt" in ctx.rule.attr.tags: + return [] + + crate_info = target[rust_common.crate_info] + + # Filter out any generated files + srcs = [src.path for src in crate_info.srcs.to_list() if src.is_source] + + manifest = ctx.actions.declare_file(ctx.label.name + ".rustfmt") + ctx.actions.write( + output = manifest, + content = "\n".join([ + crate_info.edition, + ] + srcs), + ) + + return [ + DefaultInfo( + files = depset([manifest]), + ), + OutputGroupInfo( + rustfmt = depset([manifest]), + ), + ] + +rustfmt_aspect = aspect( + implementation = _rustfmt_aspect_impl, + doc = """\ +This aspect is used to gather information about a crate for use in rustfmt + +This aspect is used directly by [rustfmt](#rustfmt) targets to determine the +appropriate flags to use when formatting Rust sources. For more details on how +to format source code, see the [rustfmt](#rustfmt) rule. +""", +) + +def _rustfmt_check_aspect_impl(target, ctx): + if rust_common.crate_info not in target: + return [] + + # Targets annotated with `norustfmt` will not be formatted + if "norustfmt" in 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] + + # Only run `rustfmt` if we actually have sources to format. Some rules may produce only + # generated sources for `CrateInfo` and these are not necessary to check. + if not srcs: + return [] + + 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(crate_info.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 [ + OutputGroupInfo( + rustfmt = depset([marker]), + ), + ] + +rustfmt_check_aspect = aspect( + implementation = _rustfmt_check_aspect_impl, + fragments = ["cpp"], + host_fragments = ["cpp"], + toolchains = [ + str(Label("//rust:toolchain")), + ], + attrs = { + "_process_wrapper": attr.label( + doc = "A process wrapper for running clippy on all platforms", + cfg = "exec", + executable = True, + default = Label("//util/process_wrapper"), + ), + }, + incompatible_use_toolchain_transition = True, + doc = """\ +Executes rustfmt in `--check` mode on the specified target. + +To enable this aspect for your workspace, simply add the following to the `.bazelrc` +file in the root of any workspace which loads `rules_rust`. + +``` +build --aspects=@rules_rust//rust:defs.bzl%rustfmt_check_aspect +build --output_groups=+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. +""", +) + +def _rustfmt_check_impl(ctx): + files = depset([], transitive = [target[OutputGroupInfo].rustfmt for target in ctx.attr.targets]) + return [DefaultInfo(files = files)] + +rustfmt_check = rule( + implementation = _rustfmt_check_impl, + attrs = { + "targets": attr.label_list( + doc = "Rust targets to run rustfmt on.", + providers = [rust_common.crate_info], + aspects = [rustfmt_check_aspect], + ), + }, + doc = """\ +A rule for defining a target which runs `rustfmt` in `--check` mode on an explicit list of targets + +For more information on the use of `rustfmt` directly, see [rustfmt_check_aspect](#rustfmt_check_aspect). +""", +) + +def rustfmt(name, config = Label("//tools/rustfmt:rustfmt.toml")): + """A macro defining a [rustfmt](https://github.com/rust-lang/rustfmt#readme) runner. + + This macro is used to generate a rustfmt binary which can be run to format the Rust source + files of `rules_rust` targets in the workspace. To define this target, simply load and call + it in a BUILD file. + + eg: `//:BUILD.bazel` + + ```python + load("@rules_rust//rust:defs.bzl", "rustfmt") + + rustfmt( + name = "rustfmt", + ) + ``` + + This now allows users to run `bazel run //:rustfmt` to format any target which provides `CrateInfo`. + + This binary also supports accepts a [label](https://docs.bazel.build/versions/master/build-ref.html#labels) or + pattern (`//my/package/...`) to allow for more granular control over what targets get formatted. This + can be useful when dealing with larger projects as `rustfmt` can only be run on a target which successfully + builds. Given the following workspace layout: + + ``` + WORKSPACE.bazel + BUILD.bazel + package_a/ + BUILD.bazel + src/ + lib.rs + mod_a.rs + mod_b.rs + package_b/ + BUILD.bazel + subpackage_1/ + BUILD.bazel + main.rs + subpackage_2/ + BUILD.bazel + main.rs + ``` + + Users can choose to only format the `rust_lib` target in `package_a` using `bazel run //:rustfmt -- //package_a:rust_lib`. + Additionally, users can format all of `package_b` using `bazel run //:rustfmt -- //package_b/...`. + + Users not looking to add a custom `rustfmt` config can simply run the `@rules_rust//tools/rustfmt` to avoid defining their + own target. + + Note that generated sources will be ignored and targets tagged as `norustfmt` will be skipped. + + Args: + name (str): The name of the rustfmt runner + config (Label, optional): The [rustfmt config](https://rust-lang.github.io/rustfmt/) to use. + """ + rust_binary( + name = name, + rustc_env = { + "RUSTFMT": "$(rootpath {})".format(Label("//tools/rustfmt:rustfmt_bin")), + "RUSTFMT_CONFIG": "$(rootpath {})".format(config), + }, + data = [ + config, + Label("//tools/rustfmt:rustfmt_bin"), + ], + srcs = [Label("//tools/rustfmt:srcs")], + edition = "2018", + ) diff --git a/test/rustfmt/BUILD.bazel b/test/rustfmt/BUILD.bazel index 2e4b67bf3e..5eaf4a197f 100644 --- a/test/rustfmt/BUILD.bazel +++ b/test/rustfmt/BUILD.bazel @@ -1,17 +1,58 @@ -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_check") + +rust_binary( + name = "formatted_2018", + srcs = ["srcs/2018/formatted.rs"], + edition = "2018", +) + +rustfmt_check( + name = "check_formatted_2018", + testonly = True, + tags = ["manual"], + targets = [":formatted_2018"], +) + +rust_binary( + name = "unformatted_2018", + srcs = ["srcs/2018/unformatted.rs"], + edition = "2018", +) + +rustfmt_check( + name = "check_unformatted_2018", + testonly = True, + tags = ["manual"], + targets = [":unformatted_2018"], +) + +rust_binary( + name = "formatted_2015", + srcs = ["srcs/2015/formatted.rs"], + edition = "2015", +) + +rustfmt_check( + name = "check_formatted_2015", + testonly = True, + tags = ["manual"], + targets = [":formatted_2015"], +) + +rust_binary( + name = "unformatted_2015", + srcs = ["srcs/2015/unformatted.rs"], + edition = "2015", +) + +rustfmt_check( + name = "check_unformatted_2015", + testonly = True, + 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..ec474634af --- /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 build //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 build //test/rustfmt:${2}\" to see the output" + exit 1 + else + echo "OK" + fi +} + +function test_all() { + local -r BUILD_OK=0 + local -r BUILD_FAILED=1 + + check_build_result $BUILD_FAILED check_unformatted_2015 + check_build_result $BUILD_FAILED check_unformatted_2018 + check_build_result $BUILD_OK check_formatted_2015 + check_build_result $BUILD_OK check_formatted_2018 +} + +function test_apply() { + local -r BUILD_OK=0 + local -r BUILD_FAILED=1 + + 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 $BUILD_FAILED check_unformatted_2015 + check_build_result $BUILD_OK check_unformatted_2018 + check_build_result $BUILD_OK check_formatted_2015 + check_build_result $BUILD_OK check_formatted_2018 + + # Format all targets + bazel run @rules_rust//tools/rustfmt + + check_build_result $BUILD_OK check_unformatted_2015 + check_build_result $BUILD_OK check_unformatted_2018 + check_build_result $BUILD_OK check_formatted_2015 + check_build_result $BUILD_OK check_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..2fb232080a --- /dev/null +++ b/test/rustfmt/srcs/2018/unformatted.rs @@ -0,0 +1,22 @@ +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/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/rustfmt/BUILD.bazel b/tools/rustfmt/BUILD.bazel new file mode 100644 index 0000000000..e8809cf769 --- /dev/null +++ b/tools/rustfmt/BUILD.bazel @@ -0,0 +1,32 @@ +load("//rust:defs.bzl", "rustfmt", "rustfmt_check") + +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", + }), +) + +filegroup( + name = "srcs", + srcs = glob(["srcs/**/*.rs"]), +) + +rustfmt( + name = "rustfmt", +) + +rustfmt_check( + name = "check", + targets = [ + "rustfmt", + ], +) 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/main.rs b/tools/rustfmt/srcs/main.rs new file mode 100644 index 0000000000..bc424530fa --- /dev/null +++ b/tools/rustfmt/srcs/main.rs @@ -0,0 +1,207 @@ +use std::collections::VecDeque; +use std::env; +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; +use std::str; + +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); +} + +/// A set of supported rules for use in a `bazel query` when querying for Rust targets. +const SUPPORTED_RULES: &str = + "rust_library|rust_proc_macro|rust_shared_library|rust_static_library|rust_binary|rust_test"; + +/// Perform a `bazel` query to determine a list of Bazel targets which are to be formatted. +fn query_rustfmt_targets(options: &Config) -> Vec { + let scope = match &options.package { + Some(target) => { + if !target.ends_with(":all") && !target.ends_with("...") { + return vec![target.to_string()]; + } + target + } + None => "//...:all", + }; + + let query_args = vec![ + "query".to_owned(), + format!( + r#"kind('{types}', {scope}) except attr(tags, 'norustfmt|manual', kind('{types}', {scope}))"#, + types = SUPPORTED_RULES, + 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 build_rustfmt_targets(options: &Config, targets: &Vec) { + let build_args = vec![ + "build", + "--aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect", + "--output_groups=+rustfmt", + ]; + + 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. + build_rustfmt_targets(&options, &targets); + + for target in targets.iter() { + // Replace any `:` characters and strip leading slashes + let target_path = target.replace(":", "/").trim_start_matches("/").to_owned(); + + // Load the manifest containing rustfmt arguments + let rustfmt_config = parse_rustfmt_manifest( + &options + .workspace + .join("bazel-bin") + .join(format!("{}.rustfmt", &target_path)), + ); + + // 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) + .current_dir(&options.workspace) + .arg("--edition") + .arg(rustfmt_config.edition) + .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, + + /// 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, + + /// An optional command line flag used to control what targets + /// to format. Users are expected to either pass a Bazel label + /// or a package pattern (`//my/package/...`). + pub package: Option, +} + +/// 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: PathBuf::from(env!("RUSTFMT")) + .canonicalize() + .expect("Unable to find rustfmt binary"), + config: PathBuf::from(env!("RUSTFMT_CONFIG")) + .canonicalize() + .expect("Unable to find rustfmt config file"), + package: env::args().nth(1), + } +} + +/// Parse rustfmt flags from a manifest generated by builds using `rustfmt_aspect`. +fn parse_rustfmt_manifest(manifest: &Path) -> RustfmtConfig { + let content = fs::read_to_string(manifest).expect(&format!( + "Failed to read rustfmt manifest: {}", + manifest.display() + )); + + let mut lines: VecDeque = content + .split("\n") + .into_iter() + .map(|s| s.to_owned()) + .collect(); + + RustfmtConfig { + edition: lines + .pop_front() + .expect("There should always be 1 line in the manifest"), + sources: lines.into(), + } +} + +/// A struct of target specific information for use in running `rustfmt`. +#[derive(Debug)] +struct RustfmtConfig { + /// The Rust edition of the Bazel target + pub edition: String, + + /// A list of all (non-generated) source files for formatting. + pub sources: Vec, +} From cbf85de4022018805ff151c3b741fd2fcd7926ce Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 6 May 2021 07:18:02 -0700 Subject: [PATCH 02/20] Regenerate documentation --- docs/flatten.md | 156 ++++++++++++++++++++++++++++++++++++++++++++++ docs/rust_fmt.md | 159 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 315 insertions(+) create mode 100644 docs/rust_fmt.md diff --git a/docs/flatten.md b/docs/flatten.md index 6a0bca539f..49dd040f7a 100644 --- a/docs/flatten.md +++ b/docs/flatten.md @@ -33,6 +33,10 @@ * [rust_wasm_bindgen](#rust_wasm_bindgen) * [rust_wasm_bindgen_repositories](#rust_wasm_bindgen_repositories) * [rust_wasm_bindgen_toolchain](#rust_wasm_bindgen_toolchain) +* [rustfmt](#rustfmt) +* [rustfmt_aspect](#rustfmt_aspect) +* [rustfmt_check](#rustfmt_check) +* [rustfmt_check_aspect](#rustfmt_check_aspect) @@ -1281,6 +1285,28 @@ For additional information, see the [Bazel toolchains documentation][toolchains] | bindgen | The label of a wasm-bindgen-cli executable. | Label | optional | None | + + +## rustfmt_check + +
+rustfmt_check(name, targets)
+
+ +A rule for defining a target which runs `rustfmt` in `--check` mode on an explicit list of targets + +For more information on the use of `rustfmt` directly, see [rustfmt_check_aspect](#rustfmt_check_aspect). + + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | +| targets | Rust targets to run rustfmt on. | List of labels | optional | [] | + + ## cargo_build_script @@ -1682,6 +1708,74 @@ Declare dependencies needed for [rust_wasm_bindgen](#rust_wasm_bindgen). | register_default_toolchain | If True, the default [rust_wasm_bindgen_toolchain](#rust_wasm_bindgen_toolchain) (@rules_rust//wasm_bindgen:default_wasm_bindgen_toolchain) is registered. This toolchain requires a set of dependencies that were generated using [cargo raze](https://github.com/google/cargo-raze). These will also be loaded. | True | + + +## rustfmt + +
+rustfmt(name, config)
+
+ +A macro defining a [rustfmt](https://github.com/rust-lang/rustfmt#readme) runner. + +This macro is used to generate a rustfmt binary which can be run to format the Rust source +files of `rules_rust` targets in the workspace. To define this target, simply load and call +it in a BUILD file. + +eg: `//:BUILD.bazel` + +```python +load("@rules_rust//rust:defs.bzl", "rustfmt") + +rustfmt( + name = "rustfmt", +) +``` + +This now allows users to run `bazel run //:rustfmt` to format any target which provides `CrateInfo`. + +This binary also supports accepts a [label](https://docs.bazel.build/versions/master/build-ref.html#labels) or +pattern (`//my/package/...`) to allow for more granular control over what targets get formatted. This +can be useful when dealing with larger projects as `rustfmt` can only be run on a target which successfully +builds. Given the following workspace layout: + +``` +WORKSPACE.bazel +BUILD.bazel +package_a/ + BUILD.bazel + src/ + lib.rs + mod_a.rs + mod_b.rs +package_b/ + BUILD.bazel + subpackage_1/ + BUILD.bazel + main.rs + subpackage_2/ + BUILD.bazel + main.rs +``` + +Users can choose to only format the `rust_lib` target in `package_a` using `bazel run //:rustfmt -- //package_a:rust_lib`. +Additionally, users can format all of `package_b` using `bazel run //:rustfmt -- //package_b/...`. + +Users not looking to add a custom `rustfmt` config can simply run the `@rules_rust//tools/rustfmt` to avoid defining their +own target. + +Note that generated sources will be ignored and targets tagged as `norustfmt` will be skipped. + + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| name | The name of the rustfmt runner | none | +| config | The [rustfmt config](https://rust-lang.github.io/rustfmt/) to use. | Label("//tools/rustfmt:rustfmt.toml") | + + ## rust_analyzer_aspect @@ -1757,3 +1851,65 @@ $ 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 + +This aspect is used directly by [rustfmt](#rustfmt) targets to determine the +appropriate flags to use when formatting Rust sources. For more details on how +to format source code, see the [rustfmt](#rustfmt) rule. + + +**ASPECT ATTRIBUTES** + + + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | + + + + +## rustfmt_check_aspect + +
+rustfmt_check_aspect(name)
+
+ +Executes rustfmt in `--check` mode on the specified target. + +To enable this aspect for your workspace, simply add the following to the `.bazelrc` +file in the root of any workspace which loads `rules_rust`. + +``` +build --aspects=@rules_rust//rust:defs.bzl%rustfmt_check_aspect +build --output_groups=+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.md b/docs/rust_fmt.md new file mode 100644 index 0000000000..677ce7ec1c --- /dev/null +++ b/docs/rust_fmt.md @@ -0,0 +1,159 @@ +# Rust Fmt + +* [rustfmt](#rustfmt) +* [rustfmt_aspect](#rustfmt_aspect) +* [rustfmt_check](#rustfmt_check) +* [rustfmt_check_aspect](#rustfmt_check_aspect) + + + +## rustfmt_check + +
+rustfmt_check(name, targets)
+
+ +A rule for defining a target which runs `rustfmt` in `--check` mode on an explicit list of targets + +For more information on the use of `rustfmt` directly, see [rustfmt_check_aspect](#rustfmt_check_aspect). + + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | +| targets | Rust targets to run rustfmt on. | List of labels | optional | [] | + + + + +## rustfmt + +
+rustfmt(name, config)
+
+ +A macro defining a [rustfmt](https://github.com/rust-lang/rustfmt#readme) runner. + +This macro is used to generate a rustfmt binary which can be run to format the Rust source +files of `rules_rust` targets in the workspace. To define this target, simply load and call +it in a BUILD file. + +eg: `//:BUILD.bazel` + +```python +load("@rules_rust//rust:defs.bzl", "rustfmt") + +rustfmt( + name = "rustfmt", +) +``` + +This now allows users to run `bazel run //:rustfmt` to format any target which provides `CrateInfo`. + +This binary also supports accepts a [label](https://docs.bazel.build/versions/master/build-ref.html#labels) or +pattern (`//my/package/...`) to allow for more granular control over what targets get formatted. This +can be useful when dealing with larger projects as `rustfmt` can only be run on a target which successfully +builds. Given the following workspace layout: + +``` +WORKSPACE.bazel +BUILD.bazel +package_a/ + BUILD.bazel + src/ + lib.rs + mod_a.rs + mod_b.rs +package_b/ + BUILD.bazel + subpackage_1/ + BUILD.bazel + main.rs + subpackage_2/ + BUILD.bazel + main.rs +``` + +Users can choose to only format the `rust_lib` target in `package_a` using `bazel run //:rustfmt -- //package_a:rust_lib`. +Additionally, users can format all of `package_b` using `bazel run //:rustfmt -- //package_b/...`. + +Users not looking to add a custom `rustfmt` config can simply run the `@rules_rust//tools/rustfmt` to avoid defining their +own target. + +Note that generated sources will be ignored and targets tagged as `norustfmt` will be skipped. + + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| name | The name of the rustfmt runner | none | +| config | The [rustfmt config](https://rust-lang.github.io/rustfmt/) to use. | Label("//tools/rustfmt:rustfmt.toml") | + + + + +## rustfmt_aspect + +
+rustfmt_aspect(name)
+
+ +This aspect is used to gather information about a crate for use in rustfmt + +This aspect is used directly by [rustfmt](#rustfmt) targets to determine the +appropriate flags to use when formatting Rust sources. For more details on how +to format source code, see the [rustfmt](#rustfmt) rule. + + +**ASPECT ATTRIBUTES** + + + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | + + + + +## rustfmt_check_aspect + +
+rustfmt_check_aspect(name)
+
+ +Executes rustfmt in `--check` mode on the specified target. + +To enable this aspect for your workspace, simply add the following to the `.bazelrc` +file in the root of any workspace which loads `rules_rust`. + +``` +build --aspects=@rules_rust//rust:defs.bzl%rustfmt_check_aspect +build --output_groups=+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 | | + + From 9d46bf07138292dea90985493b1e30c236bea9d1 Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Thu, 13 May 2021 09:40:18 -0700 Subject: [PATCH 03/20] Update rust/private/rustfmt.bzl Co-authored-by: Daniel Wagner-Hall --- rust/private/rustfmt.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/private/rustfmt.bzl b/rust/private/rustfmt.bzl index 28b6f4ed55..cd6a8aca7c 100644 --- a/rust/private/rustfmt.bzl +++ b/rust/private/rustfmt.bzl @@ -101,7 +101,7 @@ rustfmt_check_aspect = aspect( ], attrs = { "_process_wrapper": attr.label( - doc = "A process wrapper for running clippy on all platforms", + doc = "A process wrapper for running rustfmt on all platforms", cfg = "exec", executable = True, default = Label("//util/process_wrapper"), From fffe3e87b9d68c833592be448e9c33e9005eca64 Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Thu, 13 May 2021 09:40:35 -0700 Subject: [PATCH 04/20] Update test/rustfmt/rustfmt_failure_test.sh Co-authored-by: Daniel Wagner-Hall --- test/rustfmt/rustfmt_failure_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rustfmt/rustfmt_failure_test.sh b/test/rustfmt/rustfmt_failure_test.sh index ec474634af..21619edfec 100755 --- a/test/rustfmt/rustfmt_failure_test.sh +++ b/test/rustfmt/rustfmt_failure_test.sh @@ -12,7 +12,7 @@ if [[ -z "${BUILD_WORKSPACE_DIRECTORY:-}" ]]; then exit 1 fi -cd "${BUILD_WORKSPACE_DIRECTORY:-}" +cd "${BUILD_WORKSPACE_DIRECTORY}" # Executes a bazel build command and handles the return value, exiting # upon seeing an error. From 879721c15183195899e5ef3a848faf803037cb20 Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Thu, 13 May 2021 09:43:25 -0700 Subject: [PATCH 05/20] Update tools/rustfmt/srcs/main.rs Co-authored-by: Daniel Wagner-Hall --- tools/rustfmt/srcs/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/rustfmt/srcs/main.rs b/tools/rustfmt/srcs/main.rs index bc424530fa..77529c93a3 100644 --- a/tools/rustfmt/srcs/main.rs +++ b/tools/rustfmt/srcs/main.rs @@ -191,7 +191,7 @@ fn parse_rustfmt_manifest(manifest: &Path) -> RustfmtConfig { RustfmtConfig { edition: lines .pop_front() - .expect("There should always be 1 line in the manifest"), + .expect("There should always be at least 1 line in the manifest"), sources: lines.into(), } } From 95de8dd2920fa4a87e1a10926228d3a13e54489a Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 13 May 2021 10:14:53 -0700 Subject: [PATCH 06/20] Created a helper function for parsing formattable srcs --- rust/private/rustfmt.bzl | 61 ++++++++++++++++++++++++-------------- tools/rustfmt/srcs/main.rs | 2 +- 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/rust/private/rustfmt.bzl b/rust/private/rustfmt.bzl index cd6a8aca7c..490c8f96a1 100644 --- a/rust/private/rustfmt.bzl +++ b/rust/private/rustfmt.bzl @@ -4,7 +4,16 @@ load(":common.bzl", "rust_common") load(":rust.bzl", "rust_binary") load(":utils.bzl", "find_toolchain") -def _rustfmt_aspect_impl(target, ctx): +def _find_rustfmtable_srcs(target, ctx): + """Parse a target for rustfmt formattable sources. + + Args: + target (Target): The target the aspect is running on. + ctx (ctx): The aspect's context object. + + Returns: + list: A list of formattable sources (`File`). + """ if rust_common.crate_info not in target: return [] @@ -15,14 +24,30 @@ def _rustfmt_aspect_impl(target, ctx): crate_info = target[rust_common.crate_info] # Filter out any generated files - srcs = [src.path for src in crate_info.srcs.to_list() if src.is_source] + srcs = [src for src in crate_info.srcs.to_list() if src.is_source] + + return srcs + +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 + # 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([ - crate_info.edition, - ] + srcs), + edition, + ] + src_paths), ) return [ @@ -30,7 +55,7 @@ def _rustfmt_aspect_impl(target, ctx): files = depset([manifest]), ), OutputGroupInfo( - rustfmt = depset([manifest]), + rustfmt_manifest = depset([manifest]), ), ] @@ -46,25 +71,17 @@ to format source code, see the [rustfmt](#rustfmt) rule. ) def _rustfmt_check_aspect_impl(target, ctx): - if rust_common.crate_info not in target: - return [] - - # Targets annotated with `norustfmt` will not be formatted - if "norustfmt" in ctx.rule.attr.tags: - return [] + srcs = _find_rustfmtable_srcs(target, ctx) - 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] - - # Only run `rustfmt` if we actually have sources to format. Some rules may produce only - # generated sources for `CrateInfo` and these are not necessary to check. + # If there are no formattable sources, do nothing. if not srcs: return [] toolchain = find_toolchain(ctx) + # Parse the edition to use for formatting from the target + edition = target[rust_common.crate_info].edition + marker = ctx.actions.declare_file(ctx.label.name + ".rustfmt.ok") args = ctx.actions.args() @@ -73,7 +90,7 @@ def _rustfmt_check_aspect_impl(target, ctx): args.add("--") args.add(toolchain.rustfmt) args.add("--edition") - args.add(crate_info.edition) + args.add(edition) args.add("--check") args.add_all(srcs) @@ -88,7 +105,7 @@ def _rustfmt_check_aspect_impl(target, ctx): return [ OutputGroupInfo( - rustfmt = depset([marker]), + rustfmt_check = depset([marker]), ), ] @@ -116,7 +133,7 @@ file in the root of any workspace which loads `rules_rust`. ``` build --aspects=@rules_rust//rust:defs.bzl%rustfmt_check_aspect -build --output_groups=+rustfmt +build --output_groups=+rustfmt_check ``` This aspect is executed on any target which provides the `CrateInfo` provider. However @@ -126,7 +143,7 @@ source files are also ignored by this aspect. ) def _rustfmt_check_impl(ctx): - files = depset([], transitive = [target[OutputGroupInfo].rustfmt for target in ctx.attr.targets]) + files = depset([], transitive = [target[OutputGroupInfo].rustfmt_check for target in ctx.attr.targets]) return [DefaultInfo(files = files)] rustfmt_check = rule( diff --git a/tools/rustfmt/srcs/main.rs b/tools/rustfmt/srcs/main.rs index 77529c93a3..d9c56b8940 100644 --- a/tools/rustfmt/srcs/main.rs +++ b/tools/rustfmt/srcs/main.rs @@ -71,7 +71,7 @@ fn build_rustfmt_targets(options: &Config, targets: &Vec) { let build_args = vec![ "build", "--aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect", - "--output_groups=+rustfmt", + "--output_groups=+rustfmt_manifest", ]; let child = Command::new(&options.bazel) From a5658ec34f2acc2a47f8ff5326e02fcf636aa2b9 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 13 May 2021 10:15:12 -0700 Subject: [PATCH 07/20] Regenerate documentation --- docs/flatten.md | 2 +- docs/rust_fmt.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/flatten.md b/docs/flatten.md index 49dd040f7a..1e3fceab8b 100644 --- a/docs/flatten.md +++ b/docs/flatten.md @@ -1893,7 +1893,7 @@ file in the root of any workspace which loads `rules_rust`. ``` build --aspects=@rules_rust//rust:defs.bzl%rustfmt_check_aspect -build --output_groups=+rustfmt +build --output_groups=+rustfmt_check ``` This aspect is executed on any target which provides the `CrateInfo` provider. However diff --git a/docs/rust_fmt.md b/docs/rust_fmt.md index 677ce7ec1c..41406422c7 100644 --- a/docs/rust_fmt.md +++ b/docs/rust_fmt.md @@ -137,7 +137,7 @@ file in the root of any workspace which loads `rules_rust`. ``` build --aspects=@rules_rust//rust:defs.bzl%rustfmt_check_aspect -build --output_groups=+rustfmt +build --output_groups=+rustfmt_check ``` This aspect is executed on any target which provides the `CrateInfo` provider. However From 698b480b3228a91db392fee0cf9feb4b96f1a193 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 13 May 2021 10:29:58 -0700 Subject: [PATCH 08/20] Remove `canonicalize` --- tools/rustfmt/srcs/main.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tools/rustfmt/srcs/main.rs b/tools/rustfmt/srcs/main.rs index d9c56b8940..ddd3504506 100644 --- a/tools/rustfmt/srcs/main.rs +++ b/tools/rustfmt/srcs/main.rs @@ -131,6 +131,18 @@ fn apply_rustfmt(options: &Config, targets: &Vec) { } } +/// 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)] struct Config { @@ -165,11 +177,9 @@ fn parse_args() -> Config { env::var("BAZEL_REAL") .unwrap_or_else(|_| "bazel".to_owned()) ), - rustfmt: PathBuf::from(env!("RUSTFMT")) - .canonicalize() + rustfmt: absolutify_existing(&env!("RUSTFMT")) .expect("Unable to find rustfmt binary"), - config: PathBuf::from(env!("RUSTFMT_CONFIG")) - .canonicalize() + config: absolutify_existing(&env!("RUSTFMT_CONFIG")) .expect("Unable to find rustfmt config file"), package: env::args().nth(1), } From 8e37bd3a9406703520ef6d65db05b79d964be732 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 13 May 2021 10:49:41 -0700 Subject: [PATCH 09/20] Combined `rustfmt_check_aspect` into `rustfmt_aspect`. --- .bazelci/presubmit.yml | 4 +- docs/BUILD.bazel | 1 - docs/symbols.bzl | 2 - rust/defs.bzl | 4 -- rust/private/rustfmt.bzl | 106 +++++++++++++++++---------------------- 5 files changed, 48 insertions(+), 69 deletions(-) diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 6ac646e96a..06af059873 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -105,8 +105,8 @@ tasks: platform: ubuntu2004 working_directory: examples build_flags: - - "--aspects=@rules_rust//rust:defs.bzl%rustfmt_check_aspect" - - "--output_groups=rustfmt" + - "--aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect" + - "--output_groups=rustfmt_checks" build_targets: - //... rustfmt_failure: diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel index f40f8e4101..d9b87088be 100644 --- a/docs/BUILD.bazel +++ b/docs/BUILD.bazel @@ -94,7 +94,6 @@ PAGES = dict([ "rustfmt", "rustfmt_aspect", "rustfmt_check", - "rustfmt_check_aspect", ], ), page( diff --git a/docs/symbols.bzl b/docs/symbols.bzl index ac0e03b786..a74c75095f 100644 --- a/docs/symbols.bzl +++ b/docs/symbols.bzl @@ -53,7 +53,6 @@ load( _rustfmt = "rustfmt", _rustfmt_aspect = "rustfmt_aspect", _rustfmt_check = "rustfmt_check", - _rustfmt_check_aspect = "rustfmt_check_aspect", ) load( "@rules_rust//rust:repositories.bzl", @@ -121,4 +120,3 @@ crate = _crate rustfmt = _rustfmt rustfmt_aspect = _rustfmt_aspect rustfmt_check = _rustfmt_check -rustfmt_check_aspect = _rustfmt_check_aspect diff --git a/rust/defs.bzl b/rust/defs.bzl index 2ccacf73e8..dbfed285e2 100644 --- a/rust/defs.bzl +++ b/rust/defs.bzl @@ -54,7 +54,6 @@ load( _rustfmt = "rustfmt", _rustfmt_aspect = "rustfmt_aspect", _rustfmt_check = "rustfmt_check", - _rustfmt_check_aspect = "rustfmt_check_aspect", ) rust_library = _rust_library @@ -114,8 +113,5 @@ rustfmt = _rustfmt rustfmt_aspect = _rustfmt_aspect # See @rules_rust//rust/private:rustfmt.bzl for a complete description. -rustfmt_check_aspect = _rustfmt_check_aspect -# See @rules_rust//rust/private:rustfmt.bzl for a complete description. - rustfmt_check = _rustfmt_check # See @rules_rust//rust/private:rustfmt.bzl for a complete description. diff --git a/rust/private/rustfmt.bzl b/rust/private/rustfmt.bzl index 490c8f96a1..1483aca6f0 100644 --- a/rust/private/rustfmt.bzl +++ b/rust/private/rustfmt.bzl @@ -28,16 +28,7 @@ def _find_rustfmtable_srcs(target, ctx): return srcs -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 - +def _generate_manifest(edition, srcs, ctx): # Gather the source paths to non-generated files src_paths = [src.path for src in srcs] @@ -50,38 +41,11 @@ def _rustfmt_aspect_impl(target, ctx): ] + src_paths), ) - return [ - DefaultInfo( - files = depset([manifest]), - ), - OutputGroupInfo( - rustfmt_manifest = depset([manifest]), - ), - ] - -rustfmt_aspect = aspect( - implementation = _rustfmt_aspect_impl, - doc = """\ -This aspect is used to gather information about a crate for use in rustfmt - -This aspect is used directly by [rustfmt](#rustfmt) targets to determine the -appropriate flags to use when formatting Rust sources. For more details on how -to format source code, see the [rustfmt](#rustfmt) rule. -""", -) - -def _rustfmt_check_aspect_impl(target, ctx): - srcs = _find_rustfmtable_srcs(target, ctx) - - # If there are no formattable sources, do nothing. - if not srcs: - return [] + return manifest +def _perform_check(edition, srcs, ctx): toolchain = find_toolchain(ctx) - # Parse the edition to use for formatting from the target - edition = target[rust_common.crate_info].edition - marker = ctx.actions.declare_file(ctx.label.name + ".rustfmt.ok") args = ctx.actions.args() @@ -103,14 +67,51 @@ def _rustfmt_check_aspect_impl(target, ctx): 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_check = depset([marker]), + rustfmt_manifest = depset([manifest]), + rustfmt_checks = depset([marker]), ), ] -rustfmt_check_aspect = aspect( - implementation = _rustfmt_check_aspect_impl, +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`: The `rustfmt_manifest` output is used directly by [rustfmt](#rustfmt) targets +to determine the appropriate flags to use when formatting Rust sources. For more details on how to +format source code, see the [rustfmt](#rustfmt) rule. + +- `rustfmt_checks`: Executes rustfmt in `--check` mode on the specified target. To enable this check +for your workspace, simply add the following to the `.bazelrc` file in the root of any workspace +which loads `rules_rust`: +``` +build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect +build --output_groups=+rustfmt_checks +``` + +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. +""", fragments = ["cpp"], host_fragments = ["cpp"], toolchains = [ @@ -125,25 +126,10 @@ rustfmt_check_aspect = aspect( ), }, incompatible_use_toolchain_transition = True, - doc = """\ -Executes rustfmt in `--check` mode on the specified target. - -To enable this aspect for your workspace, simply add the following to the `.bazelrc` -file in the root of any workspace which loads `rules_rust`. - -``` -build --aspects=@rules_rust//rust:defs.bzl%rustfmt_check_aspect -build --output_groups=+rustfmt_check -``` - -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. -""", ) def _rustfmt_check_impl(ctx): - files = depset([], transitive = [target[OutputGroupInfo].rustfmt_check for target in ctx.attr.targets]) + files = depset([], transitive = [target[OutputGroupInfo].rustfmt_checks for target in ctx.attr.targets]) return [DefaultInfo(files = files)] rustfmt_check = rule( @@ -152,13 +138,13 @@ rustfmt_check = rule( "targets": attr.label_list( doc = "Rust targets to run rustfmt on.", providers = [rust_common.crate_info], - aspects = [rustfmt_check_aspect], + aspects = [rustfmt_aspect], ), }, doc = """\ A rule for defining a target which runs `rustfmt` in `--check` mode on an explicit list of targets -For more information on the use of `rustfmt` directly, see [rustfmt_check_aspect](#rustfmt_check_aspect). +For more information on the use of `rustfmt` directly, see [rustfmt_aspect](#rustfmt_aspect). """, ) From e35b83814a23330c29c65f71a37ce00316aca4aa Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 13 May 2021 10:51:02 -0700 Subject: [PATCH 10/20] Regenerate documentation --- docs/flatten.md | 46 ++++++++++++---------------------------------- docs/rust_fmt.md | 46 ++++++++++++---------------------------------- 2 files changed, 24 insertions(+), 68 deletions(-) diff --git a/docs/flatten.md b/docs/flatten.md index 1e3fceab8b..bfae451fcf 100644 --- a/docs/flatten.md +++ b/docs/flatten.md @@ -36,7 +36,6 @@ * [rustfmt](#rustfmt) * [rustfmt_aspect](#rustfmt_aspect) * [rustfmt_check](#rustfmt_check) -* [rustfmt_check_aspect](#rustfmt_check_aspect) @@ -1295,7 +1294,7 @@ rustfmt_check(name, name) -This aspect is used to gather information about a crate for use in rustfmt +This aspect is used to gather information about a crate for use in rustfmt and perform rustfmt checks -This aspect is used directly by [rustfmt](#rustfmt) targets to determine the -appropriate flags to use when formatting Rust sources. For more details on how -to format source code, see the [rustfmt](#rustfmt) rule. +Output Groups: +- `rustfmt_manifest`: The `rustfmt_manifest` output is used directly by [rustfmt](#rustfmt) targets +to determine the appropriate flags to use when formatting Rust sources. For more details on how to +format source code, see the [rustfmt](#rustfmt) rule. -**ASPECT ATTRIBUTES** - - - -**ATTRIBUTES** - - -| Name | Description | Type | Mandatory | Default | -| :------------- | :------------- | :------------- | :------------- | :------------- | -| name | A unique name for this target. | Name | required | | - - - - -## rustfmt_check_aspect - -
-rustfmt_check_aspect(name)
-
- -Executes rustfmt in `--check` mode on the specified target. - -To enable this aspect for your workspace, simply add the following to the `.bazelrc` -file in the root of any workspace which loads `rules_rust`. - +- `rustfmt_checks`: Executes rustfmt in `--check` mode on the specified target. To enable this check +for your workspace, simply add the following to the `.bazelrc` file in the root of any workspace +which loads `rules_rust`: ``` -build --aspects=@rules_rust//rust:defs.bzl%rustfmt_check_aspect -build --output_groups=+rustfmt_check +build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect +build --output_groups=+rustfmt_checks ``` This aspect is executed on any target which provides the `CrateInfo` provider. However @@ -1910,6 +1888,6 @@ source files are also ignored by this aspect. | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | -| name | A unique name for this target. | Name | required | | +| name | A unique name for this target. | Name | required | | diff --git a/docs/rust_fmt.md b/docs/rust_fmt.md index 41406422c7..0c7dda0e6e 100644 --- a/docs/rust_fmt.md +++ b/docs/rust_fmt.md @@ -3,7 +3,6 @@ * [rustfmt](#rustfmt) * [rustfmt_aspect](#rustfmt_aspect) * [rustfmt_check](#rustfmt_check) -* [rustfmt_check_aspect](#rustfmt_check_aspect) @@ -15,7 +14,7 @@ rustfmt_check(name, name) -This aspect is used to gather information about a crate for use in rustfmt +This aspect is used to gather information about a crate for use in rustfmt and perform rustfmt checks -This aspect is used directly by [rustfmt](#rustfmt) targets to determine the -appropriate flags to use when formatting Rust sources. For more details on how -to format source code, see the [rustfmt](#rustfmt) rule. +Output Groups: +- `rustfmt_manifest`: The `rustfmt_manifest` output is used directly by [rustfmt](#rustfmt) targets +to determine the appropriate flags to use when formatting Rust sources. For more details on how to +format source code, see the [rustfmt](#rustfmt) rule. -**ASPECT ATTRIBUTES** - - - -**ATTRIBUTES** - - -| Name | Description | Type | Mandatory | Default | -| :------------- | :------------- | :------------- | :------------- | :------------- | -| name | A unique name for this target. | Name | required | | - - - - -## rustfmt_check_aspect - -
-rustfmt_check_aspect(name)
-
- -Executes rustfmt in `--check` mode on the specified target. - -To enable this aspect for your workspace, simply add the following to the `.bazelrc` -file in the root of any workspace which loads `rules_rust`. - +- `rustfmt_checks`: Executes rustfmt in `--check` mode on the specified target. To enable this check +for your workspace, simply add the following to the `.bazelrc` file in the root of any workspace +which loads `rules_rust`: ``` -build --aspects=@rules_rust//rust:defs.bzl%rustfmt_check_aspect -build --output_groups=+rustfmt_check +build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect +build --output_groups=+rustfmt_checks ``` This aspect is executed on any target which provides the `CrateInfo` provider. However @@ -154,6 +132,6 @@ source files are also ignored by this aspect. | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | -| name | A unique name for this target. | Name | required | | +| name | A unique name for this target. | Name | required | | From 9cbd93765b02cfd5ef09803e6a32019eb8c6c70b Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 13 May 2021 11:56:13 -0700 Subject: [PATCH 11/20] Label fields should be public so they're usable outside of the crate. --- util/label/label.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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; From e7510100bd7b5ff8a90f2618f71cc7f59431f9db Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 13 May 2021 11:56:42 -0700 Subject: [PATCH 12/20] The `rustfmt` rule binaries can now take a list of targets --- rust/private/rustfmt.bzl | 13 +++++++++---- tools/rustfmt/srcs/main.rs | 40 +++++++++++++++++++++++++------------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/rust/private/rustfmt.bzl b/rust/private/rustfmt.bzl index 1483aca6f0..5618a89bc2 100644 --- a/rust/private/rustfmt.bzl +++ b/rust/private/rustfmt.bzl @@ -205,14 +205,19 @@ def rustfmt(name, config = Label("//tools/rustfmt:rustfmt.toml")): """ rust_binary( name = name, + data = [ + config, + Label("//tools/rustfmt:rustfmt_bin"), + ], + deps = [ + Label("//util/label"), + ], rustc_env = { "RUSTFMT": "$(rootpath {})".format(Label("//tools/rustfmt:rustfmt_bin")), "RUSTFMT_CONFIG": "$(rootpath {})".format(config), }, - data = [ - config, - Label("//tools/rustfmt:rustfmt_bin"), + srcs = [ + Label("//tools/rustfmt:srcs"), ], - srcs = [Label("//tools/rustfmt:srcs")], edition = "2018", ) diff --git a/tools/rustfmt/srcs/main.rs b/tools/rustfmt/srcs/main.rs index ddd3504506..35a73048b2 100644 --- a/tools/rustfmt/srcs/main.rs +++ b/tools/rustfmt/srcs/main.rs @@ -5,6 +5,8 @@ use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::str; +use label; + fn main() { // Gather all command line and environment settings let options = parse_args(); @@ -22,20 +24,32 @@ const SUPPORTED_RULES: &str = /// Perform a `bazel` query to determine a list of Bazel targets which are to be formatted. fn query_rustfmt_targets(options: &Config) -> Vec { - let scope = match &options.package { - Some(target) => { - if !target.ends_with(":all") && !target.ends_with("...") { - return vec![target.to_string()]; + // 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(); } - target + + options.packages.join(" + ") } - None => "//...:all", }; let query_args = vec![ "query".to_owned(), format!( - r#"kind('{types}', {scope}) except attr(tags, 'norustfmt|manual', kind('{types}', {scope}))"#, + r#"kind('{types}', {scope}) except attr(tags, 'norustfmt', kind('{types}', {scope}))"#, types = SUPPORTED_RULES, scope = scope ), @@ -71,7 +85,7 @@ fn build_rustfmt_targets(options: &Config, targets: &Vec) { let build_args = vec![ "build", "--aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect", - "--output_groups=+rustfmt_manifest", + "--output_groups=rustfmt_manifest", ]; let child = Command::new(&options.bazel) @@ -159,10 +173,10 @@ struct Config { /// https://rust-lang.github.io/rustfmt/ pub config: PathBuf, - /// An optional command line flag used to control what targets - /// to format. Users are expected to either pass a Bazel label - /// or a package pattern (`//my/package/...`). - pub package: Option, + /// Optionally, users can pass a list of targets/packages/scopes + /// (eg `//my:target` or `//my/pkg/...`) to control the targets + /// to be formatted. + pub packages: Vec, } /// Parse command line arguments and environment variables to @@ -181,7 +195,7 @@ fn parse_args() -> Config { .expect("Unable to find rustfmt binary"), config: absolutify_existing(&env!("RUSTFMT_CONFIG")) .expect("Unable to find rustfmt config file"), - package: env::args().nth(1), + packages: env::args().skip(1).collect(), } } From 8c070f2afddf227a35a6f70a056f576bb1d3a733 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 13 May 2021 17:11:16 -0700 Subject: [PATCH 13/20] Fixed missing configs --- tools/rustfmt/srcs/main.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/rustfmt/srcs/main.rs b/tools/rustfmt/srcs/main.rs index 35a73048b2..e564d05525 100644 --- a/tools/rustfmt/srcs/main.rs +++ b/tools/rustfmt/srcs/main.rs @@ -135,6 +135,8 @@ fn apply_rustfmt(options: &Config, targets: &Vec) { .current_dir(&options.workspace) .arg("--edition") .arg(rustfmt_config.edition) + .arg("--config-path") + .arg(&options.config) .args(rustfmt_config.sources) .status() .expect("Failed to run rustfmt"); From b69e30fcc48bf4b62975d0da5ed6bea6cdf87265 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 13 May 2021 20:40:35 -0700 Subject: [PATCH 14/20] Added a test rule and used a build setting to control the rustfmt config --- BUILD.bazel | 7 ++ docs/BUILD.bazel | 4 +- docs/rust_fmt.vm | 41 +++++++ docs/symbols.bzl | 6 +- rust/defs.bzl | 8 +- rust/private/rustfmt.bzl | 160 +++++++++----------------- test/rustfmt/BUILD.bazel | 28 +++-- test/rustfmt/rustfmt_failure_test.sh | 38 +++--- test/rustfmt/srcs/2018/unformatted.rs | 5 +- test/rustfmt/test_rustfmt.toml | 1 + tools/runfiles/runfiles.rs | 2 +- tools/rustfmt/BUILD.bazel | 46 ++++++-- tools/rustfmt/srcs/lib.rs | 72 ++++++++++++ tools/rustfmt/srcs/main.rs | 78 +++---------- tools/rustfmt/srcs/test_main.rs | 98 ++++++++++++++++ 15 files changed, 366 insertions(+), 228 deletions(-) create mode 100644 docs/rust_fmt.vm create mode 100644 test/rustfmt/test_rustfmt.toml create mode 100644 tools/rustfmt/srcs/lib.rs create mode 100644 tools/rustfmt/srcs/test_main.rs 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 d9b87088be..acb3adedfb 100644 --- a/docs/BUILD.bazel +++ b/docs/BUILD.bazel @@ -90,10 +90,10 @@ PAGES = dict([ ), page( name = "rust_fmt", + header_template = ":rust_fmt.vm", symbols = [ - "rustfmt", "rustfmt_aspect", - "rustfmt_check", + "rustfmt_test", ], ), page( 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 a74c75095f..fc9a7a2174 100644 --- a/docs/symbols.bzl +++ b/docs/symbols.bzl @@ -50,9 +50,8 @@ load( _rust_static_library = "rust_static_library", _rust_test = "rust_test", _rust_test_suite = "rust_test_suite", - _rustfmt = "rustfmt", _rustfmt_aspect = "rustfmt_aspect", - _rustfmt_check = "rustfmt_check", + _rustfmt_test = "rustfmt_test", ) load( "@rules_rust//rust:repositories.bzl", @@ -117,6 +116,5 @@ rust_analyzer_aspect = _rust_analyzer_aspect crate_universe = _crate_universe crate = _crate -rustfmt = _rustfmt rustfmt_aspect = _rustfmt_aspect -rustfmt_check = _rustfmt_check +rustfmt_test = _rustfmt_test diff --git a/rust/defs.bzl b/rust/defs.bzl index dbfed285e2..112086d157 100644 --- a/rust/defs.bzl +++ b/rust/defs.bzl @@ -51,9 +51,8 @@ load( ) load( "//rust/private:rustfmt.bzl", - _rustfmt = "rustfmt", _rustfmt_aspect = "rustfmt_aspect", - _rustfmt_check = "rustfmt_check", + _rustfmt_test = "rustfmt_test", ) rust_library = _rust_library @@ -107,11 +106,8 @@ rust_analyzer_aspect = _rust_analyzer_aspect rust_analyzer = _rust_analyzer # See @rules_rust//rust/private:rust_analyzer.bzl for a complete description. -rustfmt = _rustfmt -# See @rules_rust//rust/private:rustfmt.bzl for a complete description. - rustfmt_aspect = _rustfmt_aspect # See @rules_rust//rust/private:rustfmt.bzl for a complete description. -rustfmt_check = _rustfmt_check +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 index 5618a89bc2..c5a8db3d50 100644 --- a/rust/private/rustfmt.bzl +++ b/rust/private/rustfmt.bzl @@ -1,15 +1,14 @@ """A module defining rustfmt rules""" load(":common.bzl", "rust_common") -load(":rust.bzl", "rust_binary") load(":utils.bzl", "find_toolchain") -def _find_rustfmtable_srcs(target, ctx): +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. - ctx (ctx): The aspect's context object. + aspect_ctx (ctx, optional): The aspect's context object. Returns: list: A list of formattable sources (`File`). @@ -18,7 +17,7 @@ def _find_rustfmtable_srcs(target, ctx): return [] # Targets annotated with `norustfmt` will not be formatted - if "norustfmt" in ctx.rule.attr.tags: + if aspect_ctx and "norustfmt" in aspect_ctx.rule.attr.tags: return [] crate_info = target[rust_common.crate_info] @@ -96,27 +95,18 @@ This aspect is used to gather information about a crate for use in rustfmt and p Output Groups: -- `rustfmt_manifest`: The `rustfmt_manifest` output is used directly by [rustfmt](#rustfmt) targets -to determine the appropriate flags to use when formatting Rust sources. For more details on how to -format source code, see the [rustfmt](#rustfmt) rule. +- `rustfmt_manifest`: A manifest used by rustfmt binaries to provide crate specific settings. +- `rustfmt_checks`: Executes `rustfmt --check` on the specified target. -- `rustfmt_checks`: Executes rustfmt in `--check` mode on the specified target. To enable this check -for your workspace, simply add the following to the `.bazelrc` file in the root of any workspace -which loads `rules_rust`: -``` -build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect -build --output_groups=+rustfmt_checks -``` +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. """, - fragments = ["cpp"], - host_fragments = ["cpp"], - toolchains = [ - str(Label("//rust:toolchain")), - ], attrs = { "_process_wrapper": attr.label( doc = "A process wrapper for running rustfmt on all platforms", @@ -126,98 +116,60 @@ source files are also ignored by this aspect. ), }, incompatible_use_toolchain_transition = True, + fragments = ["cpp"], + host_fragments = ["cpp"], + toolchains = [ + str(Label("//rust:toolchain")), + ], ) -def _rustfmt_check_impl(ctx): - files = depset([], transitive = [target[OutputGroupInfo].rustfmt_checks for target in ctx.attr.targets]) - return [DefaultInfo(files = files)] +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_check = rule( - implementation = _rustfmt_check_impl, +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 on.", + 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"), + ), }, - doc = """\ -A rule for defining a target which runs `rustfmt` in `--check` mode on an explicit list of targets - -For more information on the use of `rustfmt` directly, see [rustfmt_aspect](#rustfmt_aspect). -""", + test = True, ) - -def rustfmt(name, config = Label("//tools/rustfmt:rustfmt.toml")): - """A macro defining a [rustfmt](https://github.com/rust-lang/rustfmt#readme) runner. - - This macro is used to generate a rustfmt binary which can be run to format the Rust source - files of `rules_rust` targets in the workspace. To define this target, simply load and call - it in a BUILD file. - - eg: `//:BUILD.bazel` - - ```python - load("@rules_rust//rust:defs.bzl", "rustfmt") - - rustfmt( - name = "rustfmt", - ) - ``` - - This now allows users to run `bazel run //:rustfmt` to format any target which provides `CrateInfo`. - - This binary also supports accepts a [label](https://docs.bazel.build/versions/master/build-ref.html#labels) or - pattern (`//my/package/...`) to allow for more granular control over what targets get formatted. This - can be useful when dealing with larger projects as `rustfmt` can only be run on a target which successfully - builds. Given the following workspace layout: - - ``` - WORKSPACE.bazel - BUILD.bazel - package_a/ - BUILD.bazel - src/ - lib.rs - mod_a.rs - mod_b.rs - package_b/ - BUILD.bazel - subpackage_1/ - BUILD.bazel - main.rs - subpackage_2/ - BUILD.bazel - main.rs - ``` - - Users can choose to only format the `rust_lib` target in `package_a` using `bazel run //:rustfmt -- //package_a:rust_lib`. - Additionally, users can format all of `package_b` using `bazel run //:rustfmt -- //package_b/...`. - - Users not looking to add a custom `rustfmt` config can simply run the `@rules_rust//tools/rustfmt` to avoid defining their - own target. - - Note that generated sources will be ignored and targets tagged as `norustfmt` will be skipped. - - Args: - name (str): The name of the rustfmt runner - config (Label, optional): The [rustfmt config](https://rust-lang.github.io/rustfmt/) to use. - """ - rust_binary( - name = name, - data = [ - config, - Label("//tools/rustfmt:rustfmt_bin"), - ], - deps = [ - Label("//util/label"), - ], - rustc_env = { - "RUSTFMT": "$(rootpath {})".format(Label("//tools/rustfmt:rustfmt_bin")), - "RUSTFMT_CONFIG": "$(rootpath {})".format(config), - }, - srcs = [ - Label("//tools/rustfmt:srcs"), - ], - edition = "2018", - ) diff --git a/test/rustfmt/BUILD.bazel b/test/rustfmt/BUILD.bazel index 5eaf4a197f..5ab4876efd 100644 --- a/test/rustfmt/BUILD.bazel +++ b/test/rustfmt/BUILD.bazel @@ -1,4 +1,8 @@ -load("@rules_rust//rust:defs.bzl", "rust_binary", "rustfmt_check") +load("@rules_rust//rust:defs.bzl", "rust_binary", "rustfmt_test") + +exports_files([ + "test_rustfmt.toml", +]) rust_binary( name = "formatted_2018", @@ -6,10 +10,8 @@ rust_binary( edition = "2018", ) -rustfmt_check( - name = "check_formatted_2018", - testonly = True, - tags = ["manual"], +rustfmt_test( + name = "test_formatted_2018", targets = [":formatted_2018"], ) @@ -19,9 +21,8 @@ rust_binary( edition = "2018", ) -rustfmt_check( - name = "check_unformatted_2018", - testonly = True, +rustfmt_test( + name = "test_unformatted_2018", tags = ["manual"], targets = [":unformatted_2018"], ) @@ -32,10 +33,8 @@ rust_binary( edition = "2015", ) -rustfmt_check( - name = "check_formatted_2015", - testonly = True, - tags = ["manual"], +rustfmt_test( + name = "test_formatted_2015", targets = [":formatted_2015"], ) @@ -45,9 +44,8 @@ rust_binary( edition = "2015", ) -rustfmt_check( - name = "check_unformatted_2015", - testonly = True, +rustfmt_test( + name = "test_unformatted_2015", tags = ["manual"], targets = [":unformatted_2015"], ) diff --git a/test/rustfmt/rustfmt_failure_test.sh b/test/rustfmt/rustfmt_failure_test.sh index 21619edfec..0a658e8efb 100755 --- a/test/rustfmt/rustfmt_failure_test.sh +++ b/test/rustfmt/rustfmt_failure_test.sh @@ -23,10 +23,10 @@ cd "${BUILD_WORKSPACE_DIRECTORY}" function check_build_result() { local ret=0 echo -n "Testing ${2}... " - (bazel build //test/rustfmt:"${2}" &> /dev/null) || ret="$?" && true + (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 build //test/rustfmt:${2}\" to see the output" + echo " Run \"bazel test //test/rustfmt:${2}\" to see the output" exit 1 else echo "OK" @@ -34,18 +34,18 @@ function check_build_result() { } function test_all() { - local -r BUILD_OK=0 - local -r BUILD_FAILED=1 + local -r TEST_OK=0 + local -r TEST_FAILED=3 - check_build_result $BUILD_FAILED check_unformatted_2015 - check_build_result $BUILD_FAILED check_unformatted_2018 - check_build_result $BUILD_OK check_formatted_2015 - check_build_result $BUILD_OK check_formatted_2018 + 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 BUILD_OK=0 - local -r BUILD_FAILED=1 + 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" @@ -67,18 +67,18 @@ EOF # Format a specific target bazel run @rules_rust//tools/rustfmt -- //test/rustfmt:unformatted_2018 - check_build_result $BUILD_FAILED check_unformatted_2015 - check_build_result $BUILD_OK check_unformatted_2018 - check_build_result $BUILD_OK check_formatted_2015 - check_build_result $BUILD_OK check_formatted_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 + bazel run @rules_rust//tools/rustfmt --@rules_rust//:rustfmt.toml=//test/rustfmt:test_rustfmt.toml - check_build_result $BUILD_OK check_unformatted_2015 - check_build_result $BUILD_OK check_unformatted_2018 - check_build_result $BUILD_OK check_formatted_2015 - check_build_result $BUILD_OK check_formatted_2018 + 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 diff --git a/test/rustfmt/srcs/2018/unformatted.rs b/test/rustfmt/srcs/2018/unformatted.rs index 2fb232080a..454ac49255 100644 --- a/test/rustfmt/srcs/2018/unformatted.rs +++ b/test/rustfmt/srcs/2018/unformatted.rs @@ -1,7 +1,4 @@ -use std::future::Future; -use std::sync::Arc; -use std::task::{Context, Poll, Wake}; -use std::thread::{self, Thread}; +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();}} 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/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 index e8809cf769..2394456d11 100644 --- a/tools/rustfmt/BUILD.bazel +++ b/tools/rustfmt/BUILD.bazel @@ -1,4 +1,4 @@ -load("//rust:defs.bzl", "rustfmt", "rustfmt_check") +load("//rust:defs.bzl", "rust_binary", "rust_library") package(default_visibility = ["//visibility:public"]) @@ -15,18 +15,46 @@ alias( }), ) -filegroup( - name = "srcs", - srcs = glob(["srcs/**/*.rs"]), +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)", + }, ) -rustfmt( +rust_binary( name = "rustfmt", + srcs = [ + "srcs/main.rs", + ], + data = [ + "//:rustfmt.toml", + ], + edition = "2018", + deps = [ + ":rustfmt_lib", + "//util/label", + ], ) -rustfmt_check( - name = "check", - targets = [ - "rustfmt", +rust_binary( + name = "rustfmt_test", + srcs = [ + "srcs/test_main.rs", + ], + edition = "2018", + deps = [ + ":rustfmt_lib", + "//tools/runfiles", ], ) diff --git a/tools/rustfmt/srcs/lib.rs b/tools/rustfmt/srcs/lib.rs new file mode 100644 index 0000000000..0114692a4c --- /dev/null +++ b/tools/rustfmt/srcs/lib.rs @@ -0,0 +1,72 @@ +use std::collections::VecDeque; +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: VecDeque = content + .split("\n") + .into_iter() + .filter(|s| !s.is_empty()) + .map(|s| s.to_owned()) + .collect(); + + RustfmtManifest { + edition: lines + .pop_front() + .expect("There should always be at least 1 line in the manifest"), + sources: lines.into(), + } +} diff --git a/tools/rustfmt/srcs/main.rs b/tools/rustfmt/srcs/main.rs index e564d05525..2fbeef461f 100644 --- a/tools/rustfmt/srcs/main.rs +++ b/tools/rustfmt/srcs/main.rs @@ -1,11 +1,10 @@ -use std::collections::VecDeque; use std::env; -use std::fs; -use std::path::{Path, PathBuf}; +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 @@ -19,7 +18,7 @@ fn main() { } /// A set of supported rules for use in a `bazel query` when querying for Rust targets. -const SUPPORTED_RULES: &str = +const SUPPORTED_RULES: &'static str = "rust_library|rust_proc_macro|rust_shared_library|rust_static_library|rust_binary|rust_test"; /// Perform a `bazel` query to determine a list of Bazel targets which are to be formatted. @@ -117,11 +116,12 @@ fn apply_rustfmt(options: &Config, targets: &Vec) { let target_path = target.replace(":", "/").trim_start_matches("/").to_owned(); // Load the manifest containing rustfmt arguments - let rustfmt_config = parse_rustfmt_manifest( - &options - .workspace - .join("bazel-bin") - .join(format!("{}.rustfmt", &target_path)), + let rustfmt_config = rustfmt_lib::parse_rustfmt_manifest( + &options.workspace.join("bazel-bin").join(format!( + "{}.{}", + &target_path, + rustfmt_lib::RUSTFMT_MANIFEST_EXTENSION, + )), ); // Ignore any targets which do not have source files. This can @@ -131,12 +131,12 @@ fn apply_rustfmt(options: &Config, targets: &Vec) { } // Run rustfmt - let status = Command::new(&options.rustfmt) + let status = Command::new(&options.rustfmt_config.rustfmt) .current_dir(&options.workspace) .arg("--edition") .arg(rustfmt_config.edition) .arg("--config-path") - .arg(&options.config) + .arg(&options.rustfmt_config.config) .args(rustfmt_config.sources) .status() .expect("Failed to run rustfmt"); @@ -147,18 +147,6 @@ fn apply_rustfmt(options: &Config, targets: &Vec) { } } -/// 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)] struct Config { @@ -168,12 +156,8 @@ struct Config { /// The Bazel executable to use for builds and queries. pub bazel: PathBuf, - /// 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, + /// 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 @@ -193,41 +177,7 @@ fn parse_args() -> Config { env::var("BAZEL_REAL") .unwrap_or_else(|_| "bazel".to_owned()) ), - rustfmt: absolutify_existing(&env!("RUSTFMT")) - .expect("Unable to find rustfmt binary"), - config: absolutify_existing(&env!("RUSTFMT_CONFIG")) - .expect("Unable to find rustfmt config file"), + rustfmt_config: rustfmt_lib::parse_rustfmt_config(), packages: env::args().skip(1).collect(), } } - -/// Parse rustfmt flags from a manifest generated by builds using `rustfmt_aspect`. -fn parse_rustfmt_manifest(manifest: &Path) -> RustfmtConfig { - let content = fs::read_to_string(manifest).expect(&format!( - "Failed to read rustfmt manifest: {}", - manifest.display() - )); - - let mut lines: VecDeque = content - .split("\n") - .into_iter() - .map(|s| s.to_owned()) - .collect(); - - RustfmtConfig { - edition: lines - .pop_front() - .expect("There should always be at least 1 line in the manifest"), - sources: lines.into(), - } -} - -/// A struct of target specific information for use in running `rustfmt`. -#[derive(Debug)] -struct RustfmtConfig { - /// The Rust edition of the Bazel target - pub edition: String, - - /// A list of all (non-generated) source files for formatting. - pub sources: Vec, -} 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(), + } +} From 0dfb3e272c2b11fa21540a1a8507712f5a3f33f7 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Thu, 13 May 2021 20:42:45 -0700 Subject: [PATCH 15/20] Regenerate documentation --- docs/flatten.md | 102 +++++---------------------------------- docs/rust_fmt.md | 122 +++++++++++++++++------------------------------ 2 files changed, 57 insertions(+), 167 deletions(-) diff --git a/docs/flatten.md b/docs/flatten.md index bfae451fcf..90075793e2 100644 --- a/docs/flatten.md +++ b/docs/flatten.md @@ -33,9 +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](#rustfmt) * [rustfmt_aspect](#rustfmt_aspect) -* [rustfmt_check](#rustfmt_check) +* [rustfmt_test](#rustfmt_test) @@ -1284,26 +1283,23 @@ For additional information, see the [Bazel toolchains documentation][toolchains] | bindgen | The label of a wasm-bindgen-cli executable. | Label | optional | None | - + -## rustfmt_check +## rustfmt_test 
-rustfmt_check(name, targets)
+rustfmt_test(name, targets)
-A rule for defining a target which runs `rustfmt` in `--check` mode on an explicit list of targets - -For more information on the use of `rustfmt` directly, see [rustfmt_aspect](#rustfmt_aspect). - +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 on. | List of labels | optional | [] | +| name | A unique name for this target. | Name | required | | +| targets | Rust targets to run rustfmt --check on. | List of labels | optional | [] | @@ -1707,74 +1703,6 @@ Declare dependencies needed for [rust_wasm_bindgen](#rust_wasm_bindgen). | register_default_toolchain | If True, the default [rust_wasm_bindgen_toolchain](#rust_wasm_bindgen_toolchain) (@rules_rust//wasm_bindgen:default_wasm_bindgen_toolchain) is registered. This toolchain requires a set of dependencies that were generated using [cargo raze](https://github.com/google/cargo-raze). These will also be loaded. | True | - - -## rustfmt - -
-rustfmt(name, config)
-
- -A macro defining a [rustfmt](https://github.com/rust-lang/rustfmt#readme) runner. - -This macro is used to generate a rustfmt binary which can be run to format the Rust source -files of `rules_rust` targets in the workspace. To define this target, simply load and call -it in a BUILD file. - -eg: `//:BUILD.bazel` - -```python -load("@rules_rust//rust:defs.bzl", "rustfmt") - -rustfmt( - name = "rustfmt", -) -``` - -This now allows users to run `bazel run //:rustfmt` to format any target which provides `CrateInfo`. - -This binary also supports accepts a [label](https://docs.bazel.build/versions/master/build-ref.html#labels) or -pattern (`//my/package/...`) to allow for more granular control over what targets get formatted. This -can be useful when dealing with larger projects as `rustfmt` can only be run on a target which successfully -builds. Given the following workspace layout: - -``` -WORKSPACE.bazel -BUILD.bazel -package_a/ - BUILD.bazel - src/ - lib.rs - mod_a.rs - mod_b.rs -package_b/ - BUILD.bazel - subpackage_1/ - BUILD.bazel - main.rs - subpackage_2/ - BUILD.bazel - main.rs -``` - -Users can choose to only format the `rust_lib` target in `package_a` using `bazel run //:rustfmt -- //package_a:rust_lib`. -Additionally, users can format all of `package_b` using `bazel run //:rustfmt -- //package_b/...`. - -Users not looking to add a custom `rustfmt` config can simply run the `@rules_rust//tools/rustfmt` to avoid defining their -own target. - -Note that generated sources will be ignored and targets tagged as `norustfmt` will be skipped. - - -**PARAMETERS** - - -| Name | Description | Default Value | -| :------------- | :------------- | :------------- | -| name | The name of the rustfmt runner | none | -| config | The [rustfmt config](https://rust-lang.github.io/rustfmt/) to use. | Label("//tools/rustfmt:rustfmt.toml") | - - ## rust_analyzer_aspect @@ -1862,17 +1790,13 @@ This aspect is used to gather information about a crate for use in rustfmt and p Output Groups: -- `rustfmt_manifest`: The `rustfmt_manifest` output is used directly by [rustfmt](#rustfmt) targets -to determine the appropriate flags to use when formatting Rust sources. For more details on how to -format source code, see the [rustfmt](#rustfmt) rule. +- `rustfmt_manifest`: A manifest used by rustfmt binaries to provide crate specific settings. +- `rustfmt_checks`: Executes `rustfmt --check` on the specified target. -- `rustfmt_checks`: Executes rustfmt in `--check` mode on the specified target. To enable this check -for your workspace, simply add the following to the `.bazelrc` file in the root of any workspace -which loads `rules_rust`: -``` -build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect -build --output_groups=+rustfmt_checks -``` +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 diff --git a/docs/rust_fmt.md b/docs/rust_fmt.md index 0c7dda0e6e..82b6b7d476 100644 --- a/docs/rust_fmt.md +++ b/docs/rust_fmt.md @@ -1,97 +1,67 @@ # Rust Fmt -* [rustfmt](#rustfmt) * [rustfmt_aspect](#rustfmt_aspect) -* [rustfmt_check](#rustfmt_check) +* [rustfmt_test](#rustfmt_test) - -## rustfmt_check +## Overview -
-rustfmt_check(name, targets)
-
- -A rule for defining a target which runs `rustfmt` in `--check` mode on an explicit list of targets - -For more information on the use of `rustfmt` directly, see [rustfmt_aspect](#rustfmt_aspect). - - -**ATTRIBUTES** +[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]. -| Name | Description | Type | Mandatory | Default | -| :------------- | :------------- | :------------- | :------------- | :------------- | -| name | A unique name for this target. | Name | required | | -| targets | Rust targets to run rustfmt on. | List of labels | optional | [] | - - - -## rustfmt - -
-rustfmt(name, config)
-
-A macro defining a [rustfmt](https://github.com/rust-lang/rustfmt#readme) runner. +### Setup -This macro is used to generate a rustfmt binary which can be run to format the Rust source -files of `rules_rust` targets in the workspace. To define this target, simply load and call -it in a BUILD file. -eg: `//:BUILD.bazel` +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. -```python -load("@rules_rust//rust:defs.bzl", "rustfmt") +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. -rustfmt( - name = "rustfmt", -) +```text +build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect +build --output_groups=+rustfmt_checks ``` -This now allows users to run `bazel run //:rustfmt` to format any target which provides `CrateInfo`. +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. -This binary also supports accepts a [label](https://docs.bazel.build/versions/master/build-ref.html#labels) or -pattern (`//my/package/...`) to allow for more granular control over what targets get formatted. This -can be useful when dealing with larger projects as `rustfmt` can only be run on a target which successfully -builds. Given the following workspace layout: +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: -``` -WORKSPACE.bazel -BUILD.bazel -package_a/ - BUILD.bazel - src/ - lib.rs - mod_a.rs - mod_b.rs -package_b/ - BUILD.bazel - subpackage_1/ - BUILD.bazel - main.rs - subpackage_2/ - BUILD.bazel - main.rs +```text +build --@rules_rust//:rustfmt.toml=//:rustfmt.toml ``` -Users can choose to only format the `rust_lib` target in `package_a` using `bazel run //:rustfmt -- //package_a:rust_lib`. -Additionally, users can format all of `package_b` using `bazel run //:rustfmt -- //package_b/...`. +[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/ -Users not looking to add a custom `rustfmt` config can simply run the `@rules_rust//tools/rustfmt` to avoid defining their -own target. + -Note that generated sources will be ignored and targets tagged as `norustfmt` will be skipped. +## rustfmt_test +
+rustfmt_test(name, targets)
+
+ +A test rule for performing `rustfmt --check` on a set of targets -**PARAMETERS** +**ATTRIBUTES** -| Name | Description | Default Value | -| :------------- | :------------- | :------------- | -| name | The name of the rustfmt runner | none | -| config | The [rustfmt config](https://rust-lang.github.io/rustfmt/) to use. | Label("//tools/rustfmt:rustfmt.toml") | +| 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 | [] | @@ -106,17 +76,13 @@ This aspect is used to gather information about a crate for use in rustfmt and p Output Groups: -- `rustfmt_manifest`: The `rustfmt_manifest` output is used directly by [rustfmt](#rustfmt) targets -to determine the appropriate flags to use when formatting Rust sources. For more details on how to -format source code, see the [rustfmt](#rustfmt) rule. +- `rustfmt_manifest`: A manifest used by rustfmt binaries to provide crate specific settings. +- `rustfmt_checks`: Executes `rustfmt --check` on the specified target. -- `rustfmt_checks`: Executes rustfmt in `--check` mode on the specified target. To enable this check -for your workspace, simply add the following to the `.bazelrc` file in the root of any workspace -which loads `rules_rust`: -``` -build --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect -build --output_groups=+rustfmt_checks -``` +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 From 67a1201eca07b9eb6bc16b20a6c64fdc2ad3469d Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Fri, 14 May 2021 06:40:13 -0700 Subject: [PATCH 16/20] Updated function name --- tools/rustfmt/srcs/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/rustfmt/srcs/main.rs b/tools/rustfmt/srcs/main.rs index 2fbeef461f..1119f9c5f5 100644 --- a/tools/rustfmt/srcs/main.rs +++ b/tools/rustfmt/srcs/main.rs @@ -80,7 +80,7 @@ fn query_rustfmt_targets(options: &Config) -> Vec { /// Build a list of Bazel targets using the `rustfmt_aspect` to produce the /// arguments to use when formatting the sources of those targets. -fn build_rustfmt_targets(options: &Config, targets: &Vec) { +fn generate_rustfmt_target_manifests(options: &Config, targets: &Vec) { let build_args = vec![ "build", "--aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect", @@ -109,7 +109,7 @@ fn build_rustfmt_targets(options: &Config, targets: &Vec) { 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. - build_rustfmt_targets(&options, &targets); + generate_rustfmt_target_manifests(&options, &targets); for target in targets.iter() { // Replace any `:` characters and strip leading slashes From 8ab6a7eb58fea37da2b83ad87afb827c4ad91db2 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Fri, 14 May 2021 06:41:00 -0700 Subject: [PATCH 17/20] Updated comment --- tools/rustfmt/srcs/main.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/rustfmt/srcs/main.rs b/tools/rustfmt/srcs/main.rs index 1119f9c5f5..2b8cf6fc56 100644 --- a/tools/rustfmt/srcs/main.rs +++ b/tools/rustfmt/srcs/main.rs @@ -161,7 +161,8 @@ struct Config { /// Optionally, users can pass a list of targets/packages/scopes /// (eg `//my:target` or `//my/pkg/...`) to control the targets - /// to be formatted. + /// to be formatted. If empty, all targets in the workspace will + /// be formatted. pub packages: Vec, } From e11b1ef06aa003f91267a46d64c2ffe7e5825a2a Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Fri, 14 May 2021 06:49:23 -0700 Subject: [PATCH 18/20] Updated rustmt query to be more broad --- tools/rustfmt/srcs/main.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/tools/rustfmt/srcs/main.rs b/tools/rustfmt/srcs/main.rs index 2b8cf6fc56..74fea5d158 100644 --- a/tools/rustfmt/srcs/main.rs +++ b/tools/rustfmt/srcs/main.rs @@ -17,10 +17,6 @@ fn main() { apply_rustfmt(&options, &targets); } -/// A set of supported rules for use in a `bazel query` when querying for Rust targets. -const SUPPORTED_RULES: &'static str = - "rust_library|rust_proc_macro|rust_shared_library|rust_static_library|rust_binary|rust_test"; - /// 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 @@ -49,7 +45,7 @@ fn query_rustfmt_targets(options: &Config) -> Vec { "query".to_owned(), format!( r#"kind('{types}', {scope}) except attr(tags, 'norustfmt', kind('{types}', {scope}))"#, - types = SUPPORTED_RULES, + types = "^rust_", scope = scope ), ]; @@ -115,14 +111,19 @@ fn apply_rustfmt(options: &Config, targets: &Vec) { // 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( - &options.workspace.join("bazel-bin").join(format!( - "{}.{}", - &target_path, - rustfmt_lib::RUSTFMT_MANIFEST_EXTENSION, - )), - ); + 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. From 47fe1aadbe0d0ed3323910340987859094cb0287 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Fri, 14 May 2021 06:55:39 -0700 Subject: [PATCH 19/20] Updated rustfmt manifest format --- rust/private/rustfmt.bzl | 4 ++-- tools/rustfmt/srcs/lib.rs | 16 ++++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/rust/private/rustfmt.bzl b/rust/private/rustfmt.bzl index c5a8db3d50..43051e5cc3 100644 --- a/rust/private/rustfmt.bzl +++ b/rust/private/rustfmt.bzl @@ -35,9 +35,9 @@ def _generate_manifest(edition, srcs, ctx): manifest = ctx.actions.declare_file(ctx.label.name + ".rustfmt") ctx.actions.write( output = manifest, - content = "\n".join([ + content = "\n".join(src_paths + [ edition, - ] + src_paths), + ]), ) return manifest diff --git a/tools/rustfmt/srcs/lib.rs b/tools/rustfmt/srcs/lib.rs index 0114692a4c..35ccf78a94 100644 --- a/tools/rustfmt/srcs/lib.rs +++ b/tools/rustfmt/srcs/lib.rs @@ -1,4 +1,3 @@ -use std::collections::VecDeque; use std::env; use std::fs; use std::path::{Path, PathBuf}; @@ -56,17 +55,22 @@ pub fn parse_rustfmt_manifest(manifest: &Path) -> RustfmtManifest { manifest.display() )); - let mut lines: VecDeque = content + 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: lines - .pop_front() - .expect("There should always be at least 1 line in the manifest"), - sources: lines.into(), + edition: edition, + sources: lines, } } From 8b802dd8372abfea4a8c61c5e3383cadf955222a Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 17 May 2021 10:04:43 -0700 Subject: [PATCH 20/20] Regenerate documentation --- docs/rust_fmt.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/rust_fmt.md b/docs/rust_fmt.md index 82b6b7d476..fed766ca0b 100644 --- a/docs/rust_fmt.md +++ b/docs/rust_fmt.md @@ -1,3 +1,4 @@ + # Rust Fmt * [rustfmt_aspect](#rustfmt_aspect)