Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for noclippy tag #824

Merged
merged 6 commits into from
Jul 10, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
27 changes: 16 additions & 11 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
- "..."
Expand All @@ -81,6 +86,7 @@ tasks:
- //...
build_flags:
- "--config=rustfmt"
- "--config=clippy"
docs_linux:
name: Docs
platform: ubuntu1804
Expand All @@ -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
Expand All @@ -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"
Expand All @@ -127,6 +126,9 @@ tasks:
- "//..."
test_targets:
- "//..."
build_flags:
- "--config=rustfmt"
- "--config=clippy"
crate_universe_rbe_ubuntu1604:
name: Crate Universe Examples
platform: rbe_ubuntu1604
Expand All @@ -139,6 +141,7 @@ tasks:
- "//..."
build_flags:
- "--config=rustfmt"
- "--config=clippy"
crate_universe_examples_macos:
name: Crate Universe Examples
platform: macos
Expand All @@ -151,6 +154,7 @@ tasks:
- "//..."
build_flags:
- "--config=rustfmt"
- "--config=clippy"
crate_universe_examples_windows:
name: Crate Universe Examples
platform: windows
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions docs/rust_clippy.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions docs/rust_clippy.vm
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions examples/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions examples/crate_universe/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 9 additions & 3 deletions rust/private/clippy.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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 []

Expand Down Expand Up @@ -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(
Expand Down
8 changes: 8 additions & 0 deletions test/clippy/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -84,3 +87,8 @@ rust_clippy(
tags = ["manual"],
deps = [":bad_test"],
)

sh_binary(
name = "clippy_failure_test",
srcs = ["clippy_failure_test.sh"],
)
32 changes: 32 additions & 0 deletions test/clippy/clippy_failure_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand All @@ -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
Expand Down