-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Move internal lints to their own crate #13223
base: master
Are you sure you want to change the base?
Conversation
6274ff4
to
6320f1a
Compare
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 like those changes. Splitting up Clippy definitely makes sense. We might also want to look into turning Clippy into a workspace, like cargo did. I think @xFrednet wanted to look into this.
☔ The latest upstream changes (presumably #13225) made this pull request unmergeable. Please resolve the merge conflicts. |
Marking this as blocked on #13221 |
r? rust-lang/clippy This was simplified a bit by #13221. |
tests/dogfood.rs
Outdated
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.
Could do with adding the new crate to the list of dogfooded crates
8314085
to
34be10e
Compare
]; | ||
|
||
pub fn register_lints(store: &mut LintStore) { | ||
store.register_lints(LINTS); |
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.
This no longer registers the clippy::internal
group. I assume that's intentional since it's not needed anymore for #![deny(clippy::internal)]
in tests and -Dclippy::internal
as all internal lints are now warn by default?
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.
The internal category is now completely gone. It was only really needed to mark some generated code with #[cfg(feature = "internal")]
.
@@ -194,7 +192,6 @@ pub fn rename(old_name: &str, new_name: &str, uplift: bool) { | |||
let name = f.path().file_name(); | |||
let ext = f.path().extension(); | |||
(ext == Some(OsStr::new("rs")) || ext == Some(OsStr::new("fixed"))) | |||
&& name != Some(OsStr::new("rename.rs")) |
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.
Why is this no longer needed?
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.
This changed in a previous PR. Renamed lints were moved into the same file as deprecated lints.
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 assume with previous PR you mean #13180. That removed renamed_lints.rs
, right, but the separate ui test rename.rs
still exists? Maybe I don't understand what this exclusion is for in the first place. I'd assumed it's because the ui test and the list of deprecations/renames require special handling and are regenerated, and they still are, so I would think that both conditions here are still needed?
Sorry for all the questions, I'd like to properly understand what's going on here 😅
clippy_internal_lints/Cargo.toml
Outdated
itertools = "0.12" | ||
serde = { version = "1.0", features = ["derive"] } | ||
serde_json = { version = "1.0" } | ||
tempfile = { version = "3.3.0" } |
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.
These deps all seem unused
msrv_attr_impl::MISSING_MSRV_ATTR_IMPL, | ||
outer_expn_data_pass::OUTER_EXPN_EXPN_DATA, | ||
produce_ice::PRODUCE_ICE, | ||
unnecessary_def_path::UNNECESSARY_DEF_PATH, |
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.
This is missing UNSORTED_CLIPPY_UTILS_PATHS
☔ The latest upstream changes (presumably #13343) made this pull request unmergeable. Please resolve the merge conflicts. |
This makes it so switching the internal feature on/off no longer rebuilds
clippy_lints
.r? @flip1995
changelog: none