lucet icon indicating copy to clipboard operation
lucet copied to clipboard

Decide whether to apply `clippy` to codebase

Open acfoltzer opened this issue 5 years ago • 4 comments

We of course don't need to adopt the suggestions whole cloth, but we should see whether we can get a good signal-to-noise ratio coming from Clippy.

acfoltzer avatar Apr 22 '19 18:04 acfoltzer

Just like rustfmt, clippy's behavior tend to change over time. New checks are constantly being added, and false positives are removed.

Clippy is a fantastic tool, and everybody should enable it in its text editor, but forcing the clippy output to be clean on make test would be annoying.

This would force people to use a specific version of the toolchain to work on Lucet. And this is not great.

Right now, the test suite cannot run unless a specific version of Rust is installed, which is neither the current stable one, nor the one provided by Linux distributions.

Just because of the rustfmt check.

And unlike rustfmt which is pretty stable, clippy is quite frequently broken on nightlies. But we shouldn't prevent people running rust-nightly, or a different version of clippy, from being able to seamlessly hack the Lucet code.

Clippy outputs quite a few interesting warnings (and, currently, will stop its analysis due to a fatal error in lucet-analyze). We should address them, and welcome any contribution that improves clippy-cleanliness.

But I think we shouldn't enforce it like we do with rustfmt.

jedisct1 avatar Apr 22 '19 18:04 jedisct1

I'm frustrated that rustfmt's behavior changes over time, and that our current install of it isn't on stable (because we decided to wait for 1.34.1).

Frank's recommendations for clippy use are spot on with my experiences.

pchickey avatar Apr 22 '19 18:04 pchickey

Looks like we are all on the same page :)

jedisct1 avatar Apr 23 '19 22:04 jedisct1

Going to keep this open, even though we aren't planning to include it in CI. I still want to see what it can find when we drive it manually.

acfoltzer avatar Apr 29 '19 18:04 acfoltzer