Add per-target clippy_config attribute to Rust rules

Author: zertosh

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.

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,

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.

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"],

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

test/clippy/target_config/BUILD.bazel

@@ -0,0 +1 @@
+exports_files(["clippy.toml"])

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

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",
         ],
     )