Skip to content
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

[Clippy] Get rid of most std match_def_path usage, swap to diagnostic items. #130553

Merged
merged 20 commits into from
Sep 19, 2024

Conversation

GnomedDev
Copy link
Contributor

Part of rust-lang/rust-clippy#5393.

This was going to remove all std paths, but SeekFrom has issues being cleanly replaced with a diagnostic item as the paths are for variants, which currently cannot be diagnostic items.

This also, as a last step, categories the paths to help with future path removals.

@rustbot
Copy link
Collaborator

rustbot commented Sep 19, 2024

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 19, 2024
@rustbot
Copy link
Collaborator

rustbot commented Sep 19, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@@ -416,6 +416,7 @@ impl<T> Vec<T> {
/// ```
#[inline]
#[rustc_const_stable(feature = "const_vec_new", since = "1.39.0")]
#[cfg_attr(not(test), rustc_diagnostic_item = "vec_new")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned on Zulip already, but we typically do this by making the item a diagnostic item then checking the method's identifier. That's typically how it's suggested to be done. But regardless this way is fine, and it doesn't need to be blocked on it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I had already implemented all this before talking on Zulip and seeing the idea to check method identifiers. Since there are already diagnostic items on methods, you are correct that this very much shouldn't be blocked on that though.

@compiler-errors
Copy link
Member

@rustbot author given the discussion in https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/Associated.20function.20from.20type.20DefId.20and.20Symbol.3F, since we might be able to drastically cut down on the number of diagnostic items that we need to define

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 19, 2024
@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 19, 2024
@compiler-errors
Copy link
Member

compiler-errors commented Sep 19, 2024

Discussed in another channel; author mentioned that she didn't currently plan on cleaning up the "one million diagnostic items" problem since its own large body of work.

Give me a sec, I'll review this as-is.
@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 19, 2024
@compiler-errors
Copy link
Member

@bors r+

I would like to repeat that it would be very nice if we cleaned this up "the right way" though, in the future; it would mean that clippy would need fewer rustc-based PRs to add new diagnostics, and it would mean the standard library has fewer diagnostic items.

@bors
Copy link
Contributor

bors commented Sep 19, 2024

📌 Commit 13d5732 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 19, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 19, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#128001 (Improve documentation for <integer>::from_str_radix)
 - rust-lang#130553 ([Clippy] Get rid of most `std` `match_def_path` usage, swap to diagnostic items.)
 - rust-lang#130554 (`pal::unsupported::process::ExitCode`: use an `u8` instead of a `bool`)
 - rust-lang#130556 (Mark the `link_cfg` feature as internal)
 - rust-lang#130558 (Support 128-bit atomics on s390x)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 569153a into rust-lang:master Sep 19, 2024
6 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 19, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 19, 2024
Rollup merge of rust-lang#130553 - GnomedDev:remove-clippy-paths, r=compiler-errors

[Clippy] Get rid of most `std` `match_def_path` usage, swap to diagnostic items.

Part of rust-lang/rust-clippy#5393.

This was going to remove all `std` paths, but `SeekFrom` has issues being cleanly replaced with a diagnostic item as the paths are for variants, which currently cannot be diagnostic items.

This also, as a last step, categories the paths to help with future path removals.
@GnomedDev GnomedDev deleted the remove-clippy-paths branch September 19, 2024 22:15
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 20, 2024
…=compiler-errors

[Clippy] Remove final std paths for diagnostic item

Removes the paths to SeekFrom::Start/Current that were left in rust-lang#130553.

This was split off as it involves introducing a utility to check for enum ctors, as both:
- enum variants cannot be diagnostic items
- even if they could, that wouldn't help because we need to get the enum variant ctor

While adding the `is_enum_variant_ctor`, I removed both `is_diagnostic_ctor` and `is_res_diagnostic_ctor` as they are unused and never worked due to the above bullet points.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 20, 2024
Rollup merge of rust-lang#130607 - GnomedDev:remove-seekfrom-paths, r=compiler-errors

[Clippy] Remove final std paths for diagnostic item

Removes the paths to SeekFrom::Start/Current that were left in rust-lang#130553.

This was split off as it involves introducing a utility to check for enum ctors, as both:
- enum variants cannot be diagnostic items
- even if they could, that wouldn't help because we need to get the enum variant ctor

While adding the `is_enum_variant_ctor`, I removed both `is_diagnostic_ctor` and `is_res_diagnostic_ctor` as they are unused and never worked due to the above bullet points.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-unix Operating system: Unix-like S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants