diff --git a/docs/rust_clippy.vm b/docs/rust_clippy.vm index 2576175099..31ef513063 100644 --- a/docs/rust_clippy.vm +++ b/docs/rust_clippy.vm @@ -30,3 +30,20 @@ the upstream implementation of clippy, this file must be named either `.clippy.t ```text build --@rules_rust//rust/settings:clippy.toml=//:clippy.toml ``` + +Individual targets may also point at their own configuration file via the +`clippy_config` attribute, which overrides the build setting above for that +target only. This is useful in large workspaces where different subtrees need +different `disallowed-methods`, `disallowed-types`, or other clippy options. + +```python +rust_library( + name = "hello_lib", + srcs = ["src/lib.rs"], + clippy_config = "//path/to:clippy.toml", +) +``` + +Note that `rust_test` targets which use the `crate` attribute to test a library +do not inherit that library's `clippy_config`. Set `clippy_config` explicitly on +the `rust_test` target if it should use the same configuration. diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl index 2b42e18e4e..73a4d105bf 100644 --- a/rust/private/clippy.bzl +++ b/rust/private/clippy.bzl @@ -223,6 +223,13 @@ def rust_clippy_action(ctx, clippy_executable, process_wrapper, crate_info, conf env["CLIPPY_CONF_DIR"] = "${{pwd}}/{}".format(config.dirname) compile_inputs = depset([config], transitive = [compile_inputs]) + # `CLIPPY_CONF_DIR` is an env var, so it is not rewritten by Bazel's + # path mapping. When the config is a generated file, advertising + # `supports-path-mapping` would stage it under the mapped `bazel-out/` + # prefix while the env var keeps the unmapped path, and clippy would + # fail to find the config. Source files have no config segment to map. + supports_path_mapping = args.supports_path_mapping and config.is_source + ctx.actions.run( executable = process_wrapper, inputs = compile_inputs, @@ -233,7 +240,7 @@ def rust_clippy_action(ctx, clippy_executable, process_wrapper, crate_info, conf mnemonic = "Clippy", progress_message = "Clippy %{label}", toolchain = "@rules_rust//rust:toolchain_type", - execution_requirements = {"supports-path-mapping": ""} if args.supports_path_mapping else None, + execution_requirements = {"supports-path-mapping": ""} if supports_path_mapping else None, ) def _clippy_aspect_impl(target, ctx): @@ -275,13 +282,17 @@ def _clippy_aspect_impl(target, ctx): if ctx.attr._clippy_output_diagnostics[ClippyOutputDiagnosticsInfo].output_diagnostics: clippy_diagnostics = ctx.actions.declare_file(ctx.label.name + ".clippy.diagnostics", sibling = crate_info.output) + # Prefer a per-target clippy.toml when the underlying rule sets one, + # otherwise fall back to the global //rust/settings:clippy.toml flag. + config = getattr(ctx.rule.file, "clippy_config", None) or ctx.file._config + # Run clippy using the extracted function rust_clippy_action( ctx = ctx, clippy_executable = toolchain.clippy_driver, process_wrapper = ctx.executable._process_wrapper, crate_info = crate_info, - config = ctx.file._config, + config = config, output = clippy_out, cap_at_warnings = clippy_out != None, # If we're capturing output, we want the build to continue. success_marker = clippy_success_marker, diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index 9d9c1d12b4..a8217ebd84 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -664,6 +664,21 @@ _COMMON_ATTRS = { """), default = False, ), + "clippy_config": attr.label( + doc = dedent("""\ + A `clippy.toml` configuration file used when running clippy on this target. + + When set, `rust_clippy_aspect` points `CLIPPY_CONF_DIR` at the directory + containing this file instead of the global file resolved through the + `@rules_rust//rust/settings:clippy.toml` build setting. The file must be + named `clippy.toml` or `.clippy.toml`. + + Note that `rust_test` targets using the `crate` attribute do not inherit + this setting from the crate under test. Set `clippy_config` on the test + target explicitly if it should use the same configuration. + """), + allow_single_file = True, + ), "compile_data": attr.label_list( doc = dedent("""\ List of files used by this rule at compile time. diff --git a/test/clippy/BUILD.bazel b/test/clippy/BUILD.bazel index ea449a0008..a0a7f794b4 100644 --- a/test/clippy/BUILD.bazel +++ b/test/clippy/BUILD.bazel @@ -1,3 +1,4 @@ +load("@bazel_skylib//rules:write_file.bzl", "write_file") load( "@rules_rust//rust:defs.bzl", "rust_binary", @@ -50,6 +51,24 @@ rust_proc_macro( edition = "2018", ) +rust_library( + name = "ok_library_with_clippy_config", + srcs = ["src/lib.rs"], + clippy_config = "//test/clippy/target_config:clippy.toml", + edition = "2018", +) + +rust_test( + name = "ok_crate_test_without_clippy_config", + crate = ":ok_library_with_clippy_config", +) + +rust_test( + name = "ok_crate_test_with_clippy_config", + clippy_config = "//test/clippy/target_config:clippy.toml", + crate = ":ok_library_with_clippy_config", +) + # Clippy analysis of passing targets. rust_clippy( @@ -83,6 +102,11 @@ rust_clippy( deps = [":ok_proc_macro"], ) +rust_clippy( + name = "ok_library_with_clippy_config_clippy", + deps = [":ok_library_with_clippy_config"], +) + # Declaration of failing targets. rust_binary( @@ -127,6 +151,28 @@ rust_proc_macro( tags = ["noclippy"], ) +rust_library( + name = "bad_library_with_clippy_config", + srcs = ["src/lib.rs"], + clippy_config = "//test/clippy/too_many_args:clippy.toml", + edition = "2018", + tags = ["noclippy"], +) + +write_file( + name = "generated_clippy_toml", + out = "gen/clippy.toml", + content = ["too-many-arguments-threshold = 1"], +) + +rust_library( + name = "bad_library_with_generated_clippy_config", + srcs = ["src/lib.rs"], + clippy_config = ":generated_clippy_toml", + edition = "2018", + tags = ["noclippy"], +) + # Clippy analysis of failing targets. rust_clippy( @@ -166,6 +212,18 @@ rust_clippy( deps = [":bad_proc_macro"], ) +rust_clippy( + name = "bad_library_with_clippy_config_clippy", + tags = ["manual"], + deps = [":bad_library_with_clippy_config"], +) + +rust_clippy( + name = "bad_library_with_generated_clippy_config_clippy", + tags = ["manual"], + deps = [":bad_library_with_generated_clippy_config"], +) + sh_binary( name = "clippy_failure_tester", srcs = ["clippy_failure_tester.sh"], diff --git a/test/clippy/clippy_failure_tester.sh b/test/clippy/clippy_failure_tester.sh index 05ab24c6f9..e2dfa83368 100755 --- a/test/clippy/clippy_failure_tester.sh +++ b/test/clippy/clippy_failure_tester.sh @@ -57,7 +57,7 @@ function test_all() { local -r BUILD_OK=0 local -r BUILD_FAILED=1 local -r CAPTURE_OUTPUT="--@rules_rust//rust/settings:capture_clippy_output=True --@rules_rust//rust/settings:error_format=json" - local -r BAD_CLIPPY_TOML="--@rules_rust//rust/settings:clippy.toml=//too_many_args:clippy.toml" + local -r BAD_CLIPPY_TOML="--@rules_rust//rust/settings:clippy.toml=//test/clippy/too_many_args:clippy.toml" mkdir -p "${NEW_WORKSPACE}/test/clippy" && \ cp -r test/clippy/* "${NEW_WORKSPACE}/test/clippy/" && \ @@ -103,6 +103,7 @@ EOF check_build_result $BUILD_OK ok_static_library_clippy check_build_result $BUILD_OK ok_test_clippy check_build_result $BUILD_OK ok_proc_macro_clippy + check_build_result $BUILD_OK ok_library_with_clippy_config_clippy check_build_result $BUILD_FAILED bad_binary_clippy check_build_result $BUILD_FAILED bad_library_clippy check_build_result $BUILD_FAILED bad_shared_library_clippy @@ -123,6 +124,20 @@ EOF # Test that we can make the ok_library_clippy fail when using an extra config file. # Proves that the config file is used and overrides default settings. check_build_result $BUILD_FAILED ok_library_clippy $BAD_CLIPPY_TOML + + # Test that a per-target clippy_config attribute is honored. The library + # passes with the default config but its clippy_config attribute points at + # too_many_args/clippy.toml, which lowers the threshold so clippy fails. + check_build_result $BUILD_FAILED bad_library_with_clippy_config_clippy + + # Test that a per-target clippy_config attribute takes precedence over the + # global label_flag. The global flag would fail this library, but the + # per-target config raises the threshold so clippy passes. + check_build_result $BUILD_OK ok_library_with_clippy_config_clippy $BAD_CLIPPY_TOML + + # Test that a generated clippy_config (output under bazel-out/) is wired + # through correctly via CLIPPY_CONF_DIR. + check_build_result $BUILD_FAILED bad_library_with_generated_clippy_config_clippy } test_all diff --git a/test/clippy/target_config/BUILD.bazel b/test/clippy/target_config/BUILD.bazel new file mode 100644 index 0000000000..09e246fd35 --- /dev/null +++ b/test/clippy/target_config/BUILD.bazel @@ -0,0 +1 @@ +exports_files(["clippy.toml"]) diff --git a/test/clippy/target_config/clippy.toml b/test/clippy/target_config/clippy.toml new file mode 100644 index 0000000000..1387da090d --- /dev/null +++ b/test/clippy/target_config/clippy.toml @@ -0,0 +1,4 @@ +# A benign per-target clippy configuration. The threshold is high +# enough that src/lib.rs continues to pass clippy when this file is +# wired in via the `clippy_config` attribute. +too-many-arguments-threshold = 10 diff --git a/test/unit/clippy/clippy_test.bzl b/test/unit/clippy/clippy_test.bzl index 6efefed319..facce62929 100644 --- a/test/unit/clippy/clippy_test.bzl +++ b/test/unit/clippy/clippy_test.bzl @@ -2,7 +2,7 @@ load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") load("//rust:defs.bzl", "rust_clippy_aspect") -load("//test/unit:common.bzl", "assert_argv_contains", "assert_argv_contains_prefix_suffix") +load("//test/unit:common.bzl", "assert_argv_contains", "assert_argv_contains_prefix_suffix", "assert_env_value") def _find_clippy_action(actions): for action in actions: @@ -93,6 +93,35 @@ def _clippy_aspect_with_explicit_flags_test_impl(ctx): _CLIPPY_EXPLICIT_FLAGS + _CLIPPY_INDIVIDUALLY_ADDED_EXPLICIT_FLAGS, ) +def _clippy_aspect_conf_dir_test_impl(ctx, expected_dir): + env = analysistest.begin(ctx) + target = analysistest.target_under_test(env) + + clippy_action = _find_clippy_action(target.actions) + assert_env_value( + env, + clippy_action, + "CLIPPY_CONF_DIR", + "${{pwd}}/{}".format(expected_dir), + ) + + # Exactly one clippy.toml should appear in the action inputs, and it + # should come from `expected_dir`. Any other clippy.toml (e.g., the + # unchosen global default) is a bug: the aspect would still rerun on + # changes to a config it never reads. + config_inputs = [ + f + for f in clippy_action.inputs.to_list() + if f.basename in ("clippy.toml", ".clippy.toml") + ] + asserts.true( + env, + len(config_inputs) == 1 and config_inputs[0].dirname == expected_dir, + "expected exactly one clippy config from {} in action inputs, got {}".format(expected_dir, config_inputs), + ) + + return analysistest.end(env) + def make_clippy_aspect_unittest(impl, **kwargs): return analysistest.make( impl, @@ -146,6 +175,14 @@ clippy_aspect_with_output_diagnostics_test = make_clippy_aspect_unittest( }, ) +clippy_aspect_uses_default_conf_dir_test = make_clippy_aspect_unittest( + lambda ctx: _clippy_aspect_conf_dir_test_impl(ctx, "rust/settings"), +) + +clippy_aspect_uses_target_conf_dir_test = make_clippy_aspect_unittest( + lambda ctx: _clippy_aspect_conf_dir_test_impl(ctx, "test/clippy/target_config"), +) + def clippy_test_suite(name): """Entry-point macro called from the BUILD file. @@ -193,6 +230,23 @@ def clippy_test_suite(name): target_under_test = Label("//test/clippy:ok_library"), ) + clippy_aspect_uses_default_conf_dir_test( + name = "clippy_aspect_uses_default_conf_dir_test", + target_under_test = Label("//test/clippy:ok_library"), + ) + clippy_aspect_uses_target_conf_dir_test( + name = "clippy_aspect_uses_target_conf_dir_test", + target_under_test = Label("//test/clippy:ok_library_with_clippy_config"), + ) + clippy_aspect_uses_default_conf_dir_test( + name = "clippy_aspect_crate_test_ignores_library_conf_dir_test", + target_under_test = Label("//test/clippy:ok_crate_test_without_clippy_config"), + ) + clippy_aspect_uses_target_conf_dir_test( + name = "clippy_aspect_crate_test_uses_own_conf_dir_test", + target_under_test = Label("//test/clippy:ok_crate_test_with_clippy_config"), + ) + native.test_suite( name = name, tests = [ @@ -205,5 +259,9 @@ def clippy_test_suite(name): ":clippy_aspect_without_clippy_error_format_test", ":clippy_aspect_with_clippy_error_format_test", ":clippy_aspect_with_output_diagnostics_test", + ":clippy_aspect_uses_default_conf_dir_test", + ":clippy_aspect_uses_target_conf_dir_test", + ":clippy_aspect_crate_test_ignores_library_conf_dir_test", + ":clippy_aspect_crate_test_uses_own_conf_dir_test", ], )