Skip to content

Add hint for unresolved associated trait items if the trait has a single item#87775

Merged
bors merged 1 commit intorust-lang:masterfrom
Kobzol:single-associated-item-hint
Aug 6, 2021
Merged

Add hint for unresolved associated trait items if the trait has a single item#87775
bors merged 1 commit intorust-lang:masterfrom
Kobzol:single-associated-item-hint

Conversation

@Kobzol
Copy link
Member

@Kobzol Kobzol commented Aug 4, 2021

This PR introduces a special-cased hint for unresolved trait items paths. It is shown if:

  • the path was not resolved to any existing trait item
  • and no existing trait item's name was reasonably close with regard to edit distance
  • and the trait only has a single item in the corresponding namespace

I didn't know where I should put tests, therefore so far I just managed to bless two existing tests. I would be glad for hints where should tests for a hint like this be created, how should they be named (with reference to the original issue?) and what tests should I create (is it enough to test it just for types? or create separate tests also for functions and constants?).

It could also be turned into a machine applicable suggestion I suppose.

This is my first rustc PR, so please go easy on me :)

Fixes: #87638

@rust-highfive
Copy link
Contributor

rust-highfive commented Aug 4, 2021

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nagisa (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 4, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Kobzol Kobzol force-pushed the single-associated-item-hint branch from c846c0b to 94f8741 Compare August 5, 2021 06:45
@oli-obk
Copy link
Contributor

oli-obk commented Aug 5, 2021

I didn't know where I should put tests, therefore so far I just managed to bless two existing tests. I would be glad for hints where should tests for a hint like this be created,

not sure we have a dedicated diagnostics folder, but you could put them in the associated items folder.

how should they be named (with reference to the original issue?) and what tests should I create

I think a single issue-87638.rs file should suffice. You can then even add //~ NOTE comments to make sure the notes are actually emitted (and no one accidentally blesses them away).

(is it enough to test it just for types? or create separate tests also for functions and constants?).

It would be enough, but it also doesn't hurt to test all three.

It could also be turned into a machine applicable suggestion I suppose.

That would be cool, but not necessary. If you want to take a shot at it in this PR, feel free to do it. You can even test+bless it by adding a // run-rustfix comment to your test file.

oli-obk
oli-obk reviewed Aug 5, 2021
View reviewed changes
@Kobzol Kobzol force-pushed the single-associated-item-hint branch from 94f8741 to 6b7f4a2 Compare August 5, 2021 14:19
@Kobzol
Copy link
Member Author

Kobzol commented Aug 5, 2021

I added the distinguishing enum to TypoSuggestion. It could be prettier but it seems to me that changing the other functions would result in much more mess.

@oli-obk
Copy link
Contributor

oli-obk commented Aug 5, 2021

Thanks! This looks good now. Not sure why CI is failing, I'll try restarting it.

@Kobzol

This comment has been minimized.

Sign in to view
@Kobzol Kobzol force-pushed the single-associated-item-hint branch from 6b7f4a2 to d0d4947 Compare August 6, 2021 09:32
@oli-obk
Copy link
Contributor

oli-obk commented Aug 6, 2021

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 6, 2021

📌 Commit d0d4947 has been approved by oli-obk

@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 Aug 6, 2021
@oli-obk
Copy link
Contributor

oli-obk commented Aug 6, 2021

Thanks for taking care of this so fast!

@JohnTitor JohnTitor mentioned this pull request Aug 6, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 6, 2021
@bors
Auto merge of rust-lang#87822 - JohnTitor:rollup-kxojii0, r=JohnTitor
5ad7389 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#85807 (bootstrap: Disable initial-exec TLS model on powerpc)
 - rust-lang#87761 (Fix overflow in rustc happening if the `err_count()` is reduced in a stage.)
 - rust-lang#87775 (Add hint for unresolved associated trait items if the trait has a single item)
 - rust-lang#87779 (Remove special case for statement `NodeId` assignment)
 - rust-lang#87787 (Use `C-unwind` ABI for `__rust_start_panic` in `panic_abort`)
 - rust-lang#87809 (Fix typo in the ptr documentation)
 - rust-lang#87816 (Sync rustc_codegen_cranelift)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8ee962f into rust-lang:master Aug 6, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 6, 2021
@Kobzol Kobzol deleted the single-associated-item-hint branch August 6, 2021 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obk oli-obk oli-obk left review comments

Assignees

@nagisa nagisa

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.56.0

Development

Successfully merging this pull request may close these issues.

"cannot find associated type" could hint existing associated types

7 participants

@Kobzol @rust-highfive @rust-log-analyzer @oli-obk @bors @nagisa @rustbot