Skip to content

Improve cross-crate trait impl param mismatch suggestions #149027

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
reddevilmidzy:suggest
Feb 28, 2026
Merged

Improve cross-crate trait impl param mismatch suggestions #149027
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
reddevilmidzy:suggest

Conversation

@reddevilmidzy
Copy link
Member

@reddevilmidzy reddevilmidzy commented Nov 17, 2025

resolve: #106999

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 17, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@reddevilmidzy reddevilmidzy marked this pull request as ready for review November 17, 2025 17:04
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 17, 2025
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 17, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 17, 2025

r? @madsmtm

rustbot has assigned @madsmtm.
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

@rust-log-analyzer

This comment has been minimized.

@madsmtm madsmtm added A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` labels Nov 20, 2025
Copy link
Contributor

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in getting back here, this slipped my notifications.

Did you have a look at the previous implementation? #107548

Some of the tests in that one contains //~| HELP attributes, which I think is useful for actually testing that the behaviour is what we desire.

View changes since this review

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot

This comment has been minimized.

@reddevilmidzy
Copy link
Member Author

@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 Dec 15, 2025
Copy link
Contributor

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

Thanks, I had another look through, I like the test you've added, but I still feel that the approach with extracting the signature from spans is the wrong one (though maybe that's just me hating span calculation code), have expanded upon it in a comment.

View changes since this review

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 1, 2026

This PR was rebased onto a different main 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.

@wesleywiser
Copy link
Member

Hi @madsmtm, just a friendly reminder if you have time to look at this PR. The author seems to have addressed all the previous review feedback and rebased 🙂

Copy link
Contributor

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

Thanks for the ping, I'm sorry I haven't gotten back to this sooner.

Things look great, feel free to r=me when the potentially_plural_count is resolved.

View changes since this review

@madsmtm
Copy link
Contributor

madsmtm commented Feb 26, 2026

@bors delegate+

@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 26, 2026

✌️ @reddevilmidzy, you can now approve this pull request!

If @madsmtm told you to "r=me" after making some further change, then please make that change and post @bors r=madsmtm.

@madsmtm madsmtm 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 26, 2026
@reddevilmidzy
Copy link
Member Author

Thanks for the review! I added the comment you mentioned and changed potentially_plural_count to pluralize!.

@rustbot

This comment has been minimized.

@reddevilmidzy
Copy link
Member Author

@bors r=madsmtm

@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 28, 2026

📌 Commit 97748ad has been approved by madsmtm

It is now in the queue for this repository.

@rust-bors rust-bors bot 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 Feb 28, 2026
@reddevilmidzy reddevilmidzy changed the title Implement method signature suggestion for trait mismatches error Improve cross-crate trait impl param mismatch suggestions Feb 28, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 28, 2026
Improve cross-crate trait impl param mismatch suggestions

resolve: rust-lang#106999
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Feb 28, 2026
Improve cross-crate trait impl param mismatch suggestions

resolve: rust-lang#106999
rust-bors bot pushed a commit that referenced this pull request Feb 28, 2026
…uwer

Rollup of 12 pull requests

Successful merges:

 - #153211 (`rust-analyzer` subtree update)
 - #149027 (Improve cross-crate trait impl param mismatch suggestions )
 - #152730 (add field representing types)
 - #153136 (Correctly handle `#[doc(alias = "...")]` attribute on inlined reexports)
 - #152165 (Normalize capture place `ty`s to prevent ICE)
 - #152615 (refactor 'valid for read/write' definition: exclude null)
 - #153109 (Fix LegacyKeyValueFormat report from docker build: aarch64-gnu-debug)
 - #153172 (Fix comment about placeholders)
 - #153187 (Fix ICE when macro-expanded extern crate shadows std)
 - #153190 (Don't allow subdiagnostic to use variables from their parent)
 - #153200 (Remove redundant clone)
 - #153216 (mark two polonius tests as known-bug)
@rust-bors rust-bors bot merged commit 4dd3fea into rust-lang:main Feb 28, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Feb 28, 2026
rust-timer added a commit that referenced this pull request Feb 28, 2026
Rollup merge of #149027 - reddevilmidzy:suggest, r=madsmtm

Improve cross-crate trait impl param mismatch suggestions

resolve: #106999
@reddevilmidzy reddevilmidzy deleted the suggest branch February 28, 2026 16:03
github-actions bot pushed a commit to rust-lang/rust-analyzer that referenced this pull request Mar 2, 2026
…uwer

Rollup of 12 pull requests

Successful merges:

 - rust-lang/rust#153211 (`rust-analyzer` subtree update)
 - rust-lang/rust#149027 (Improve cross-crate trait impl param mismatch suggestions )
 - rust-lang/rust#152730 (add field representing types)
 - rust-lang/rust#153136 (Correctly handle `#[doc(alias = "...")]` attribute on inlined reexports)
 - rust-lang/rust#152165 (Normalize capture place `ty`s to prevent ICE)
 - rust-lang/rust#152615 (refactor 'valid for read/write' definition: exclude null)
 - rust-lang/rust#153109 (Fix LegacyKeyValueFormat report from docker build: aarch64-gnu-debug)
 - rust-lang/rust#153172 (Fix comment about placeholders)
 - rust-lang/rust#153187 (Fix ICE when macro-expanded extern crate shadows std)
 - rust-lang/rust#153190 (Don't allow subdiagnostic to use variables from their parent)
 - rust-lang/rust#153200 (Remove redundant clone)
 - rust-lang/rust#153216 (mark two polonius tests as known-bug)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trait method signature mismatch could suggest changing the signature inline

5 participants