From ed64716a87d0ab3a7ad02a3c0742a084b256930a Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Sat, 10 Jul 2021 09:22:07 -0700 Subject: [PATCH] Added support for `noclippy` tag (#824) * Added support for `noclippy` tag * Regenerate documentation * Added regression tests for `noclippy` tag. * Fixed typo * Fixed bad cherry-pick --- .bazelci/presubmit.yml | 27 +++++++++++++++---------- .bazelrc | 4 ++++ docs/rust_clippy.md | 2 ++ docs/rust_clippy.vm | 2 ++ examples/.bazelrc | 4 ++++ examples/crate_universe/.bazelrc | 4 ++++ rust/private/clippy.bzl | 12 ++++++++--- test/clippy/BUILD.bazel | 10 +++++++++- test/clippy/clippy_failure_test.sh | 32 ++++++++++++++++++++++++++++++ 9 files changed, 82 insertions(+), 15 deletions(-) diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index a3a8ea0fd6..96fb068dc6 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -17,6 +17,7 @@ tasks: - "//test/versioned_dylib:versioned_dylib_test" build_flags: - "--config=rustfmt" + - "--config=clippy" ubuntu2004: name: "Minimum Supported Version" bazel: "3.5.0" @@ -24,6 +25,7 @@ tasks: test_targets: *default_linux_targets build_flags: - "--config=rustfmt" + - "--config=clippy" macos: osx_targets: &osx_targets - "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245 @@ -35,6 +37,7 @@ tasks: test_targets: *osx_targets build_flags: - "--config=rustfmt" + - "--config=clippy" rbe_ubuntu1604: test_targets: - "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245 @@ -51,10 +54,12 @@ tasks: - "-@examples//ffi/rust_calling_c:matrix_dylib_test" build_flags: - "--config=rustfmt" + - "--config=clippy" windows: build_flags: - "--enable_runfiles" # this is not enabled by default on windows and is necessary for the cargo build scripts - "--config=rustfmt" + - "--config=clippy" windows_targets: &windows_targets - "--" # Allows negative patterns; hack for https://github.com/bazelbuild/continuous-integration/pull/245 - "..." @@ -81,6 +86,7 @@ tasks: - //... build_flags: - "--config=rustfmt" + - "--config=clippy" docs_linux: name: Docs platform: ubuntu1804 @@ -89,20 +95,11 @@ tasks: - //... run_targets: - "//:test_docs" - clippy_examples: - name: Clippy on Examples - platform: ubuntu1804 - working_directory: examples - build_flags: - - "--aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect" - - "--output_groups=clippy_checks" - build_targets: - - //... clippy_failure: name: Negative Clippy Tests platform: ubuntu1804 - shell_commands: - - ./test/clippy/clippy_failure_test.sh + run_targets: + - "//test/clippy:clippy_failure_test" rustfmt_failure: name: Negative Rustfmt Tests platform: ubuntu2004 @@ -112,6 +109,8 @@ tasks: name: Ubuntu 20.04 with Clang platform: ubuntu2004 build_flags: + - "--config=rustfmt" + - "--config=clippy" - "--repo_env=CC=clang" # TODO(hlopko): Make this work (some tests were failing) # - "--linkopt=-fuse-ld=lld" @@ -127,6 +126,9 @@ tasks: - "//..." test_targets: - "//..." + build_flags: + - "--config=rustfmt" + - "--config=clippy" crate_universe_rbe_ubuntu1604: name: Crate Universe Examples platform: rbe_ubuntu1604 @@ -139,6 +141,7 @@ tasks: - "//..." build_flags: - "--config=rustfmt" + - "--config=clippy" crate_universe_examples_macos: name: Crate Universe Examples platform: macos @@ -151,6 +154,7 @@ tasks: - "//..." build_flags: - "--config=rustfmt" + - "--config=clippy" crate_universe_examples_windows: name: Crate Universe Examples platform: windows @@ -160,6 +164,7 @@ tasks: build_flags: - "--enable_runfiles" # this is not enabled by default on windows and is necessary for the cargo build scripts - "--config=rustfmt" + - "--config=clippy" crate_universe_windows_targets: &crate_universe_windows_targets - "//..." # TODO: There are windows specific build issues in the generated diff --git a/.bazelrc b/.bazelrc index db9695ba08..c54ffa8be3 100644 --- a/.bazelrc +++ b/.bazelrc @@ -4,3 +4,7 @@ # Enable rustfmt for all targets in the workspace build:rustfmt --aspects=//rust:defs.bzl%rustfmt_aspect build:rustfmt --output_groups=+rustfmt_checks + +# Enable clippy for all targets in the workspace +build:clippy --aspects=//rust:defs.bzl%rust_clippy_aspect +build:clippy --output_groups=+clippy_checks diff --git a/docs/rust_clippy.md b/docs/rust_clippy.md index 8646a5c978..16addd8d37 100644 --- a/docs/rust_clippy.md +++ b/docs/rust_clippy.md @@ -27,6 +27,8 @@ 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 + ## rust_clippy diff --git a/docs/rust_clippy.vm b/docs/rust_clippy.vm index e059e8d136..d444a04543 100644 --- a/docs/rust_clippy.vm +++ b/docs/rust_clippy.vm @@ -20,3 +20,5 @@ 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 diff --git a/examples/.bazelrc b/examples/.bazelrc index a61cbe3c11..7dcdbcf2bb 100644 --- a/examples/.bazelrc +++ b/examples/.bazelrc @@ -4,3 +4,7 @@ # Enable rustfmt for all targets in the workspace build:rustfmt --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect build:rustfmt --output_groups=+rustfmt_checks + +# Enable clippy for all targets in the workspace +build:clippy --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect +build:clippy --output_groups=+clippy_checks diff --git a/examples/crate_universe/.bazelrc b/examples/crate_universe/.bazelrc index a61cbe3c11..7dcdbcf2bb 100644 --- a/examples/crate_universe/.bazelrc +++ b/examples/crate_universe/.bazelrc @@ -4,3 +4,7 @@ # Enable rustfmt for all targets in the workspace build:rustfmt --aspects=@rules_rust//rust:defs.bzl%rustfmt_aspect build:rustfmt --output_groups=+rustfmt_checks + +# Enable clippy for all targets in the workspace +build:clippy --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect +build:clippy --output_groups=+clippy_checks diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl index 913accd39e..1cccc87800 100644 --- a/rust/private/clippy.bzl +++ b/rust/private/clippy.bzl @@ -23,11 +23,12 @@ load( ) load("//rust/private:utils.bzl", "determine_output_hash", "find_cc_toolchain", "find_toolchain") -def _get_clippy_ready_crate_info(target): +def _get_clippy_ready_crate_info(target, aspect_ctx): """Check that a target is suitable for clippy and extract the `CrateInfo` provider from it. Args: target (Target): The target the aspect is running on. + aspect_ctx (ctx, optional): The aspect's context object. Returns: CrateInfo, optional: A `CrateInfo` provider if clippy should be run or `None`. @@ -37,6 +38,10 @@ def _get_clippy_ready_crate_info(target): if target.label.workspace_root.startswith("external"): return None + # Targets annotated with `noclippy` will not be formatted + if aspect_ctx and "noclippy" in aspect_ctx.rule.attr.tags: + return None + # Obviously ignore any targets that don't contain `CrateInfo` if rust_common.crate_info not in target: return None @@ -44,7 +49,7 @@ def _get_clippy_ready_crate_info(target): return target[rust_common.crate_info] def _clippy_aspect_impl(target, ctx): - crate_info = _get_clippy_ready_crate_info(target) + crate_info = _get_clippy_ready_crate_info(target, ctx) if not crate_info: return [] @@ -189,7 +194,8 @@ $ bazel build --aspects=@rules_rust//rust:defs.bzl%rust_clippy_aspect \ ) def _rust_clippy_rule_impl(ctx): - files = depset([], transitive = [dep[OutputGroupInfo].clippy_checks for dep in ctx.attr.deps]) + clippy_ready_targets = [dep for dep in ctx.attr.deps if "clippy_checks" in dir(dep[OutputGroupInfo])] + files = depset([], transitive = [dep[OutputGroupInfo].clippy_checks for dep in clippy_ready_targets]) return [DefaultInfo(files = files)] rust_clippy = rule( diff --git a/test/clippy/BUILD.bazel b/test/clippy/BUILD.bazel index 29e6e3050d..c93d849f8a 100644 --- a/test/clippy/BUILD.bazel +++ b/test/clippy/BUILD.bazel @@ -1,5 +1,5 @@ load( - "//rust:rust.bzl", + "@rules_rust//rust:rust.bzl", "rust_binary", "rust_clippy", "rust_library", @@ -50,18 +50,21 @@ rust_binary( name = "bad_binary", srcs = ["bad_src/main.rs"], edition = "2018", + tags = ["noclippy"], ) rust_library( name = "bad_library", srcs = ["bad_src/lib.rs"], edition = "2018", + tags = ["noclippy"], ) rust_test( name = "bad_test", srcs = ["bad_src/lib.rs"], edition = "2018", + tags = ["noclippy"], ) # Clippy analysis of failing targets. @@ -84,3 +87,8 @@ rust_clippy( tags = ["manual"], deps = [":bad_test"], ) + +sh_binary( + name = "clippy_failure_test", + srcs = ["clippy_failure_test.sh"], +) diff --git a/test/clippy/clippy_failure_test.sh b/test/clippy/clippy_failure_test.sh index b6c00028db..79dae7b5b4 100755 --- a/test/clippy/clippy_failure_test.sh +++ b/test/clippy/clippy_failure_test.sh @@ -7,6 +7,13 @@ set -euo pipefail +if [[ -z "${BUILD_WORKSPACE_DIRECTORY:-}" ]]; then + echo "This script should be run under Bazel" + exit 1 +fi + +cd "${BUILD_WORKSPACE_DIRECTORY}" + # Executes a bazel build command and handles the return value, exiting # upon seeing an error. # @@ -31,6 +38,31 @@ function test_all() { local -r BUILD_FAILED=1 local -r TEST_FAIL=3 + temp_dir="$(mktemp -d -t ci-XXXXXXXXXX)" + new_workspace="${temp_dir}/rules_rust_test_clippy" + + mkdir -p "${new_workspace}/test/clippy" && \ + cp -r test/clippy/* "${new_workspace}/test/clippy/" && \ + cat << EOF > "${new_workspace}/WORKSPACE.bazel" +workspace(name = "rules_rust_test_clippy") +local_repository( + name = "rules_rust", + path = "${BUILD_WORKSPACE_DIRECTORY}", +) +load("@rules_rust//rust:repositories.bzl", "rust_repositories") +rust_repositories() +EOF + + # Drop the 'noclippy' tags + if [ "$(uname)" == "Darwin" ]; then + SEDOPTS=(-i '' -e) + else + SEDOPTS=(-i) + fi + sed ${SEDOPTS[@]} 's/"noclippy"//' "${new_workspace}/test/clippy/BUILD.bazel" + + pushd "${new_workspace}" + check_build_result $BUILD_OK ok_binary_clippy check_build_result $BUILD_OK ok_library_clippy check_build_result $BUILD_OK ok_test_clippy