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

@@ -17,13 +17,15 @@ tasks:
       - "//test/versioned_dylib:versioned_dylib_test"
     build_flags:
       - "--config=rustfmt"
+      - "--config=clippy"
   ubuntu2004:
     name: "Minimum Supported Version"
     bazel: "3.5.0"
     build_targets: *default_linux_targets
     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

.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

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
+
 <a id="#rust_clippy"></a>
 
 ## rust_clippy

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

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

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

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,14 +38,18 @@ 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
 
     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(

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"],
+)

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