-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[Roadmap] False Positive Rate #6623
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
Comments
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 🙃 |
Perhaps a str slicing lint would help us find the lints that are in need of test cases containing unicode identifiers... |
new lint: string-slice This is a restriction lint to highlight code that should have tests containing non-ascii characters. See #6623. changelog: new lint: [`string-slice`]
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. |
Good news: @xFrednet is currently working on this: rust-lang/rust#87835 |
Add Clippy version to Clippy's lint list Hey, hey, the semester is finally over, and I wanted to get back into hacking on Clippy. It has also been some time since our metadata collection monster has been feed. So, this PR adds a new attribute `clippy::version` to document which version a lint was stabilized. I considered using `git blame` but that would be very hacky and probably not accurate. I'm also thinking that this attribute can be used to have a `clippy::nightly` lint group which is allow-by-default that delays setting the actual lint group until the defined version is reached. Just something to consider regarding #6623 🙃 This PR only adds the version to 4 lints to keep it reviewable. I'll do a followup PR to add the version to other lints if the implementation is accepted 🙃  Also, mobile approved xD  --- r? `@flip1995` cc: #7172 closes: #6492 changelog: [Clippy's lint list](https://rust-lang.github.io/rust-clippy/master/index.html) now displays the version a lint was added. 🎉 --- Example lint declaration after this update: ```rs declare_clippy_lint! { /// [...] /// /// ### Example /// ```rust /// // Bad /// let x = 3.14; /// // Good /// let x = std::f32::consts::PI; /// ``` #[clippy::version = "pre 1.29.0"] pub APPROX_CONSTANT, correctness, "the approximate of a known float constant (in `std::fXX::consts`)" } ```
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. 🙃 |
To add an additional datapoint, I have run clippy on all files (standalone) in the rustc repo [1]
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. |
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? |
I think none of those failed to compile in the first place because I don't pass |
I uploaded the errors.json which contains the files and the clippy lints that failed to apply with them. |
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. |
the newly stabalized |
I will close this issue, as the only real open point is to work out if we want nightly lints. But that is its own whole project (that needs to be done in rustc). So we done it! Another point checked of the 2021 roadmap 😁 |
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:
cargo dev-lintcheck
for Clippy (add "cargo dev crater" to run clippy on a fixed set of crates and diff the lint warnings #6469)CFP: Ask people using Clippy to submit their crates, so we can check new lints on them before releasing the lints to stablecrater
tool on peoples private code / CI to get even more inputThe text was updated successfully, but these errors were encountered: