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.

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

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

Merged
merged 1 commit into from
Aug 6, 2021
Merged

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

merged 1 commit into from
Aug 6, 2021

Conversation

Kobzol
Copy link
Contributor

@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
Collaborator

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.

@Kobzol Kobzol force-pushed the single-associated-item-hint branch from 94f8741 to 6b7f4a2 Compare August 5, 2021 14:19
@Kobzol
Copy link
Contributor 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
Contributor

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