rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.28k stars 1.52k forks source link

Please be more considerate when adding new lints with `warning` level or higher or strictly support a MSRV #7387

Open Matthias247 opened 3 years ago

Matthias247 commented 3 years ago

First of all: Thanks for all the work that goes into clippy - I think there are some really good things for newcomers which actively prevent bugs and help enforce a more common style.

I unfortunately have to admit that lately dealing with clippy got a lot more of a chore than a pleasure, and a major blocker to upgrading to new Rust versions. Whenever a new compiler is released, a couple of packages that I'm either actively maintaining or utilizing start to break due to new lints being added.

Fixing that requires spending additional effort when updating the compiler for all those dependencies. That effort would often be much better invested elsewhere than fixing nitpicks and is usually not accounted for on product roadmaps.

For some examples:

Now of course we could back to "if you are not happy, just disable clippy". That's an option, but I don't think its the right path to take since projects doing that would also lose the benefits of clippy.

I instead would like to encourage the time to think twice on whether something is actually really worth a warning or should be rather in the pedantic category - and to strictly enable new lints for projects which have opted into it by setting a clippy MSRV to an appropriate value.

Sorry if this comes a bit over as rant - it should not be! My main goal is to highlight that there is a huge cost of adding new lints outside of the clippy package which might not be seen when those are added. And I want to highlight that cost a bit more from a "consumer" point of view.

ghost commented 3 years ago

Now of course we could back to "if you are not happy, just disable clippy".

You should just globally allow individual lints that aren't worth fixing regardless of whether they are old or new. You don't have to choose between fixing every lint that is enabled by default or not using Clippy at all.

... and to strictly enable new lints for projects which have opted into it by setting a clippy MSRV to an appropriate value

MSRV doesn't work like that. MSRV doesn't disable new lints. It disables lints that require compiler features above the MSRV. A lint could be added to the next version of Clippy with MSRV 1.17 for example.

Even if there were an option to disable new lints sooner or later some of those lints would be 'stabilized' and then you'd have the same problem. It might still be a good idea though.

roblabla commented 3 years ago

You should just globally allow individual lints that aren't worth fixing regardless of whether they are old or new.

Some of us maintain multiple crates, or have workspaces (CC https://github.com/rust-lang/cargo/issues/5034). If it was simpler to "just globally allow" individual lints, this wouldn't be as much of a problem.

MSRV doesn't work like that.

I mean, it could? There could be a setting in Cargo.toml, say:

[metadata.clippy]
version = "1.53.0"

Such that any new lint or changed defaults that happened after the clippy bundled in release 1.53.0 would be invisible no matter the clippy version. Alternatively, it could work similarly to editions in Rust.

djc commented 3 years ago

Note that part of the solution here would be cargo clippy --fix, which works today on nightly (with -Z unstable-options). This makes it much easier to fix new lints.