Add support for clippy.toml (#835)

* Add support for clippy.toml

* Regenerate documentation

* Fix build file naming

* Fix use of depset

* Fix path for non-root config file locations

* Document Rust 1.34.0 requirement

* Regenerate documentation

* Document location of clippy documentation

* buildifier

* Use modern python formatting

Co-authored-by: UebelAndre <[email protected]>

* Use modern python formatting

Co-authored-by: UebelAndre <[email protected]>

* Add a test

* Address review comments

Co-authored-by: UebelAndre <[email protected]>

Author: djmarcin

BUILD.bazel

@@ -18,6 +18,13 @@ error_format(
     visibility = ["//visibility:public"],
 )
 
+# This setting is used by the clippy rules. See https://bazelbuild.github.io/rules_rust/rust_clippy.html
+label_flag(
+    name = "clippy.toml",
+    build_setting_default = "//tools/clippy:clippy.toml",
+    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",

docs/rust_clippy.md

@@ -29,6 +29,14 @@ This will enable clippy on all [Rust targets](./defs.md).
 
 Note that targets tagged with `noclippy` will not perform clippy checks
 
+To use a local clippy.toml, add the following flag to your `.bazelrc`. Note that due to
+the upstream implementation of clippy, this file must be named either `.clippy.toml` or
+`clippy.toml`. Using a custom config file requires Rust 1.34.0 or newer.
+
+```text
+build --@rules_rust//:clippy.toml=//:clippy.toml
+```
+
 <a id="#rust_clippy"></a>
 
 ## rust_clippy

docs/rust_clippy.vm

@@ -22,3 +22,11 @@ build --output_groups=+clippy_checks
 This will enable clippy on all [Rust targets](./defs.md).
 
 Note that targets tagged with `noclippy` will not perform clippy checks
+
+To use a local clippy.toml, add the following flag to your `.bazelrc`. Note that due to
+the upstream implementation of clippy, this file must be named either `.clippy.toml` or
+`clippy.toml`. Using a custom config file requires Rust 1.34.0 or newer.
+
+```text
+build --@rules_rust//:clippy.toml=//:clippy.toml
+```

rust/private/clippy.bzl

@@ -116,6 +116,14 @@ def _clippy_aspect_impl(target, ctx):
     if crate_info.is_test:
         args.add("--test")
 
+    # Upstream clippy requires one of these two filenames or it silently uses
+    # the default config. Enforce the naming so users are not confused.
+    valid_config_file_names = [".clippy.toml", "clippy.toml"]
+    if ctx.file._config.basename not in valid_config_file_names:
+        fail("The clippy config file must be named one of: {}".format(valid_config_file_names))
+    env["CLIPPY_CONF_DIR"] = "${{pwd}}/{}".format(ctx.file._config.dirname)
+    compile_inputs = depset([ctx.file._config], transitive = [compile_inputs])
+
     ctx.actions.run(
         executable = ctx.executable._process_wrapper,
         inputs = compile_inputs,
@@ -145,6 +153,11 @@ rust_clippy_aspect = aspect(
             ),
             default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"),
         ),
+        "_config": attr.label(
+            doc = "The `clippy.toml` file used for configuration",
+            allow_single_file = True,
+            default = Label("//:clippy.toml"),
+        ),
         "_error_format": attr.label(
             doc = "The desired `--error-format` flags for clippy",
             default = "//:error_format",

test/clippy/clippy_failure_test.sh

@@ -20,10 +20,17 @@ cd "${BUILD_WORKSPACE_DIRECTORY}"
 # Takes two arguments:
 # ${1}: The expected return code.
 # ${2}: The target within "//test/clippy" to be tested.
+#
+# Optional third argument:
+# ${3}: The clippy.toml file to pass as an override.
 function check_build_result() {
   local ret=0
   echo -n "Testing ${2}... "
-  (bazel build //test/clippy:"${2}" &> /dev/null) || ret="$?" && true
+  ADDITIONAL_ARGS=()
+  if [[ -n "${3-}" ]]; then
+    ADDITIONAL_ARGS+=("--@rules_rust//:clippy.toml=${3}")
+  fi
+  (bazel build "${ADDITIONAL_ARGS[@]}" //test/clippy:"${2}" &> /dev/null) || ret="$?" && true
   if [[ "${ret}" -ne "${1}" ]]; then
     echo "FAIL: Unexpected return code [saw: ${ret}, want: ${1}] building target //test/clippy:${2}"
     echo "  Run \"bazel build //test/clippy:${2}\" to see the output"
@@ -69,6 +76,10 @@ EOF
   check_build_result $BUILD_FAILED bad_binary_clippy
   check_build_result $BUILD_FAILED bad_library_clippy
   check_build_result $BUILD_FAILED bad_test_clippy
+
+  # 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 //too_many_args:clippy.toml
 }
 
 test_all

test/clippy/src/lib.rs

@@ -2,6 +2,9 @@ pub fn greeting() -> String {
     "Hello World".to_owned()
 }
 
+// too_many_args/clippy.toml will require no more than 2 args.
+pub fn with_args(_: u32, _: u32, _: u32) {}
+
 #[cfg(test)]
 mod tests {
     use super::*;

test/clippy/too_many_args/BUILD.bazel

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

test/clippy/too_many_args/clippy.toml

@@ -0,0 +1 @@
+too-many-arguments-threshold = 2

tools/clippy/BUILD.bazel

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

tools/clippy/clippy.toml

@@ -0,0 +1 @@
+# clippy options: https://rust-lang.github.io/rust-clippy/