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

@@ -274,13 +274,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 raised
+# above the default so 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

@@ -1,8 +1,8 @@
 """Unittest to verify properties of clippy rules"""
 
-load("@bazel_skylib//lib:unittest.bzl", "analysistest")
+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", "assert_list_contains_adjacent_elements")
+load("//test/unit:common.bzl", "assert_argv_contains", "assert_argv_contains_prefix_suffix", "assert_env_value", "assert_list_contains_adjacent_elements")
 
 def _find_clippy_action(actions):
     for action in actions:
@@ -83,6 +83,31 @@ 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),
+    )
+
+    config_inputs = [
+        f
+        for f in clippy_action.inputs.to_list()
+        if f.dirname == expected_dir and f.basename in ("clippy.toml", ".clippy.toml")
+    ]
+    asserts.true(
+        env,
+        len(config_inputs) == 1,
+        "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,
@@ -136,6 +161,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.
 
@@ -183,6 +216,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 = [
@@ -195,5 +245,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",
         ],
     )