Implement Clippy Aspect & Build Rule

.bazelci/presubmit.yml

@@ -50,3 +50,18 @@ tasks:
     working_directory: examples
     test_targets:
     - //...
+  clippy_examples:
+    name: Clippy on Examples
+    platform: ubuntu1804
+    working_directory: examples
+    build_flags:
+    - "--aspects=@io_bazel_rules_rust//rust:rust.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
+

examples/ffi/rust_calling_c/src/matrix.rs

@@ -45,7 +45,7 @@ impl Matrix {
             if matrix.is_null() {
                 panic!("Failed to allocate Matrix.");
             }
-            Matrix { matrix: matrix }
+            Matrix { matrix }
         }
     }
 

examples/proto/helloworld/greeter_server/greeter_server.rs

@@ -36,7 +36,7 @@ impl Greeter for GreeterImpl {
 
 fn main() {
     let mut server = grpc::ServerBuilder::<tls_api_stub::TlsAcceptor>::new();
-    let port = u16::from_str(&env::args().nth(1).unwrap_or("50051".to_owned())).unwrap();
+    let port = u16::from_str(&env::args().nth(1).unwrap_or_else(|| "50051".to_owned())).unwrap();
     server.http.set_port(port);
     server.add_service(GreeterServer::new_service_def(GreeterImpl));
     server.http.set_cpu_pool_threads(4);

examples/proto/helloworld/helloworld_test.rs

@@ -56,17 +56,19 @@ impl ServerInfo {
                     .expect("Waiting for server startup");
                 line = line.trim().to_owned();
                 if line.starts_with(port_prefix) {
-                    port = u16::from_str(&line[port_prefix.len()..]).expect(&format!(
-                        "Invalid port number {}",
-                        &line[port_prefix.len()..]
-                    ))
+                    port = u16::from_str(&line[port_prefix.len()..]).unwrap_or_else(|_|
+                        panic!(
+                            "Invalid port number {}",
+                            &line[port_prefix.len()..]
+                        )
+                    )
                 }
             }
         }
         println!("Started server on port {}", port);
         ServerInfo {
             process: c,
-            port: port,
+            port,
         }
     }
 

rust/private/clippy.bzl

@@ -0,0 +1,215 @@
+# Copyright 2020 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load(
+    "@io_bazel_rules_rust//rust:private/rustc.bzl",
+    "CrateInfo",
+    "collect_deps",
+    "collect_inputs",
+    "construct_arguments",
+    "construct_compile_command",
+)
+load(
+    "@io_bazel_rules_rust//rust:private/rust.bzl",
+    "crate_root_src",
+)
+load("@io_bazel_rules_rust//rust:private/utils.bzl", "find_toolchain")
+
+_rust_extensions = [
+    "rs",
+]
+
+def _is_rust_target(srcs):
+    return any([src.extension in _rust_extensions for src in srcs])
+
+def _rust_sources(target, rule):
+    srcs = []
+    if "srcs" in dir(rule.attr):
+        srcs += [f for src in rule.attr.srcs for f in src.files.to_list()]
+    if "hdrs" in dir(rule.attr):
+        srcs += [f for hdr in rule.attr.hdrs for f in hdr.files.to_list()]
+    return [src for src in srcs if src.extension in _rust_extensions]
+
+def _clippy_aspect_impl(target, ctx):
+    if CrateInfo not in target:
+        return []
+    rust_srcs = _rust_sources(target, ctx.rule)
+    if rust_srcs == []:
+        return []
+
+    toolchain = find_toolchain(ctx)
+    root = crate_root_src(ctx.rule.attr, srcs = rust_srcs)
+    crate_info = target[CrateInfo]
+
+    dep_info, build_info = collect_deps(
+        ctx.label,
+        crate_info.deps,
+        crate_info.proc_macro_deps,
+        crate_info.aliases,
+        toolchain,
+    )
+
+    compile_inputs, out_dir = collect_inputs(
+        ctx,
+        ctx.rule.file,
+        ctx.rule.files,
+        toolchain,
+        crate_info,
+        dep_info,
+        build_info
+    )
+
+    args, env = construct_arguments(
+        ctx,
+        ctx.rule.file,
+        toolchain,
+        crate_info,
+        dep_info,
+        output_hash = repr(hash(root.path)),
+        rust_flags = [])
+
+    # A marker file indicating clippy has executed successfully.
+    # This file is necessary because "ctx.actions.run" mandates an output.
+    clippy_marker = ctx.actions.declare_file(ctx.label.name + "_clippy.ok")
+
+    command = construct_compile_command(
+        ctx,
+        toolchain.clippy_driver.path,
+        toolchain,
+        crate_info,
+        build_info,
+        out_dir,
+    ) + (" && touch %s" % clippy_marker.path)
+
+    # Deny the default-on clippy warning levels.
+    #
+    # If these are left as warnings, then Bazel will consider the execution
+    # result of the aspect to be "success", and Clippy won't be re-triggered
+    # unless the source file is modified.
+    args.add("-Dclippy::style")
+    args.add("-Dclippy::correctness");
+    args.add("-Dclippy::complexity");
+    args.add("-Dclippy::perf");
+
+    ctx.actions.run_shell(
+        command = command,
+        inputs = compile_inputs,
+        outputs = [clippy_marker],
+        env = env,
+        tools = [toolchain.clippy_driver],
+        arguments = [args],
+        mnemonic = "Clippy",
+    )
+
+    return [
+        OutputGroupInfo(clippy_checks = depset([clippy_marker])),
+    ]
+
+# Example: Run the clippy checker on all targets in the codebase.
+#   bazel build --aspects=@io_bazel_rules_rust//rust:rust.bzl%rust_clippy_aspect \
+#               --output_groups=clippy_checks \
+#               //...
+rust_clippy_aspect = aspect(
+    fragments = ["cpp"],
+    host_fragments = ["cpp"],
+    attrs = {
+        "_cc_toolchain": attr.label(
+            default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"),
+        ),
+    },
+    toolchains = [
+        "@io_bazel_rules_rust//rust:toolchain",
+        "@bazel_tools//tools/cpp:toolchain_type"
+    ],
+    implementation = _clippy_aspect_impl,
+    doc = """
+Executes the clippy checker on specified targets.
+
+This aspect applies to existing rust_library, rust_test, and rust_binary rules.
+
+As an example, if the following is defined in `hello_lib/BUILD`:
+
+```python
+package(default_visibility = ["//visibility:public"])
+
+load("@io_bazel_rules_rust//rust:rust.bzl", "rust_library", "rust_test")
+
+rust_library(
+    name = "hello_lib",
+    srcs = ["src/lib.rs"],
+)
+
+rust_test(
+    name = "greeting_test",
+    srcs = ["tests/greeting.rs"],
+    deps = [":hello_lib"],
+)
+```
+
+Then the targets can be analyzed with clippy using the following command:
+
+$ bazel build --aspects=@io_bazel_rules_rust//rust:rust.bzl%rust_clippy_aspect \
+              --output_groups=clippy_checks //hello_lib:all
+""",
+
+)
+
+def _rust_clippy_rule_impl(ctx):
+    files = depset([], transitive = [dep[OutputGroupInfo].clippy_checks for dep in ctx.attr.deps])
+    return [DefaultInfo(files = files)]
+
+rust_clippy = rule(
+    implementation = _rust_clippy_rule_impl,
+    attrs = {
+        'deps': attr.label_list(aspects = [rust_clippy_aspect]),
+    },
+    doc = """
+Executes the clippy checker on a specific target.
+
+Similar to `rust_clippy_aspect`, but allows specifying a list of dependencies
+within the build system.
+
+For example, given the following example targets:
+
+```python
+package(default_visibility = ["//visibility:public"])
+
+load("@io_bazel_rules_rust//rust:rust.bzl", "rust_library", "rust_test")
+
+rust_library(
+    name = "hello_lib",
+    srcs = ["src/lib.rs"],
+)
+
+rust_test(
+    name = "greeting_test",
+    srcs = ["tests/greeting.rs"],
+    deps = [":hello_lib"],
+)
+```
+
+Rust clippy can be set as a build target with the following:
+
+```python
+rust_clippy(
+    name = "hello_library_clippy",
+    testonly = True,
+    deps = [
+        ":hello_lib",
+        ":greeting_test",
+    ],
+)
+```
+""",
+)

rust/private/rust.bzl

@@ -71,24 +71,28 @@ def _determine_lib_name(name, crate_type, toolchain, lib_hash = ""):
         extension = extension,
     )
 
-def _get_edition(ctx, toolchain):
-    if getattr(ctx.attr, "edition"):
-        return ctx.attr.edition
+def get_edition(attr, toolchain):
+    if getattr(attr, "edition"):
+        return attr.edition
     else:
         return toolchain.default_edition
 
-def _crate_root_src(ctx, file_name = "lib.rs"):
+def crate_root_src(attr, srcs, file_name = "lib.rs"):
     """Finds the source file for the crate root."""
-    srcs = ctx.files.srcs
 
-    crate_root = (
-        ctx.file.crate_root or
-        (srcs[0] if len(srcs) == 1 else None) or
-        _shortest_src_with_basename(srcs, file_name) or
-        _shortest_src_with_basename(srcs, ctx.attr.name + ".rs")
-    )
+    crate_root = None
+    if hasattr(attr, "crate_root"):
+        if attr.crate_root:
+            crate_root = attr.crate_root.files.to_list()[0]
+
+    if not crate_root:
+        crate_root = (
+            (srcs[0] if len(srcs) == 1 else None) or
+            _shortest_src_with_basename(srcs, file_name) or
+            _shortest_src_with_basename(srcs, attr.name + ".rs")
+        )
     if not crate_root:
-        file_names = [file_name, ctx.attr.name + ".rs"]
+        file_names = [file_name, attr.name + ".rs"]
         fail("No {} source file found.".format(" or ".join(file_names)), "srcs")
     return crate_root
 
@@ -105,7 +109,7 @@ def _shortest_src_with_basename(srcs, basename):
 
 def _rust_library_impl(ctx):
     # Find lib.rs
-    lib_rs = _crate_root_src(ctx)
+    lib_rs = crate_root_src(ctx.attr, ctx.files.srcs)
 
     toolchain = find_toolchain(ctx)
 
@@ -133,7 +137,7 @@ def _rust_library_impl(ctx):
             proc_macro_deps = ctx.attr.proc_macro_deps,
             aliases = ctx.attr.aliases,
             output = rust_lib,
-            edition = _get_edition(ctx, toolchain),
+            edition = get_edition(ctx.attr, toolchain),
             rustc_env = ctx.attr.rustc_env,
         ),
         output_hash = output_hash,
@@ -156,13 +160,13 @@ def _rust_binary_impl(ctx):
         crate_info = CrateInfo(
             name = crate_name,
             type = crate_type,
-            root = _crate_root_src(ctx, "main.rs"),
+            root = crate_root_src(ctx.attr, ctx.files.srcs, file_name = "main.rs"),
             srcs = ctx.files.srcs,
             deps = ctx.attr.deps,
             proc_macro_deps = ctx.attr.proc_macro_deps,
             aliases = ctx.attr.aliases,
             output = output,
-            edition = _get_edition(ctx, toolchain),
+            edition = get_edition(ctx.attr, toolchain),
             rustc_env = ctx.attr.rustc_env,
         ),
     )
@@ -205,13 +209,13 @@ def _rust_test_common(ctx, test_binary):
         target = CrateInfo(
             name = test_binary.basename,
             type = "lib",
-            root = _crate_root_src(ctx),
+            root = crate_root_src(ctx.attr, ctx.files.srcs),
             srcs = ctx.files.srcs,
             deps = ctx.attr.deps,
             proc_macro_deps = ctx.attr.proc_macro_deps,
             aliases = ctx.attr.aliases,
             output = test_binary,
-            edition = _get_edition(ctx, toolchain),
+            edition = get_edition(ctx.attr, toolchain),
             rustc_env = ctx.attr.rustc_env,
         )