diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel index e0dcce339c..3c21edb4ff 100644 --- a/docs/BUILD.bazel +++ b/docs/BUILD.bazel @@ -109,6 +109,8 @@ stardoc( symbol_names = [ "rust_clippy", "rust_clippy_aspect", + "rust_clippy_test", + "rust_clippy_test_aspect", ], table_of_contents_template = "@stardoc//stardoc:templates/markdown_tables/table_of_contents.vm", deps = [":all_docs"], @@ -134,6 +136,7 @@ stardoc( symbol_names = [ "rustfmt_aspect", "rustfmt_test", + "rustfmt_test_aspect", ], table_of_contents_template = "@stardoc//stardoc:templates/markdown_tables/table_of_contents.vm", deps = [":all_docs"], diff --git a/docs/rust_clippy.vm b/docs/rust_clippy.vm index 2576175099..b6e83933e9 100644 --- a/docs/rust_clippy.vm +++ b/docs/rust_clippy.vm @@ -30,3 +30,27 @@ the upstream implementation of clippy, this file must be named either `.clippy.t ```text build --@rules_rust//rust/settings:clippy.toml=//:clippy.toml ``` + +#[[ +### Running clippy over a tree of targets as a test +]]# + +`rust_clippy_test` is a `test = True` rule that runs clippy over the targets you list and +transitively across their `deps`, `proc_macro_deps`, and `crate`. The clippy actions run +during the build phase, so any clippy failure fails `bazel test` before the test executable +is invoked. The test itself simply prints the paths of the collected clippy marker files. + +This is the recommended way to enforce clippy in CI without wiring up `--aspects` / +`--output_groups` flags. + +```python +load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_clippy_test", "rust_library") + +rust_library(name = "lib", srcs = ["src/lib.rs"], edition = "2021") +rust_binary(name = "app", srcs = ["src/main.rs"], edition = "2021", deps = [":lib"]) + +rust_clippy_test( + name = "clippy_tree_test", + targets = [":app"], +) +``` diff --git a/docs/rust_fmt.vm b/docs/rust_fmt.vm index 5175352575..5457c00177 100644 --- a/docs/rust_fmt.vm +++ b/docs/rust_fmt.vm @@ -39,6 +39,37 @@ file is used whenever `rustfmt` is run: build --@rules_rust//rust/settings:rustfmt.toml=//:rustfmt.toml ``` +#[[ +### Running rustfmt over a tree of targets as a test +]]# + +`rustfmt_test` runs `rustfmt --check` over a set of Rust targets. By default it walks each +listed target's `deps`, `proc_macro_deps`, and `crate` transitively, so listing a top-level +target formats its whole crate graph. Set `transitive = False` to check only the exact targets +listed. An optional `platform` attribute transitions `targets` to the given platform. + +The rustfmt actions run during the build phase, so any formatting failure fails `bazel test` +before the test executable is invoked. The test itself simply prints the paths of the collected +`.rustfmt.ok` marker files. + +```python +load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library", "rustfmt_test") + +rust_library(name = "lib", srcs = ["src/lib.rs"], edition = "2021") +rust_binary(name = "app", srcs = ["src/main.rs"], edition = "2021", deps = [":lib"]) + +rustfmt_test( + name = "fmt_tree_test", + targets = [":app"], +) + +rustfmt_test( + name = "fmt_app_only_test", + targets = [":app"], + transitive = False, +) +``` + [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 diff --git a/rust/defs.bzl b/rust/defs.bzl index 4f2ef72582..d8fa8f21bd 100644 --- a/rust/defs.bzl +++ b/rust/defs.bzl @@ -27,6 +27,8 @@ load( _rust_clippy = "rust_clippy", _rust_clippy_action = "rust_clippy_action", _rust_clippy_aspect = "rust_clippy_aspect", + _rust_clippy_test = "rust_clippy_test", + _rust_clippy_test_aspect = "rust_clippy_test_aspect", ) load("//rust/private:common.bzl", _rust_common = "rust_common") load( @@ -71,6 +73,7 @@ load( "//rust/private:rustfmt.bzl", _rustfmt_aspect = "rustfmt_aspect", _rustfmt_test = "rustfmt_test", + _rustfmt_test_aspect = "rustfmt_test_aspect", ) load( "//rust/private:unpretty.bzl", @@ -118,6 +121,12 @@ rust_clippy_aspect = _rust_clippy_aspect rust_clippy = _rust_clippy # See @rules_rust//rust/private:clippy.bzl for a complete description. +rust_clippy_test_aspect = _rust_clippy_test_aspect +# See @rules_rust//rust/private:clippy.bzl for a complete description. + +rust_clippy_test = _rust_clippy_test +# See @rules_rust//rust/private:clippy.bzl for a complete description. + capture_clippy_output = _capture_clippy_output # See @rules_rust//rust/private:clippy.bzl for a complete description. @@ -166,6 +175,9 @@ rustfmt_aspect = _rustfmt_aspect rustfmt_test = _rustfmt_test # See @rules_rust//rust/private:rustfmt.bzl for a complete description. +rustfmt_test_aspect = _rustfmt_test_aspect +# See @rules_rust//rust/private:rustfmt.bzl for a complete description. + rust_stdlib_filegroup = _rust_stdlib_filegroup # See @rules_rust//rust:toolchain.bzl for a complete description. diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl index c4118eca6f..1757ca4863 100644 --- a/rust/private/clippy.bzl +++ b/rust/private/clippy.bzl @@ -16,6 +16,13 @@ load("@bazel_skylib//lib:structs.bzl", "structs") load("//rust/private:common.bzl", "rust_common") +load( + "//rust/private:lint_test.bzl", + "LINT_TEST_COMMON_ATTRS", + "lint_test_aspect_impl", + "lint_test_rule_impl", + "platform_transition", +) load( "//rust/private:providers.bzl", "CaptureClippyOutputInfo", @@ -452,6 +459,80 @@ rust_clippy( """, ) +RustClippyTestInfo = provider( + doc = "Clippy check outputs surfaced by `rust_clippy_test_aspect`.", + fields = { + "checks": "depset[File]: Clippy markers for the visited target plus every crate reached via `deps`, `proc_macro_deps`, and `crate`.", + "direct_markers": "list[File]: Clippy markers for the visited target only.", + }, +) + +# clippy contributes to two output groups: `clippy_checks` (`.clippy.ok` +# marker in the default config, `.clippy.out` when `capture_clippy_output` +# is on) and `clippy_output` (`.clippy.diagnostics` JSON when +# `clippy_output_diagnostics` is on). Capture modes make clippy exit 0 even +# on real issues, so the runner inspects file contents to decide pass/fail. +_CLIPPY_OUTPUT_GROUPS = ["clippy_checks", "clippy_output"] + +def _rust_clippy_test_aspect_impl(target, ctx): + return lint_test_aspect_impl(target, ctx, RustClippyTestInfo, _CLIPPY_OUTPUT_GROUPS) + +rust_clippy_test_aspect = aspect( + implementation = _rust_clippy_test_aspect_impl, + attr_aspects = ["deps", "proc_macro_deps", "crate"], + requires = [rust_clippy_aspect], + provides = [RustClippyTestInfo], + doc = "Walks `deps`/`proc_macro_deps`/`crate` and rolls up the markers produced by `rust_clippy_aspect` into a transitive `RustClippyTestInfo`.", +) + +def _rust_clippy_test_impl(ctx): + return lint_test_rule_impl(ctx, RustClippyTestInfo) + +rust_clippy_test = rule( + implementation = _rust_clippy_test_impl, + attrs = dict(LINT_TEST_COMMON_ATTRS, **{ + "targets": attr.label_list( + doc = "Rust targets to run clippy on.", + providers = [ + [rust_common.crate_info], + [rust_common.test_crate_info], + ], + aspects = [rust_clippy_test_aspect], + cfg = platform_transition, + ), + }), + test = True, + doc = """\ +A test rule that runs `clippy` over a set of Rust targets. + +By default (`transitive = True`), the aspect walks `deps`, `proc_macro_deps`, and `crate` +transitively so that listing a top-level target checks its whole crate graph. Set +`transitive = False` to run clippy only on the exact targets listed. + +The clippy actions run during the build phase, so a clippy failure fails `bazel test` before +the test executable is invoked. When `capture_clippy_output` or `clippy_output_diagnostics` is +set globally clippy exits 0 even on real issues; in that case the runner inspects the +captured stderr / JSON diagnostics and reports the verdict. + +An optional `platform` attribute transitions `targets` to the given platform before running +clippy. + +Example: + +```python +load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_clippy_test", "rust_library") + +rust_library(name = "lib", srcs = ["src/lib.rs"], edition = "2021") +rust_binary(name = "app", srcs = ["src/main.rs"], edition = "2021", deps = [":lib"]) + +rust_clippy_test(name = "clippy_tree_test", targets = [":app"]) +rust_clippy_test(name = "clippy_app_only_test", targets = [":app"], transitive = False) +``` + +Targets tagged `no_clippy`, `no_lint`, `nolint`, or `noclippy` are skipped. +""", +) + def _capture_clippy_output_impl(ctx): """Implementation of the `capture_clippy_output` rule diff --git a/rust/private/lint_test.bzl b/rust/private/lint_test.bzl new file mode 100644 index 0000000000..e5f62f40b8 --- /dev/null +++ b/rust/private/lint_test.bzl @@ -0,0 +1,143 @@ +"""Shared helpers for `rust_clippy_test` and `rustfmt_test`. + +Both rules follow the same shape: a thin wrapper aspect that walks +`deps`/`proc_macro_deps`/`crate` and collects the output-group markers +produced by the underlying real aspect (`rust_clippy_aspect` / +`rustfmt_aspect`), plus a rule impl that symlinks a shared runner binary +and hands it the collected marker rlocationpaths via `RUST_LINT_TEST_MARKERS`. + +The pieces exposed here — `rlocationpath`, `platform_transition`, +`LINT_TEST_COMMON_ATTRS`, `lint_test_aspect_impl`, `lint_test_rule_impl` — +let each rule file supply only what actually differs (the provider type +and the output-group names it collects). +""" + +def rlocationpath(file, workspace_name): + """Compute the runfile rlocationpath for a File.""" + if file.short_path.startswith("../"): + return file.short_path[len("../"):] + return "{}/{}".format(workspace_name, file.short_path) + +def _platform_transition_impl(_settings, attr): + if not attr.platform: + return {} + platform = str(attr.platform) + if not platform.startswith("@"): + platform = "@" + platform + return {"//command_line_option:platforms": platform} + +platform_transition = transition( + implementation = _platform_transition_impl, + inputs = [], + outputs = ["//command_line_option:platforms"], +) + +# Attrs every lint-test rule needs alongside its own `targets`. Callers +# merge this dict into their `attrs = {...}`. +LINT_TEST_COMMON_ATTRS = { + "platform": attr.label( + doc = "Optional platform to transition `targets` to before running the aspect. When set, `--platforms` is switched to this label for the duration of this rule's aspect actions.", + ), + "transitive": attr.bool( + doc = "If True (default), lint `targets` and every crate reachable via `deps`, `proc_macro_deps`, and `crate`. If False, lint only the exact targets listed.", + default = True, + ), + "_allowlist_function_transition": attr.label( + default = "@bazel_tools//tools/allowlists/function_transition_allowlist", + ), + "_runner": attr.label( + doc = "The shared runner (prints/inspects collected marker paths).", + cfg = "exec", + executable = True, + default = Label("//rust/private/lint_test_runner"), + ), +} + +def lint_test_aspect_impl(target, ctx, info_provider, output_group_names): + """Thin collector: walk deps and roll up the markers the underlying aspect produced. + + Args: + target: Aspect target. + ctx: Aspect ctx. + info_provider: Provider type to read from deps and return. + output_group_names: List[str] of `OutputGroupInfo` field names to + collect from the current target (e.g. `["clippy_checks", "clippy_output"]` + or `["rustfmt_checks"]`). + + Returns: + A single-element list with a `info_provider(direct_markers, checks)`. + """ + transitive = [] + for attr_name in ("deps", "proc_macro_deps"): + for dep in getattr(ctx.rule.attr, attr_name, []): + if info_provider in dep: + transitive.append(dep[info_provider].checks) + crate_dep = getattr(ctx.rule.attr, "crate", None) + if crate_dep and info_provider in crate_dep: + transitive.append(crate_dep[info_provider].checks) + + direct = [] + if OutputGroupInfo in target: + og = target[OutputGroupInfo] + for name in output_group_names: + if hasattr(og, name): + direct = direct + getattr(og, name).to_list() + + return [info_provider( + direct_markers = direct, + checks = depset(direct, transitive = transitive), + )] + +def lint_test_rule_impl(ctx, info_provider): + """Symlink the shared runner and hand it the collected marker rlocationpaths. + + Args: + ctx: Rule ctx. + info_provider: Provider carrying `direct_markers` / `checks`. + + Returns: + DefaultInfo + RunEnvironmentInfo for the test. + """ + is_windows = ctx.executable._runner.extension == ".exe" + runner = ctx.actions.declare_file("{}{}".format( + ctx.label.name, + ".exe" if is_windows else "", + )) + ctx.actions.symlink( + output = runner, + target_file = ctx.executable._runner, + is_executable = True, + ) + + marker_files = [] + check_depsets = [] + for target in ctx.attr.targets: + if info_provider not in target: + continue + info = target[info_provider] + if ctx.attr.transitive: + check_depsets.append(info.checks) + else: + marker_files.extend(info.direct_markers) + + checks = depset(marker_files, transitive = check_depsets) + runfiles = ctx.runfiles(transitive_files = checks).merge( + ctx.attr._runner[DefaultInfo].default_runfiles, + ) + + markers_env = ctx.configuration.host_path_separator.join([ + rlocationpath(f, ctx.workspace_name) + for f in checks.to_list() + ]) + + return [ + DefaultInfo( + files = depset([runner]), + runfiles = runfiles, + executable = runner, + ), + RunEnvironmentInfo(environment = { + "RUST_BACKTRACE": "1", + "RUST_LINT_TEST_MARKERS": markers_env, + }), + ] diff --git a/rust/private/lint_test_runner/BUILD.bazel b/rust/private/lint_test_runner/BUILD.bazel new file mode 100644 index 0000000000..1092d3ea38 --- /dev/null +++ b/rust/private/lint_test_runner/BUILD.bazel @@ -0,0 +1,11 @@ +load("//rust/private:rust.bzl", "rust_binary") + +rust_binary( + name = "lint_test_runner", + srcs = ["lint_test_runner.rs"], + edition = "2021", + visibility = ["//visibility:public"], + deps = [ + "//rust/runfiles", + ], +) diff --git a/rust/private/lint_test_runner/lint_test_runner.rs b/rust/private/lint_test_runner/lint_test_runner.rs new file mode 100644 index 0000000000..34d6dffc69 --- /dev/null +++ b/rust/private/lint_test_runner/lint_test_runner.rs @@ -0,0 +1,118 @@ +use std::env; +use std::fs; +use std::path::Path; +use std::process::ExitCode; + +#[cfg(target_family = "windows")] +const PATH_ENV_SEP: char = ';'; + +#[cfg(not(target_family = "windows"))] +const PATH_ENV_SEP: char = ':'; + +const MARKERS_ENV: &str = "RUST_LINT_TEST_MARKERS"; + +fn main() -> ExitCode { + let raw = env::var(MARKERS_ENV).unwrap_or_default(); + let entries: Vec<&str> = raw.split(PATH_ENV_SEP).filter(|s| !s.is_empty()).collect(); + + if entries.is_empty() { + println!("No lint outputs to report."); + return ExitCode::SUCCESS; + } + + let runfiles = match runfiles::Runfiles::create() { + Ok(r) => r, + Err(err) => { + eprintln!("Failed to locate runfiles: {err}"); + return ExitCode::FAILURE; + } + }; + + let mut failed = false; + for rlocation in &entries { + let Some(path) = runfiles::rlocation!(runfiles, rlocation) else { + eprintln!("Missing runfile: {rlocation}"); + failed = true; + continue; + }; + + match classify(&path) { + (Verdict::Pass, _) => println!("PASS {}", path.display()), + (Verdict::Fail(reason), contents) => { + eprintln!("FAIL {} — {reason}", path.display()); + if let Some(text) = contents { + if !text.trim().is_empty() { + eprintln!("--- {} ---\n{text}\n--- end ---", path.display()); + } + } + failed = true; + } + } + } + + if failed { ExitCode::FAILURE } else { ExitCode::SUCCESS } +} + +enum Verdict { + Pass, + Fail(&'static str), +} + +enum Kind { + Marker, // .clippy.ok / .rustfmt.ok — presence is success. + ClippyOut, // .clippy.out — captured stderr; non-empty means clippy had something to say. + Diagnostics, // .clippy.diagnostics — JSON stream; warning/error entries mean failure. + Unknown, +} + +fn kind_of(path: &Path) -> Kind { + let name = path.file_name().and_then(|s| s.to_str()).unwrap_or(""); + if name.ends_with(".clippy.ok") || name.ends_with(".rustfmt.ok") { + Kind::Marker + } else if name.ends_with(".clippy.out") { + Kind::ClippyOut + } else if name.ends_with(".clippy.diagnostics") { + Kind::Diagnostics + } else { + Kind::Unknown + } +} + +/// Decide whether a collected lint artifact represents pass or fail. +/// +/// Both `capture_clippy_output` and `clippy_output_diagnostics` make clippy +/// exit 0 even on real issues (via `--cap-lints=warn`), so we inspect file +/// contents rather than presence. Returns the file contents on failure so +/// callers can print them without re-reading from disk. +fn classify(path: &Path) -> (Verdict, Option) { + match kind_of(path) { + Kind::Marker => (Verdict::Pass, None), + Kind::ClippyOut => match fs::read_to_string(path) { + Ok(text) if text.trim().is_empty() => (Verdict::Pass, None), + Ok(text) => ( + Verdict::Fail("clippy stderr is non-empty (capture_clippy_output masks the exit code)"), + Some(text), + ), + Err(_) => (Verdict::Fail("failed to read captured clippy stderr"), None), + }, + Kind::Diagnostics => match fs::read_to_string(path) { + Ok(text) if text.lines().any(is_diagnostic_json_line) => ( + Verdict::Fail("clippy diagnostics contain warning/error entries (cap_at_warnings masks the exit code)"), + Some(text), + ), + Ok(_) => (Verdict::Pass, None), + Err(_) => (Verdict::Fail("failed to read clippy diagnostics"), None), + }, + Kind::Unknown => { + eprintln!("warning: unrecognized lint artifact {}", path.display()); + (Verdict::Pass, None) + } + } +} + +/// rustc emits compact newline-separated JSON (no whitespace between tokens); +/// a real diagnostic has `"level":"warning"` or `"level":"error"` as a +/// top-level field. Notes / help entries use other levels and are ignored. +fn is_diagnostic_json_line(line: &str) -> bool { + line.contains(r#""level":"warning""#) || line.contains(r#""level":"error""#) +} diff --git a/rust/private/rustfmt.bzl b/rust/private/rustfmt.bzl index c55033d69e..1ea1b9b375 100644 --- a/rust/private/rustfmt.bzl +++ b/rust/private/rustfmt.bzl @@ -1,6 +1,14 @@ """A module defining rustfmt rules""" load(":common.bzl", "rust_common") +load( + ":lint_test.bzl", + "LINT_TEST_COMMON_ATTRS", + "lint_test_aspect_impl", + "lint_test_rule_impl", + "platform_transition", + "rlocationpath", +) def _get_rustfmt_ready_crate_info(target): """Check that a target is suitable for rustfmt and extract the `CrateInfo` provider from it. @@ -56,24 +64,6 @@ def _find_rustfmtable_srcs(crate_info, aspect_ctx = None): return srcs -def _generate_manifest(edition, srcs, ctx): - workspace = ctx.label.workspace_name or ctx.workspace_name - - # Gather the source paths to non-generated files - content = ctx.actions.args() - content.set_param_file_format("multiline") - content.add_all(srcs, format_each = workspace + "/%s") - content.add(edition) - - # Write the rustfmt manifest - manifest = ctx.actions.declare_file(ctx.label.name + ".rustfmt") - ctx.actions.write( - output = manifest, - content = content, - ) - - return manifest - def _perform_check(edition, srcs, ctx): rustfmt_toolchain = ctx.toolchains[Label("//rust/rustfmt:toolchain_type")] @@ -201,120 +191,76 @@ generated source files are also ignored by this aspect. ], ) -_RustfmtTestManifestInfo = provider( - doc = "A container for rustfmt manifest info to use in `rustfmt_test`", +RustfmtTestInfo = provider( + doc = "Rustfmt check outputs surfaced by `rustfmt_test_aspect`.", fields = { - "manifest": "File: The manifest of formattable sources.", - "srcs": "depset[File]: The formattable sources.", + "checks": "depset[File]: Rustfmt markers for the visited target plus every crate reached via `deps`, `proc_macro_deps`, and `crate`.", + "direct_markers": "list[File]: Rustfmt markers for the visited target only.", }, ) -def _rustfmt_test_target_aspect_impl(target, ctx): - if RustfmtTargetInfo not in target: - return [] +_RUSTFMT_OUTPUT_GROUPS = ["rustfmt_checks"] - info = target[RustfmtTargetInfo] - manifest = _generate_manifest(info.edition, info.srcs, ctx) +def _rustfmt_test_aspect_impl(target, ctx): + return lint_test_aspect_impl(target, ctx, RustfmtTestInfo, _RUSTFMT_OUTPUT_GROUPS) - return [ - _RustfmtTestManifestInfo( - manifest = manifest, - srcs = depset(info.srcs), - ), - ] - -_rustfmt_test_target_aspect = aspect( - implementation = _rustfmt_test_target_aspect_impl, - doc = """This aspect is used to gather information about a crate for use in `rustfmt_test`""", - requires = [rustfmt_srcs_aspect], - fragments = ["cpp"], - toolchains = [ - str(Label("//rust/rustfmt:toolchain_type")), - ], +rustfmt_test_aspect = aspect( + implementation = _rustfmt_test_aspect_impl, + attr_aspects = ["deps", "proc_macro_deps", "crate"], + requires = [rustfmt_aspect], + provides = [RustfmtTestInfo], + doc = "Walks `deps`/`proc_macro_deps`/`crate` and rolls up the markers produced by `rustfmt_aspect` into a transitive `RustfmtTestInfo`.", ) -def _rlocationpath(file, workspace_name): - if file.short_path.startswith("../"): - return file.short_path[len("../"):] - - return "{}/{}".format(workspace_name, file.short_path) - 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. - is_windows = ctx.executable._runner.extension == ".exe" - runner = ctx.actions.declare_file("{}{}".format( - ctx.label.name, - ".exe" if is_windows else "", - )) - - ctx.actions.symlink( - output = runner, - target_file = ctx.executable._runner, - is_executable = True, - ) - - srcs = [] - manifests = [] - for target in ctx.attr.targets: - if _RustfmtTestManifestInfo not in target: - continue - info = target[_RustfmtTestManifestInfo] - manifests.append(info.manifest) - srcs.append(info.srcs) - - runfiles = ctx.runfiles( - transitive_files = depset(manifests, transitive = srcs), - ) - - runfiles = runfiles.merge( - ctx.attr._runner[DefaultInfo].default_runfiles, - ) - - return [ - DefaultInfo( - files = depset([runner]), - runfiles = runfiles, - executable = runner, - ), - RunEnvironmentInfo( - environment = { - "RUSTFMT_MANIFESTS": ctx.configuration.host_path_separator.join([ - _rlocationpath(manifest, ctx.workspace_name) - for manifest in manifests - ]), - "RUST_BACKTRACE": "1", - }, - ), - ] + return lint_test_rule_impl(ctx, RustfmtTestInfo) rustfmt_test = rule( implementation = _rustfmt_test_impl, - doc = "A test rule for performing `rustfmt --check` on a set of targets", - attrs = { + attrs = dict(LINT_TEST_COMMON_ATTRS, **{ "targets": attr.label_list( doc = "Rust targets to run `rustfmt --check` on.", providers = [ [rust_common.crate_info], [rust_common.test_crate_info], ], - aspects = [_rustfmt_test_target_aspect], - ), - "_runner": attr.label( - doc = "The rustfmt test runner", - cfg = "exec", - executable = True, - default = Label("//tools/rustfmt:rustfmt_test"), + aspects = [rustfmt_test_aspect], + cfg = platform_transition, ), - }, + }), test = True, + doc = """\ +A test rule that runs `rustfmt --check` over a set of Rust targets. + +By default (`transitive = True`), the aspect walks `deps`, `proc_macro_deps`, and `crate` +transitively so that listing a top-level target checks its whole crate graph. Set +`transitive = False` to format only the exact targets listed. The `rustfmt` actions run +during the build phase, so a formatting failure fails `bazel test` before the test +executable is invoked. + +An optional `platform` attribute transitions `targets` to the given platform before running +`rustfmt`. + +Example: + +```python +load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library", "rustfmt_test") + +rust_library(name = "lib", srcs = ["src/lib.rs"], edition = "2021") +rust_binary(name = "app", srcs = ["src/main.rs"], edition = "2021", deps = [":lib"]) + +rustfmt_test(name = "fmt_tree_test", targets = [":app"]) +rustfmt_test(name = "fmt_app_only_test", targets = [":app"], transitive = False) +``` + +Targets tagged `no_format`, `no_rustfmt`, or `norustfmt` are skipped. +""", ) def _rustfmt_toolchain_impl(ctx): make_variables = { "RUSTFMT": ctx.file.rustfmt.path, - "RUSTFMT_RLOCATIONPATH": _rlocationpath(ctx.file.rustfmt, ctx.workspace_name), + "RUSTFMT_RLOCATIONPATH": rlocationpath(ctx.file.rustfmt, ctx.workspace_name), } if ctx.attr.rustc: diff --git a/test/lint_tree/BUILD.bazel b/test/lint_tree/BUILD.bazel new file mode 100644 index 0000000000..3e09b848f1 --- /dev/null +++ b/test/lint_tree/BUILD.bazel @@ -0,0 +1,132 @@ +load( + "@rules_rust//rust:defs.bzl", + "rust_binary", + "rust_clippy_test", + "rust_library", + "rustfmt_test", +) + +package(default_visibility = ["//visibility:private"]) + +rust_library( + name = "lib2", + srcs = ["src/lib2.rs"], + crate_name = "lib2", + crate_root = "src/lib2.rs", + edition = "2021", +) + +rust_library( + name = "lib", + srcs = ["src/lib.rs"], + crate_name = "lib", + crate_root = "src/lib.rs", + edition = "2021", + deps = [":lib2"], +) + +rust_binary( + name = "app", + srcs = ["src/main.rs"], + crate_root = "src/main.rs", + edition = "2021", + deps = [":lib"], +) + +# Both tests run over just `:app`; the aspect walks its `deps` transitively, +# so `:lib` and `:lib2` are checked as well without being named here. +rust_clippy_test( + name = "tree_clippy_test", + targets = [":app"], +) + +rustfmt_test( + name = "tree_fmt_test", + targets = [":app"], +) + +# Leaf tagged `no_clippy`. The wrapper `deps` on it but does not `use` it — +# Bazel deps don't require a Rust `use`. Proves that the aspect gracefully +# skips a tagged crate while still processing the rest of the tree. +rust_library( + name = "leaf_no_clippy", + srcs = ["src/wrapper.rs"], + crate_name = "leaf_no_clippy", + crate_root = "src/wrapper.rs", + edition = "2021", + tags = ["no_clippy"], +) + +rust_library( + name = "wrapper_over_no_clippy", + srcs = ["src/wrapper.rs"], + crate_name = "wrapper_over_no_clippy", + crate_root = "src/wrapper.rs", + edition = "2021", + deps = [":leaf_no_clippy"], +) + +rust_clippy_test( + name = "tree_clippy_test_with_skip", + targets = [":wrapper_over_no_clippy"], +) + +# Leaf tagged `no_rustfmt`. +rust_library( + name = "leaf_no_rustfmt", + srcs = ["src/wrapper.rs"], + crate_name = "leaf_no_rustfmt", + crate_root = "src/wrapper.rs", + edition = "2021", + tags = ["no_rustfmt"], +) + +rust_library( + name = "wrapper_over_no_rustfmt", + srcs = ["src/wrapper.rs"], + crate_name = "wrapper_over_no_rustfmt", + crate_root = "src/wrapper.rs", + edition = "2021", + deps = [":leaf_no_rustfmt"], +) + +rustfmt_test( + name = "tree_fmt_test_with_skip", + targets = [":wrapper_over_no_rustfmt"], +) + +# `transitive = False` variants: only the exact target listed is checked, +# so the leaf crates are ignored even without any skip tags. +rust_clippy_test( + name = "app_only_clippy_test", + targets = [":app"], + transitive = False, +) + +rustfmt_test( + name = "app_only_fmt_test", + targets = [":app"], + transitive = False, +) + +# Deliberately fails clippy. Kept `manual` so it doesn't break the default +# test run. Useful for smoke-testing that the runner still catches clippy +# issues in the flag configurations that mask the exit code: +# +# bazel test //test/lint_tree:bad_bin_clippy_test # fails at build (default) +# bazel test //test/lint_tree:bad_bin_clippy_test \ +# --@rules_rust//rust/settings:capture_clippy_output=true # fails at runtime +# bazel test //test/lint_tree:bad_bin_clippy_test \ +# --@rules_rust//rust/settings:clippy_output_diagnostics=true # fails at runtime +rust_binary( + name = "bad_bin", + srcs = ["src/bad.rs"], + crate_root = "src/bad.rs", + edition = "2021", +) + +rust_clippy_test( + name = "bad_bin_clippy_test", + tags = ["manual"], + targets = [":bad_bin"], +) diff --git a/test/lint_tree/src/bad.rs b/test/lint_tree/src/bad.rs new file mode 100644 index 0000000000..cf9d4e6f4e --- /dev/null +++ b/test/lint_tree/src/bad.rs @@ -0,0 +1,6 @@ +fn main() { + loop { + println!("{}", "Hello World"); + break; + } +} diff --git a/test/lint_tree/src/lib.rs b/test/lint_tree/src/lib.rs new file mode 100644 index 0000000000..af994449bf --- /dev/null +++ b/test/lint_tree/src/lib.rs @@ -0,0 +1,3 @@ +pub fn greet(name: &str) -> String { + format!("hello {name}, {}", lib2::add(1, 2)) +} diff --git a/test/lint_tree/src/lib2.rs b/test/lint_tree/src/lib2.rs new file mode 100644 index 0000000000..b4a2a9e5dd --- /dev/null +++ b/test/lint_tree/src/lib2.rs @@ -0,0 +1,3 @@ +pub fn add(a: i32, b: i32) -> i32 { + a + b +} diff --git a/test/lint_tree/src/main.rs b/test/lint_tree/src/main.rs new file mode 100644 index 0000000000..85f37334d8 --- /dev/null +++ b/test/lint_tree/src/main.rs @@ -0,0 +1,3 @@ +fn main() { + println!("{}", lib::greet("world")); +} diff --git a/test/lint_tree/src/wrapper.rs b/test/lint_tree/src/wrapper.rs new file mode 100644 index 0000000000..bcb67cf2e9 --- /dev/null +++ b/test/lint_tree/src/wrapper.rs @@ -0,0 +1,3 @@ +pub fn ok() -> bool { + true +} diff --git a/test/unit/lint_tests/BUILD.bazel b/test/unit/lint_tests/BUILD.bazel new file mode 100644 index 0000000000..62c7579487 --- /dev/null +++ b/test/unit/lint_tests/BUILD.bazel @@ -0,0 +1,3 @@ +load(":lint_tests.bzl", "lint_tests_suite") + +lint_tests_suite(name = "lint_tests_suite") diff --git a/test/unit/lint_tests/lint_tests.bzl b/test/unit/lint_tests/lint_tests.bzl new file mode 100644 index 0000000000..ddc01f5155 --- /dev/null +++ b/test/unit/lint_tests/lint_tests.bzl @@ -0,0 +1,126 @@ +"""Unit tests for `rust_clippy_test` and `rustfmt_test`. + +One test per rule. Each verifies that with `transitive = True` (the default) +the collected marker count matches the dep graph, and that a crate tagged +for opt-out is skipped without breaking propagation to its own deps. +""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") +load( + "//rust:defs.bzl", + "rust_binary", + "rust_clippy_test", + "rust_library", + "rustfmt_test", +) + +# Graph shared by both tests: +# +# bin --> tagged_mid --> leaf +# +# `tagged_mid` carries `no_clippy` + `no_rustfmt`. With `transitive = True` +# both `bin` and `leaf` should get markers (2 total); `tagged_mid` is +# skipped but does NOT stop propagation to `leaf`. + +def _markers(target): + raw = target[RunEnvironmentInfo].environment.get("RUST_LINT_TEST_MARKERS", "") + return [s for s in raw.replace(";", ":").split(":") if s] + +def _make_transitive_count_test(marker_suffix): + def _impl(ctx): + env = analysistest.begin(ctx) + markers = _markers(analysistest.target_under_test(env)) + + bin_marker = "bin" + marker_suffix + leaf_marker = "leaf" + marker_suffix + tagged_marker = "tagged_mid" + marker_suffix + + asserts.equals( + env, + 2, + len([m for m in markers if m.endswith(marker_suffix)]), + "Expected 2 '{}' markers, got {}".format(marker_suffix, markers), + ) + asserts.true( + env, + any([bin_marker in m for m in markers]), + "Expected top-level bin marker in {}".format(markers), + ) + asserts.true( + env, + any([leaf_marker in m for m in markers]), + "Expected leaf-through-tagged-crate marker in {}".format(markers), + ) + for m in markers: + asserts.false( + env, + tagged_marker in m, + "Tagged crate should not have produced marker {}".format(m), + ) + return analysistest.end(env) + + return analysistest.make(_impl) + +clippy_transitive_test = _make_transitive_count_test(".clippy.ok") +rustfmt_transitive_test = _make_transitive_count_test(".rustfmt.ok") + +def lint_tests_suite(name): + """Wire up the fixture graph and the two analysistests. + + Args: + name: The name for the enclosing test_suite. + """ + rust_library( + name = "leaf", + srcs = ["src/lib.rs"], + crate_name = "leaf", + crate_root = "src/lib.rs", + edition = "2021", + ) + rust_library( + name = "tagged_mid", + srcs = ["src/lib.rs"], + crate_name = "tagged_mid", + crate_root = "src/lib.rs", + edition = "2021", + tags = [ + "no_clippy", + "no_rustfmt", + ], + deps = [":leaf"], + ) + rust_binary( + name = "bin", + srcs = ["src/main.rs"], + crate_root = "src/main.rs", + edition = "2021", + deps = [":tagged_mid"], + ) + + rust_clippy_test( + name = "clippy_fixture", + targets = [":bin"], + tags = ["manual"], + ) + rustfmt_test( + name = "rustfmt_fixture", + targets = [":bin"], + tags = ["manual"], + ) + + clippy_transitive_test( + name = "clippy_transitive_test", + target_under_test = ":clippy_fixture", + ) + rustfmt_transitive_test( + name = "rustfmt_transitive_test", + target_under_test = ":rustfmt_fixture", + ) + + native.test_suite( + name = name, + tests = [ + ":clippy_transitive_test", + ":rustfmt_transitive_test", + ], + ) diff --git a/test/unit/lint_tests/src/lib.rs b/test/unit/lint_tests/src/lib.rs new file mode 100644 index 0000000000..5c078f70ee --- /dev/null +++ b/test/unit/lint_tests/src/lib.rs @@ -0,0 +1,3 @@ +pub fn ping() -> &'static str { + "pong" +} diff --git a/test/unit/lint_tests/src/main.rs b/test/unit/lint_tests/src/main.rs new file mode 100644 index 0000000000..87daa7a7ed --- /dev/null +++ b/test/unit/lint_tests/src/main.rs @@ -0,0 +1,3 @@ +fn main() { + println!("ok"); +} diff --git a/test/unit/rustfmt/rustfmt_unit_test.bzl b/test/unit/rustfmt/rustfmt_unit_test.bzl index 56bb7f29b5..8b83057d8c 100644 --- a/test/unit/rustfmt/rustfmt_unit_test.bzl +++ b/test/unit/rustfmt/rustfmt_unit_test.bzl @@ -96,82 +96,38 @@ _generated_main_srcs_excluded_test = _make_generated_srcs_excluded_test("main.rs # ---------- rustfmt_test rule tests ---------- -def _get_manifest_runfiles(tut): - """Get .rustfmt manifest files from a target's default runfiles.""" - return [f for f in tut[DefaultInfo].default_runfiles.files.to_list() if f.extension == "rustfmt"] - -def _get_source_runfiles(tut): - """Get .rs source files from a target's default runfiles.""" - return [f for f in tut[DefaultInfo].default_runfiles.files.to_list() if f.extension == "rs"] - -def _rustfmt_test_single_target_test_impl(ctx): - env = analysistest.begin(ctx) - tut = analysistest.target_under_test(env) - - manifests = _get_manifest_runfiles(tut) - asserts.equals(env, 1, len(manifests), "Expected 1 manifest in runfiles, got {}".format( - [f.basename for f in manifests], - )) - - rs_files = _get_source_runfiles(tut) - basenames = [f.basename for f in rs_files] - asserts.true(env, "lib.rs" in basenames, "Expected lib.rs in runfiles, got {}".format(basenames)) - - return analysistest.end(env) - -_rustfmt_test_single_target_test = analysistest.make(_rustfmt_test_single_target_test_impl) - -def _rustfmt_test_multi_target_test_impl(ctx): - env = analysistest.begin(ctx) - tut = analysistest.target_under_test(env) - - manifests = _get_manifest_runfiles(tut) - asserts.equals(env, 2, len(manifests), "Expected 2 manifests in runfiles, got {}".format( - [f.basename for f in manifests], - )) - - return analysistest.end(env) - -_rustfmt_test_multi_target_test = analysistest.make(_rustfmt_test_multi_target_test_impl) - -def _rustfmt_test_norustfmt_skipped_test_impl(ctx): - env = analysistest.begin(ctx) - tut = analysistest.target_under_test(env) - - rs_files = _get_source_runfiles(tut) - asserts.equals(env, 0, len(rs_files), "Expected 0 source files for norustfmt target, got {}".format( - [f.basename for f in rs_files], - )) - - return analysistest.end(env) - -_rustfmt_test_norustfmt_skipped_test = analysistest.make(_rustfmt_test_norustfmt_skipped_test_impl) - -def _rustfmt_test_mixed_tags_test_impl(ctx): - env = analysistest.begin(ctx) - tut = analysistest.target_under_test(env) - - rs_files = _get_source_runfiles(tut) - basenames = [f.basename for f in rs_files] - asserts.equals(env, 1, len(rs_files), "Expected 1 source file (tagged target skipped), got {}".format(basenames)) - asserts.true(env, "lib.rs" in basenames, "Expected lib.rs in runfiles, got {}".format(basenames)) - - return analysistest.end(env) - -_rustfmt_test_mixed_tags_test = analysistest.make(_rustfmt_test_mixed_tags_test_impl) - -def _rustfmt_test_tag_variant_test_impl(ctx): - env = analysistest.begin(ctx) - tut = analysistest.target_under_test(env) - - rs_files = _get_source_runfiles(tut) - asserts.equals(env, 0, len(rs_files), "Expected 0 source files for format-skip tag variant, got {}".format( - [f.basename for f in rs_files], - )) +def _count_env_markers(tut, suffix): + run_env = tut[RunEnvironmentInfo].environment + raw = run_env.get("RUST_LINT_TEST_MARKERS", "") + if not raw: + return 0 + count = 0 + for part in raw.split(":"): + for sub in part.split(";"): + if sub.endswith(suffix): + count += 1 + return count + +def _make_marker_count_test(expected_count): + def _impl(ctx): + env = analysistest.begin(ctx) + tut = analysistest.target_under_test(env) + n = _count_env_markers(tut, ".rustfmt.ok") + asserts.equals( + env, + expected_count, + n, + "Expected {} rustfmt marker(s) in RUST_LINT_TEST_MARKERS, got {}".format(expected_count, n), + ) + return analysistest.end(env) - return analysistest.end(env) + return analysistest.make(_impl) -_rustfmt_test_tag_variant_test = analysistest.make(_rustfmt_test_tag_variant_test_impl) +_rustfmt_test_single_target_test = _make_marker_count_test(1) +_rustfmt_test_multi_target_test = _make_marker_count_test(2) +_rustfmt_test_norustfmt_skipped_test = _make_marker_count_test(0) +_rustfmt_test_mixed_tags_test = _make_marker_count_test(1) +_rustfmt_test_tag_variant_test = _make_marker_count_test(0) def _define_targets(): rust_library( diff --git a/tools/rustfmt/BUILD.bazel b/tools/rustfmt/BUILD.bazel index 3888b155c8..3d23a5b1c8 100644 --- a/tools/rustfmt/BUILD.bazel +++ b/tools/rustfmt/BUILD.bazel @@ -1,4 +1,4 @@ -load("//rust:defs.bzl", "rust_binary", "rust_clippy", "rust_library") +load("//rust:defs.bzl", "rust_binary", "rust_clippy") load("//tools/private:tool_utils.bzl", "aspect_repository") exports_files( @@ -9,29 +9,6 @@ exports_files( visibility = ["//visibility:public"], ) -rust_library( - name = "rustfmt_lib", - srcs = glob( - include = ["src/**/*.rs"], - exclude = [ - "src/**/*main.rs", - "src/bin/**", - ], - ), - data = [ - "//rust/settings:rustfmt.toml", - "//rust/toolchain:current_rustfmt_toolchain_for_target", - ], - edition = "2018", - rustc_env = { - "RUSTFMT": "$(rlocationpath //rust/toolchain:current_rustfmt_toolchain_for_target)", - "RUSTFMT_CONFIG": "$(rlocationpath //rust/settings:rustfmt.toml)", - }, - deps = [ - "//rust/runfiles", - ], -) - # Deprecated but present for compatibility. alias( name = "rustfmt", @@ -44,35 +21,23 @@ alias( # and will try to do things like set the correct edition for files when formatting them based on their owning targets. rust_binary( name = "target_aware_rustfmt", - srcs = [ - "src/main.rs", - ], + srcs = ["src/main.rs"], data = [ "//rust/settings:rustfmt.toml", + "//rust/toolchain:current_rustfmt_toolchain_for_target", ], edition = "2018", rustc_env = { "ASPECT_REPOSITORY": aspect_repository(), + "RUSTFMT": "$(rlocationpath //rust/toolchain:current_rustfmt_toolchain_for_target)", + "RUSTFMT_CONFIG": "$(rlocationpath //rust/settings:rustfmt.toml)", "RUST_DEFAULT_EDITION": "$(RUST_DEFAULT_EDITION)", }, toolchains = ["@rules_rust//rust/toolchain:current_rust_toolchain"], visibility = ["//visibility:public"], deps = [ - ":rustfmt_lib", - "//util/label", - ], -) - -rust_binary( - name = "rustfmt_test", - srcs = [ - "src/bin/test_main.rs", - ], - edition = "2018", - visibility = ["//visibility:public"], - deps = [ - ":rustfmt_lib", "//rust/runfiles", + "//util/label", ], ) diff --git a/tools/rustfmt/src/bin/test_main.rs b/tools/rustfmt/src/bin/test_main.rs deleted file mode 100644 index d4cbe31dcf..0000000000 --- a/tools/rustfmt/src/bin/test_main.rs +++ /dev/null @@ -1,74 +0,0 @@ -use std::path::PathBuf; -use std::process::Command; - -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 occurred 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) - .arg("--config") - .arg("skip_children=true") - .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 settings from the environment into a config struct -fn parse_args() -> Config { - let manifests: Vec = rustfmt_lib::find_manifests(); - - if manifests.is_empty() { - panic!("No manifests were found"); - } - - Config { - rustfmt_config: rustfmt_lib::parse_rustfmt_config(), - manifests: manifests - .iter() - .map(|manifest| rustfmt_lib::parse_rustfmt_manifest(manifest)) - .collect(), - } -} diff --git a/tools/rustfmt/src/lib.rs b/tools/rustfmt/src/lib.rs deleted file mode 100644 index 840aa39dfc..0000000000 --- a/tools/rustfmt/src/lib.rs +++ /dev/null @@ -1,97 +0,0 @@ -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: &str = "rustfmt"; - -/// 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 { - let runfiles = runfiles::Runfiles::create().unwrap(); - - let rustfmt = runfiles::rlocation!(runfiles, env!("RUSTFMT")).unwrap(); - if !rustfmt.exists() { - panic!("rustfmt does not exist at: {}", rustfmt.display()); - } - - let config = runfiles::rlocation!(runfiles, env!("RUSTFMT_CONFIG")).unwrap(); - if !config.exists() { - panic!( - "rustfmt config file does not exist at: {}", - config.display() - ); - } - - RustfmtConfig { rustfmt, config } -} - -/// 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) - .unwrap_or_else(|_| panic!("Failed to read rustfmt manifest: {}", manifest.display())); - - let mut lines: Vec = content - .split('\n') - .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`."); - - let runfiles = runfiles::Runfiles::create().unwrap(); - - RustfmtManifest { - edition, - sources: lines - .into_iter() - .map(|src| runfiles::rlocation!(runfiles, src).unwrap()) - .collect(), - } -} - -#[cfg(target_family = "windows")] -const PATH_ENV_SEP: &str = ";"; - -#[cfg(target_family = "unix")] -const PATH_ENV_SEP: &str = ":"; - -/// Parse the runfiles of the current executable for manifests generated -/// by the `rustfmt_aspect` aspect. -pub fn find_manifests() -> Vec { - let runfiles = runfiles::Runfiles::create().unwrap(); - - std::env::var("RUSTFMT_MANIFESTS") - .map(|var| { - var.split(PATH_ENV_SEP) - .map(|path| runfiles::rlocation!(runfiles, path).unwrap()) - .collect() - }) - .unwrap_or_default() -} diff --git a/tools/rustfmt/src/main.rs b/tools/rustfmt/src/main.rs index 7104cf3fa3..a1833c39dc 100644 --- a/tools/rustfmt/src/main.rs +++ b/tools/rustfmt/src/main.rs @@ -6,6 +6,35 @@ use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::str; +/// Details used for executing rustfmt. +#[derive(Debug)] +struct RustfmtConfig { + /// The rustfmt binary from the currently active toolchain + rustfmt: PathBuf, + + /// The rustfmt config file containing rustfmt settings. + /// https://rust-lang.github.io/rustfmt/ + config: PathBuf, +} + +/// Locate the rustfmt binary and config via the runfiles env vars baked in +/// at compile time. +fn parse_rustfmt_config() -> RustfmtConfig { + let runfiles = runfiles::Runfiles::create().unwrap(); + + let rustfmt = runfiles::rlocation!(runfiles, env!("RUSTFMT")).unwrap(); + if !rustfmt.exists() { + panic!("rustfmt does not exist at: {}", rustfmt.display()); + } + + let config = runfiles::rlocation!(runfiles, env!("RUSTFMT_CONFIG")).unwrap(); + if !config.exists() { + panic!("rustfmt config file does not exist at: {}", config.display()); + } + + RustfmtConfig { rustfmt, config } +} + /// The Bazel Rustfmt tool entry point fn main() { // Gather all command line and environment settings @@ -173,7 +202,7 @@ struct Config { pub bazel: PathBuf, /// Information about the current rustfmt binary to run. - pub rustfmt_config: rustfmt_lib::RustfmtConfig, + pub rustfmt_config: RustfmtConfig, /// Optionally, users can pass a list of targets/packages/scopes /// (eg `//my:target` or `//my/pkg/...`) to control the targets @@ -194,7 +223,7 @@ fn parse_args() -> Config { env::var("BAZEL_REAL") .unwrap_or_else(|_| "bazel".to_owned()) ), - rustfmt_config: rustfmt_lib::parse_rustfmt_config(), + rustfmt_config: parse_rustfmt_config(), packages: env::args().skip(1).collect(), } }