[Roadmap] False Positive Rate

[flip1995]

In the worst case, new lints are only available in nightly for 2 weeks, before
hitting beta and ultimately stable. This and the fact that fewer people use
nightly Rust nowadays makes it more probable that a lint with many FPs hits
stable. This leads to annoyed users, that will disable these new lints in the
best case and to more annoyed users, that will stop using Clippy in the worst.
A process should be developed and implemented to prevent this from happening.

#6429

Steps to completion:

• Implement crater aka cargo dev-lintcheck for Clippy (add "cargo dev crater" to run clippy on a fixed set of crates and diff the lint warnings #6469)
• Implement framework so that lints can be enabled in nightly only
  • Previous attempt to implement this in Clippy Implement framework so that lints can be enabled in nightly only #8211
• Define clear process for stabilizing lints
• CFP: Ask people using Clippy to submit their crates, so we can check new lints on them before releasing the lints to stable
• Make it easy to run the crater tool on peoples private code / CI to get even more input

[xFrednet]

I think it would be a good idea to add some crates to the lintcheck tool that use unicode identifiers. Looking and fixing #7869 made me realize how fast byte and character indices can be mixed up 🙃

[llogiq]

Perhaps a str slicing lint would help us find the lints that are in need of test cases containing unicode identifiers...

[EdJoPaTo]

Something I think that would be helpful to deal with false positives would be #3122. In case false positives are found they will be allowed short term.
In the long term this false positive will be fixed and the allow comment shouldn't be there anymore.

[flip1995]

Good news: @xFrednet is currently working on this: rust-lang/rust#87835

[xFrednet]

I would like to take a stab at the second point: "Implement framework so that lints can be enabled in nightly only". I have three ideas how this can be implemented, with advantages and drawbacks. I'll create an issue outlining the possibilities and to ask for feedback. 🙃

[matthiaskrgr]

To add an additional datapoint, I have run clippy on all files (standalone) in the rustc repo [1]
(basically: run cargo clippy --fix -Wall::lints on a file, check if rustfix fails to apply clippys suggestion, bisect the offending lint), and these are the lints and the number of times a applied lint suggestion caused a compilation failure (aka invalid suggestion)

      1       "--force-warn clippy::assign-op-pattern",
      1       "--force-warn clippy::blocks-in-if-conditions",
      1       "--force-warn clippy::bool-comparison",
      1       "--force-warn clippy::bool-to-int-with-if",
      1       "--force-warn clippy::borrow-deref-ref",
      1       "--force-warn clippy::cloned-instead-of-copied",
      1       "--force-warn clippy::derive-partial-eq-without-eq",
      1       "--force-warn clippy::excessive-precision",
      1       "--force-warn clippy::explicit-auto-deref",
      1       "--force-warn clippy::match-single-binding",
      1       "--force-warn clippy::needless-arbitrary-self-type",
      1       "--force-warn clippy::needless-for-each",
      1       "--force-warn clippy::needless-late-init",
      1       "--force-warn clippy::range-minus-one",
      1       "--force-warn clippy::range-plus-one",
      1       "--force-warn clippy::redundant-pattern-matching",
      1       "--force-warn clippy::ref-binding-to-reference",
      1       "--force-warn clippy::short-circuit-statement",
      1       "--force-warn clippy::single-component-path-imports",
      1       "--force-warn clippy::transmutes-expressible-as-ptr-casts",
      1       "--force-warn clippy::unnecessary-sort-by",
      2       "--force-warn clippy::cast-lossless",
      2       "--force-warn clippy::equatable-if-let",
      2       "--force-warn clippy::manual-async-fn",
      2       "--force-warn clippy::needless-lifetimes",
      2       "--force-warn clippy::redundant-closure",
      2       "--force-warn clippy::redundant-closure-call",
      2       "--force-warn clippy::redundant-pub-crate",
      2       "--force-warn clippy::unused-unit",
      3       "--force-warn clippy::needless-return",
      3       "--force-warn clippy::no-mangle-with-rust-abi",
      3       "--force-warn clippy::redundant-clone",
      3       "--force-warn clippy::trait-duplication-in-bounds",
      3       "--force-warn clippy::unnecessary-fold",
      3       "--force-warn clippy::unnested-or-patterns",
      4       "--force-warn clippy::unit-arg",
      4       "--force-warn clippy::unnecessary-operation",
      4       "--force-warn clippy::wildcard-imports",
      6       "--force-warn clippy::redundant-closure-for-method-calls",
      6       "--force-warn clippy::use-self",
     13       "--force-warn clippy::ptr-as-ptr",
     23       "--force-warn clippy::let-unit-value",
     40       "--force-warn clippy::borrow-as-ptr",

Note that this is by no means a complete picture, it only shows lints that can be applied automatically in the first place, and lints that are more likely to fire on code inside short snippets of rustcs unit tests.

[llogiq]

I wonder how many of those are in compile tests that would have failed to compile in the first place?

Do you have the raw data somewhere?

[matthiaskrgr]

I think none of those failed to compile in the first place because I don't pass --broken code flag to rustfix which is required to fix code that does not compile in the first place iirc..? I can't check in detail right now.

[matthiaskrgr]

I uploaded the errors.json which contains the files and the clippy lints that failed to apply with them.
The summary was obtained via cat errors.json | grep clippy:: | sort -n | uniq -c | sort -n
errors.json.tar.gz

[xFrednet]

Implement framework so that lints can be enabled in nightly only

I've tried to implement this in Clippy, but we determined, that this should be implemented in rustc instead. The compiler team requested us to create a MCP if we want this for rustc.

@Jarcho created a working implementation in rustc, but it was closed until an MCP was created and accepted: rust-lang/rust#109063.