Chromium Docs

Clippy in Chromium

How to run Clippy lints against Rust code in Chromium

When building locally, Clippy is disabled by default to avoid adding build overhead to Chromium engineers primarily working with C, C++, or Java.

However, Clippy warnings are enforced by the CQ. Therefore, it is strongly recommended that engineers working with Rust code enable Clippy warnings by setting enable_rust_clippy = true in their args.gn.

Doing so allows you to fix Clippy warnings as they occur, rather than waiting to discover them until after uploading a CL and running it through the CQ. It also lets you verify that you've fixed all the problems when rebuilding locally.

Chromium policy

Chromium has a relaxed, non-strict Clippy policy. Clippy is a tool that hopefully helps to improve the quality of Chromium code, but ultimately we trust the judgement of Chromium engineers.

  • We hope that most Clippy violations point out actual opportunities for improving the code (e.g. catching common mistakes, teaching idiomatic Rust patterns, etc.).
  • If a Clippy lint doesn’t seem useful in a certain scenario, then use your best judgement and feel free to suppress the lint using the #[allow(...)] macro.
    • We recommend leaving a short comment that will help future readers understand why the lint was suppressed.
    • We recommend consulting with the @security team before suppressing unsafe-related lints like clippy::missing_safety_doc or clippy::clippy::undocumented_unsafe_blocks.
    • We recommend applying suppression to a single item, but it is okay to suppress at a module, or crate level if it makes sense (e.g. if all functions in a given test module have to violate a certain lint)
  • If a Clippy lint is frequently suppressed, then one option on the table is disabling the lint globally in Chromium - please talk with @chrome-rust if you find yourself oftentimes disabling a particular lint.

For a list of Clippy lints that apply by default to Chromium targets please see the //build/config/compiler:default_clippy_lints target.

Known issues

Clippy coverage in Chromium CQ/CI

Clippy is enabled on a subset of CQ bots: android-arm64-rel, linux-chromeos-rel, linux-rel, mac-rel, win-rel.

TODO(https://crbug.com/41484295): Add other bots and target platforms if additional code needs to be covered by Clippy (ios-simulator?).

Future work

Next steps:

Automatically applying fix suggestions

IDE integration

TODO: See if/how this works + write this section.

Command-line

When build fails because of Clippy, then a note toward the end spells out a command for using build/rust/apply_fixes.py to apply machine-applicable fix suggestions from this particular Clippy invocation.

Example:

$ autoninja -C out/rel ...
...
error: this expression creates a reference which is immediately dereferenced by the compiler
   --> ../../third_party/blink/renderer/core/xml/parser/xml_ffi.rs:271:33
    |
271 |                 q_name.push_str(&pref);
    |                                 ^^^^^ help: change this to: `pref`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
    = note: `-D clippy::needless-borrow` implied by `-D clippy::style`
    = help: to override `-D clippy::style` add `#[allow(clippy::needless_borrow)]`

error: aborting due to 1 previous error

NOTE: See `//docs/rust/clippy.md` for more Clippy info.
NOTE: To apply machine-applicable fix suggestions (if any) run: build/rust/apply_fixes.py out/rel clippy-driver obj/third_party/blink/renderer/core/xml_ffi.rustflags.json
...

$ build/rust/apply_fixes.py out/rel clippy-driver obj/third_party/blink/renderer/core/xml_ffi.rustflags.json
Invoking clippy-driver to repro the build problem...
Looking for machine-applicable fixes...
Applying fixes to `../../third_party/blink/renderer/core/xml/parser/xml_ffi.rs`...
  Applying a fix on line 271...

$ git diff
diff --git a/third_party/blink/renderer/core/xml/parser/xml_ffi.rs b/third_party/blink/renderer/core/xml/parser/xml_ffi.rs
index 707c74bf87784..b02eaf1266d2c 100644
--- a/third_party/blink/renderer/core/xml/parser/xml_ffi.rs
+++ b/third_party/blink/renderer/core/xml/parser/xml_ffi.rs
@@ -268,7 +268,7 @@ fn attributes_next<'a>(
         match &name.prefix {
             Some(pref) => {
                 let mut q_name = String::with_capacity(pref.len() + 1 + name.local_name.len());
-                q_name.push_str(&pref);
+                q_name.push_str(pref);
                 q_name.push(':');
                 q_name.push_str(&name.local_name);
                 attribute_view.as_mut().Populate(

Implementation detail: When exactly does Clippy run?

When enable_rust_clippy = true then by default each Rust target (e.g. rust_static_library("foo")) will have a validations dependency edge to an action("foo_clippy") target. This means that foo_clippy will be invoked whenever foo is built (when ninja is asked to build foo directly or when foo is transitively needed). OTOH, targets that depend on foo will not be blocked waiting for completion of foo_clippy.

If you want to minimize overhead of Clippy even further, you can consider setting enable_rust_clippy_eager = false. This removes the validations dependency edge described above, so that building foo no longer triggers foo_clippy. This mode is discouraged, because it means that Clippy failures will go undetected until a full build (e.g. until CQ). Nevertheless, this mode may be useful when wanting to quickly iterate on code.

References