-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add Clippy like lint groups #16464
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
Add Clippy like lint groups #16464
Conversation
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
| | Group | Description | Default level | | ||
| |----------------------|------------------------------------------------------------------|---------------| | ||
| | `cargo::complexity` | code that does something simple but in a complex way | warn | | ||
| | `cargo::correctness` | code that is outright wrong or useless | deny | | ||
| | `cargo::nursery` | new lints that are still under development | allow | | ||
| | `cargo::pedantic` | lints which are rather strict or have occasional false positives | allow | | ||
| | `cargo::perf` | code that can be written to run faster | warn | | ||
| | `cargo::restriction` | lints which prevent the use of language and library features | allow | | ||
| | `cargo::style` | code that should be written in a more idiomatic way | warn | | ||
| | `cargo::suspicious` | code that is most likely wrong or useless | warn | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It is nice we have a similar set of groups as clippy's! Easy for users to learn.
- It might be a bit confusing since we don't have that many lints right now. And also once we stabilize one group its hard to revert back.
Anyway, this is fine when being as a nightly feature. we need to revisit when stabilization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: Dynamic groups (i.e., all or warnings) aren't supported at this time
I am a bit unsure whether we want to have cargo::all. clippy::all is not a real "all" (it includes only all lints that are on by default). So sometimes the name is a bit surprising. Anyhow, this is not a blocker for this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that we should not mirror clippy::all
5e8bd81 to
11324cd
Compare
This comment has been minimized.
This comment has been minimized.
11324cd to
085d9d1
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This PR overhauls how lint groups work within Cargo, aligning them with how Clippy handles groups1, all while simplifying the implementation (in my opinion). The first commit adds eight lint groups that mirror Clippy's groups, while the subsequent commits work towards matching how Clippy handles groups, as well as improving how lint groups work within the existing linting system.
The major changes:
Note: Dynamic groups (i.e.,
allorwarnings) aren't supported at this timeNote: There are open questions about which
Levelwe should default the groups to. This PR does not attempt to answer those questions and simply uses Clippy's default levels for convenience.Footnotes
Clippy uses groups and categories interchangeably ↩