Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/rust_clippy.vm
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
27 changes: 25 additions & 2 deletions rust/private/clippy.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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),
Expand All @@ -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",
Expand All @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions rust/settings/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -69,6 +70,8 @@ clippy_output_diagnostics()

clippy_toml()

clippy_aspect_traverse_deps()

codegen_units()

collect_cfgs()
Expand Down
9 changes: 9 additions & 0 deletions rust/settings/settings.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 14 additions & 1 deletion test/clippy/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down Expand Up @@ -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"],
)

Expand Down
28 changes: 25 additions & 3 deletions test/clippy/clippy_failure_tester.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two are drive-by changes, but I realized we should have the flags if we want to have an easy repro.

exit 1
else
echo "OK"
Expand All @@ -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/" && \
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down