feat(clippy): Introduce flag to toggle transitive aspect

Author: blorente

docs/rust_clippy.vm

@@ -30,3 +30,10 @@ the upstream implementation of clippy, this file must be named either `.clippy.t
 ```text
 build --@rules_rust//rust/settings:clippy.toml=//:clippy.toml
 ```
+
+By default, the Clippy aspect won't traverse a target's dependencies. If you'd like it to do so,
+toggle that behavior with the following setting:
+
+```text
+build --@rules_rust//rust/settings:experimental_clippy_aspect_traverse_deps=True
+```

rust/private/clippy.bzl

@@ -15,6 +15,7 @@
 """A module defining clippy rules"""
 
 load("@bazel_skylib//lib:structs.bzl", "structs")
+load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
 load("//rust/private:common.bzl", "rust_common")
 load(
     "//rust/private:providers.bzl",
@@ -241,9 +242,26 @@ def _clippy_aspect_impl(target, ctx):
         toolchain = "@rules_rust//rust:toolchain_type",
     )
 
-    output_group_info = {"clippy_checks": depset([clippy_out])}
+    traverse_deps = ctx.attr._clippy_aspect_traverse_deps[BuildSettingInfo].value
+
+    dep_infos = []
+    if traverse_deps:
+        dep_infos = [
+            dep[ClippyInfo].output
+            for dep in ctx.rule.attr.deps
+            if ClippyInfo in dep
+        ]
+
+    output_group_info = {"clippy_checks": depset([clippy_out], transitive = dep_infos)}
     if clippy_diagnostics:
-        output_group_info["clippy_output"] = depset([clippy_diagnostics])
+        dep_diagnostics = []
+        if traverse_deps:
+            dep_diagnostics = [
+                dep[OutputGroupInfo]["clippy_output"]
+                for dep in ctx.rule.attr.deps
+                if OutputGroupInfo in dep and "clippy_output" in dep[OutputGroupInfo]
+            ]
+        output_group_info["clippy_output"] = depset([clippy_diagnostics], transitive = dep_diagnostics)
 
     return [
         OutputGroupInfo(**output_group_info),
@@ -256,6 +274,7 @@ def _clippy_aspect_impl(target, ctx):
 #               //...
 rust_clippy_aspect = aspect(
     fragments = ["cpp"],
+    attr_aspects = ["deps"],  # We traverse along the `deps` edges, but we only request the relevant files iff the _clippy_aspect_traverse_deps attr is True.
     attrs = {
         "_capture_output": attr.label(
             doc = "Value of the `capture_clippy_output` build setting",
@@ -278,6 +297,10 @@ rust_clippy_aspect = aspect(
             doc = "Value of the `clippy_output_diagnostics` build setting",
             default = "//rust/settings:clippy_output_diagnostics",
         ),
+        "_clippy_aspect_traverse_deps": attr.label(
+            doc = "Whether the clippy aspect should traverse dependencies.",
+            default = "//rust/settings:experimental_clippy_aspect_traverse_deps",
+        ),
         "_config": attr.label(
             doc = "The `clippy.toml` file used for configuration",
             allow_single_file = True,

rust/settings/BUILD.bazel

@@ -3,6 +3,7 @@ load(
     ":settings.bzl",
     "always_enable_metadata_output_groups",
     "capture_clippy_output",
+    "clippy_aspect_traverse_deps",
     "clippy_error_format",
     "clippy_flag",
     "clippy_flags",
@@ -68,6 +69,8 @@ clippy_output_diagnostics()
 
 clippy_toml()
 
+clippy_aspect_traverse_deps()
+
 codegen_units()
 
 error_format()

rust/settings/settings.bzl

@@ -245,6 +245,15 @@ def clippy_toml():
         build_setting_default = ".clippy.toml",
     )
 
+# buildifier: disable=unnamed-macro
+def clippy_aspect_traverse_deps():
+    """This setting is used by the clippy rules. See https://bazelbuild.github.io/rules_rust/rust_clippy.html
+    """
+    bool_flag(
+        name = "experimental_clippy_aspect_traverse_deps",
+        build_setting_default = False,
+    )
+
 # buildifier: disable=unnamed-macro
 def rustfmt_toml():
     """This setting is used by the rustfmt rules. See https://bazelbuild.github.io/rules_rust/rust_fmt.html

test/clippy/BUILD.bazel

@@ -99,6 +99,14 @@ rust_library(
     tags = ["noclippy"],
 )
 
+rust_binary(
+    name = "bad_dep",
+    srcs = ["src/main.rs"],
+    edition = "2018",
+    tags = ["noclippy"],
+    deps = [":bad_library"],
+)
+
 rust_library(
     name = "bad_shared_library",
     srcs = ["bad_src/lib.rs"],
@@ -142,8 +150,13 @@ rust_clippy(
 )
 
 rust_clippy(
-    name = "bad_shared_library_clippy",
+    name = "bad_dep_clippy",
     tags = ["manual"],
+    deps = [":bad_dep"],
+)
+
+rust_clippy(
+    name = "bad_shared_library_clippy",
     deps = [":bad_shared_library"],
 )
 

test/clippy/clippy_failure_tester.sh

@@ -28,10 +28,11 @@ NEW_WORKSPACE="${TEMP_DIR}/rules_rust_test_clippy"
 function check_build_result() {
   local ret=0
   echo -n "Testing ${2}... "
+
   (bazel build ${@:3} //test/clippy:"${2}" &> /dev/null) || ret="$?" && true
   if [[ "${ret}" -ne "${1}" ]]; then
     >&2 echo "FAIL: Unexpected return code [saw: ${ret}, want: ${1}] building target //test/clippy:${2}"
-    >&2 echo "  Run \"bazel build //test/clippy:${2}\" to see the output"
+    >&2 echo "  Run \"bazel build //test/clippy:${2} ${@:3}\" to see the output"
     exit 1
   elif [[ $# -ge 3 ]] && [[ "${@:3}" == *"capture_clippy_output"* ]]; then
     # Make sure that content was written to the output file
@@ -40,10 +41,26 @@ function check_build_result() {
     else
       STATOPTS=(-c%s)
     fi
-    if [[ $(stat ${STATOPTS[@]} "${NEW_WORKSPACE}/bazel-bin/test/clippy/${2%_clippy}.clippy.out") == 0 ]]; then
+    # Because bad_dep_clippy itself produces an empty report, we must check its dependencies.
+    if [[ "${2}" == "bad_dep_clippy" ]]; then
+      # If we are traversing deps, check that the output for the dependency has been created
+      if [[ "${@:3}" == *"clippy_aspect_traverse_deps"* ]] && [[ $(stat ${STATOPTS[@]} "${NEW_WORKSPACE}/bazel-bin/test/clippy/bad_library.clippy.out") == 0 ]]; then
+        >&2 echo "FAIL: Output of dependency bad_library wasn't written to out file building target //test/clippy:${2}"
+        >&2 echo "  Output file: ${NEW_WORKSPACE}/bazel-bin/test/clippy/bad_library.clippy.out"
+        >&2 echo "  Run \"bazel build //test/clippy:${2} ${@:3}\" to see the output"
+      fi
+      if [[ -f "${NEW_WORKSPACE}/bazel-bin/test/clippy/${2%_clippy}.clippy.out" ]]; then
+        >&2 echo "FAIL: Output wasn't written to out file building target //test/clippy:${2}"
+        >&2 echo "  Output file: ${NEW_WORKSPACE}/bazel-bin/test/clippy/${2%_clippy}.clippy.out"
+        >&2 echo "  Run \"bazel build //test/clippy:${2} ${@:3}\" to see the output"
+      fi
+    elif [[ $(stat ${STATOPTS[@]} "${NEW_WORKSPACE}/bazel-bin/test/clippy/${2%_clippy}.clippy.out") == 0 ]]; then
       >&2 echo "FAIL: Output wasn't written to out file building target //test/clippy:${2}"
       >&2 echo "  Output file: ${NEW_WORKSPACE}/bazel-bin/test/clippy/${2%_clippy}.clippy.out"
-      >&2 echo "  Run \"bazel build //test/clippy:${2}\" to see the output"
+      >&2 echo "  Run \"bazel build //test/clippy:${2} ${@:3}\" to see the output"
+      find -L . -name "${2%_clippy}.clippy.out"
+      stat ${STATOPTS[@]} "${NEW_WORKSPACE}/bazel-bin/test/clippy/${2%_clippy}.clippy.out"
+
       exit 1
     else
       echo "OK"
@@ -58,6 +75,7 @@ function test_all() {
   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 TRAVERSE_DEPS="--@rules_rust//rust/settings:experimental_clippy_aspect_traverse_deps=True"
 
   mkdir -p "${NEW_WORKSPACE}/test/clippy" && \
   cp -r test/clippy/* "${NEW_WORKSPACE}/test/clippy/" && \
@@ -105,6 +123,8 @@ EOF
   check_build_result $BUILD_OK ok_proc_macro_clippy
   check_build_result $BUILD_FAILED bad_binary_clippy
   check_build_result $BUILD_FAILED bad_library_clippy
+  check_build_result $BUILD_OK bad_dep_clippy
+  check_build_result $BUILD_FAILED bad_dep_clippy $TRAVERSE_DEPS
   check_build_result $BUILD_FAILED bad_shared_library_clippy
   check_build_result $BUILD_FAILED bad_static_library_clippy
   check_build_result $BUILD_FAILED bad_test_clippy
@@ -114,6 +134,8 @@ EOF
   # should succeed.
   check_build_result $BUILD_OK bad_binary_clippy $CAPTURE_OUTPUT
   check_build_result $BUILD_OK bad_library_clippy $CAPTURE_OUTPUT
+  check_build_result $BUILD_OK bad_dep_clippy $CAPTURE_OUTPUT
+  check_build_result $BUILD_OK bad_dep_clippy $CAPTURE_OUTPUT $TRAVERSE_DEPS
   check_build_result $BUILD_OK bad_shared_library_clippy $CAPTURE_OUTPUT
   check_build_result $BUILD_OK bad_static_library_clippy $CAPTURE_OUTPUT
   check_build_result $BUILD_OK bad_test_clippy $CAPTURE_OUTPUT