diff --git a/docs/rust_clippy.vm b/docs/rust_clippy.vm index 2576175099..4d36036980 100644 --- a/docs/rust_clippy.vm +++ b/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 +``` diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl index dd787d3ff0..4401796207 100644 --- a/rust/private/clippy.bzl +++ b/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, diff --git a/rust/settings/BUILD.bazel b/rust/settings/BUILD.bazel index a93b91cc64..389a9817ea 100644 --- a/rust/settings/BUILD.bazel +++ b/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", @@ -69,6 +70,8 @@ clippy_output_diagnostics() clippy_toml() +clippy_aspect_traverse_deps() + codegen_units() collect_cfgs() diff --git a/rust/settings/settings.bzl b/rust/settings/settings.bzl index abedffa4c0..be35501a38 100644 --- a/rust/settings/settings.bzl +++ b/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 diff --git a/test/clippy/BUILD.bazel b/test/clippy/BUILD.bazel index ea449a0008..0dd164438f 100644 --- a/test/clippy/BUILD.bazel +++ b/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"], ) diff --git a/test/clippy/clippy_failure_tester.sh b/test/clippy/clippy_failure_tester.sh index 05ab24c6f9..52880833a4 100755 --- a/test/clippy/clippy_failure_tester.sh +++ b/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