Clippy lints will be checked by fx build and will block CLs

[Adam Perry]

TLDR: Adding #![warn(clippy::foo)] or #![deny(clippy::foo)] to a module will soon generate warnings or errors during fx build without needing to run fx clippy on its own. The deny_warnings GN arg will work the same for Clippy lints as it does for rustc lints.

Earlier this year, the Rust on Fuchsia team enabled Clippy in our build so that lint warnings would appear in Tricium/CQ. Many thanks to Joseph Ryan for all the work he put in to make it possible! Since then, it has been possible to add lint-enabling attributes to Rust crates so that they could be surfaced during code review, but reproducing the warnings locally required running a separate command, fx clippy. This worked well to smoothly roll out support without disrupting anyone's workflows while still being able to choose certain lints that would always produce errors.

As part of an effort to improve reliability in Component Manager the CF team is considering opting its code into a stricter set of lints, but we expect it would be awkward to have a workflow where developers either must run a separate command from fx build to see the errors or only discover them when uploading a CL and running it through infra. Addressing this awkwardness requires running Clippy lints by default in builds, and ensuring that they behave consistently with other warnings/errors that are configured in source code the same way. In turn, running Clippy in the build by default requires that any existing lint violations are resolved or suppressed.

Today I'm going to land a CL which addresses all existing lint violations, either by running rustfix, fixing the easy ones manually, or adding an #[allow(...)] attribute. In the coming days I'll land a follow-up that actually flips the flags to include Clippy in builds by default. Once that's done I'll reply to this thread to notify anyone interested, which will probably be a good time to double check the lint configuration for any crates you own.

After these changes, Clippy warnings should still appear in Tricium for faster feedback. Please file a bug against the Rust>tools>clippy component and ping me and Joseph Ryan if you encounter any issues.

Thanks!

Adam Perry on behalf of the Rust on Fuchsia Working Group

[Adam Perry]

The change to enforce Clippy lints during fx build has landed and seems like it stands a good chance of sticking. Now is probably a good time to double-check which lints you'd like enabled for your code!

A number of lint violations I "fixed" in my quick-and-dirty cleanup were in test code where it may not be as valuable to have guardrails against things like performance anti-patterns. For that kind of lint you might consider enabling it with #[cfg_attr(not(test), warn(clippy::foo))] so that the lint will only apply to non-test code.

--

Adam Perry

[Adam Perry]

I spoke too soon! This did end up racing with another CL, I'll update when a reland sticks.

--

Adam Perry

[Adam Perry]

This relanded last night and seems to have stuck! *Now* is probably a good time to check your lint configurations :).

--

Adam Perry