Skip to content

Add diagnostic items for Clippy #142787

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add diagnostic items for Clippy #142787

wants to merge 3 commits into from
+30 −34

Conversation

samueltardieu
Copy link
Contributor

Clippy still uses some paths to access items from the standard library. Adding the missing diagnostic items allows removing the last remaining paths.

Closes rust-lang/rust-clippy#5393

@rustbot
Copy link
Collaborator

rustbot commented Jun 20, 2025

r? @tgross35

rustbot has assigned @tgross35.
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 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 Jun 20, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 20, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Urgau
Urgau approved these changes Jun 20, 2025
View reviewed changes
Copy link
Member

@Urgau Urgau left a comment

Choose a reason for hiding this comment

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

r=me on the compiler side

@Urgau
Copy link
Member

Urgau commented Jun 20, 2025

We don't have a clippy group, so choosing at random.

r? @flip1995

@rustbot rustbot assigned flip1995 and unassigned tgross35 Jun 20, 2025
@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 20, 2025

📌 Commit 07338a4 has been approved by Manishearth

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 Jun 20, 2025
@flip1995
Copy link
Member

Does

r? clippy

not work in the Rust repo? 🤔

@rustbot rustbot assigned Centri3 and unassigned flip1995 Jun 20, 2025
@flip1995
Copy link
Member

It does. Sorry for the ping 😅 r? Manishearth

@rustbot rustbot assigned Manishearth and unassigned Centri3 Jun 20, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Urgau
Copy link
Member

Urgau commented Jun 20, 2025

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 20, 2025
@Urgau
Copy link
Member

Urgau commented Jun 20, 2025

Does r? clippy not work in the Rust repo? 🤔

Oh you're right, it does work because it fallback to the clippy team, forgot that it tried that when it doesn't find an adhoc_group with the same name.

@samueltardieu
Bless UI tests: do not print alternate implementors of Step
890ade5 
Alternative candidates for a trait implementation are not printed when
the trait has a diagnostic name, to avoid printing alternatives for
common stdlib traits such as `Copy` or `Debug`. However, this affects
all traits for which a diagnostic item is added.

Here, the list of alternatives candidates for `Step` does not seem
useful, and `Step` is unstable, so this will not be missed.
@samueltardieu
Copy link
Contributor Author

samueltardieu commented Jun 20, 2025 •
edited
Loading

The failure was really "fun": when a trait expectation is not fulfilled, other types implementing the trait are listed except when the trait is also a diagnostic item. This has been done to prevent a long list of alternate types to be suggested for common traits such as Debug, Copy, etc.

As a consequence, the Step trait no longer suggests a list of implementors since I added a diagnostic item to it. However, I think that the suggestion will not be missed. Here is the extra help that was removed by the third commit of this PR:

 LL |     for i in false..true {}
    |              ^^^^^^^^^^^ the trait `Step` is not implemented for `bool`
    |
-   = help: the following other types implement trait `Step`:
-             Char
-             Ipv4Addr
-             Ipv6Addr
-             char
-             i128
-             i16
-             i32
-             i64
-           and 8 others

I don't think the suggestion was useful to start with. Thoughts?

@Urgau
Copy link
Member

Urgau commented Jun 20, 2025

I think it's fine.

@bors r=Manishearth,Urgau rollup

@bors
Copy link
Collaborator

bors commented Jun 20, 2025

📌 Commit 890ade5 has been approved by Manishearth,Urgau

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 20, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 21, 2025
@matthiaskrgr
Rollup merge of rust-lang#142787 - samueltardieu:diag-items-for-clipp…
24f797e 
…y, r=Manishearth,Urgau

Add diagnostic items for Clippy

Clippy still uses some paths to access items from the standard library. Adding the missing diagnostic items allows removing the last remaining paths.

Closes rust-lang/rust-clippy#5393
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 21, 2025
Open
bors added a commit that referenced this pull request Jun 21, 2025
@bors
Auto merge of #142817 - matthiaskrgr:rollup-mrobovy, r=matthiaskrgr
6d0c9e2 
Rollup of 7 pull requests

Successful merges:

 - #142502 (rustdoc_json: improve handling of generic args)
 - #142597 (error on calls to ABIs that cannot be called)
 - #142785 (fix(linkcheck): Build using the lockfile)
 - #142787 (Add diagnostic items for Clippy)
 - #142788 (add doc(alias("AsciiChar")) to core::ascii::Char)
 - #142801 (Use gen blocks in the compiler instead of `from_coroutine`)
 - #142804 (Rename `LayoutS` to `LayoutData` in comments)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Urgau Urgau Urgau approved these changes

Assignees

@Manishearth Manishearth

Labels
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move our paths over to diagnostic items
9 participants
@samueltardieu @rustbot @Urgau @Manishearth @bors @flip1995 @rust-log-analyzer @tgross35 @Centri3