Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Matthias247 opened this issue Jun 22, 2021 · 3 comments

Comments

@Matthias247
Copy link

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
Copy link

ghost commented Jun 22, 2021

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
Copy link
Contributor

roblabla commented Jun 25, 2021

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 rust-lang/cargo#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
Copy link
Contributor

djc commented Jun 25, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants