Skip to content
Open
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
17 changes: 17 additions & 0 deletions docs/rust_clippy.vm
Original file line number Diff line number Diff line change
Expand Up @@ -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.
6 changes: 5 additions & 1 deletion rust/private/clippy.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 15 additions & 0 deletions rust/private/rust.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
58 changes: 58 additions & 0 deletions test/clippy/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
load("@bazel_skylib//rules:write_file.bzl", "write_file")
load(
"@rules_rust//rust:defs.bzl",
"rust_binary",
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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"],
Expand Down
17 changes: 16 additions & 1 deletion test/clippy/clippy_failure_tester.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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/" && \
Expand Down Expand Up @@ -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
Expand All @@ -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
1 change: 1 addition & 0 deletions test/clippy/target_config/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
exports_files(["clippy.toml"])
4 changes: 4 additions & 0 deletions test/clippy/target_config/clippy.toml
Original file line number Diff line number Diff line change
@@ -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
58 changes: 56 additions & 2 deletions test/unit/clippy/clippy_test.bzl
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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 = [
Expand All @@ -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",
],
)