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

Suggest similar names for types #96839

Closed
wants to merge 5 commits into from
Closed

Suggest similar names for types #96839

wants to merge 5 commits into from

Conversation

m-ysk
Copy link
Contributor

@m-ysk m-ysk commented May 8, 2022 •
edited
Loading

Fixes #96625
Fixes #95462

This PR enables to suggest similar names for types like the following examples. About the detailed motivation of this PR, please see the above issues.

error[E0433]: failed to resolve: use of undeclared crate or module `string`
  --> $DIR/suggest-type-to-lowercase-path.rs:7:13
   |
LL |     let _ = string::new();
   |             ^^^^^^
   |             |
   |             use of undeclared crate or module `string`
   |             help: there is a struct with a similar name (notice the capitalization): `String`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0433`.

error[E0433]: failed to resolve: use of undeclared type `FooTyp`
  --> $DIR/suggest-similar-type-name.rs:8:5
   |
LL |     FooTyp::bar()
   |     ^^^^^^
   |     |
   |     use of undeclared type `FooTyp`
   |     help: there is a struct with a similar name: `FooType`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0433`.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 8, 2022
@rust-highfive
Copy link
Collaborator

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2022
@m-ysk m-ysk changed the title Suggest similar names for types WIP: Suggest similar names for types May 8, 2022
m-ysk
m-ysk commented May 9, 2022
View reviewed changes
compiler/rustc_resolve/src/diagnostics.rs
Comment on lines +1834 to +1857
let mut names = Vec::new();
if let Some(ribs) = ribs {
// Search in lexical scope.
// Walk backwards up the ribs in scope and collect candidates.
for rib in ribs[TypeNS].iter().rev() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I implemented the logics after this line with reference to the following function:

rust/compiler/rustc_resolve/src/late/diagnostics.rs

Line 1343 in e013f9e

fn lookup_typo_candidate(

Copy link
Member

Choose a reason for hiding this comment

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

Any way these could be merged or share logic? Might be worth leaving a comment at the very least.

@m-ysk m-ysk marked this pull request as ready for review May 9, 2022 16:16
@m-ysk m-ysk changed the title WIP: Suggest similar names for types Suggest similar names for types May 9, 2022
@m-ysk
Copy link
Contributor Author

m-ysk commented May 15, 2022

@jackh726
I have finished all works I can do now on this PR. Could you review it?

jackh726
jackh726 requested changes May 16, 2022
View reviewed changes
compiler/rustc_resolve/src/diagnostics.rs Outdated
Comment on lines 1871 to 1872
names.sort_by_key(|name| (name.candidate, name.res));
names.dedup();
Copy link
Member

Choose a reason for hiding this comment

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

I assume this is what the PartialOrd/Ord derives are for. Can you try using an IndexSet instead? I'm wary of adding these impls, given their potential as a source of incremental bugs.

Copy link
Member

Choose a reason for hiding this comment

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

Or, sort as in lookup_typo_candidate by the candidate name str

compiler/rustc_resolve/src/diagnostics.rs
Comment on lines +1834 to +1857
let mut names = Vec::new();
if let Some(ribs) = ribs {
// Search in lexical scope.
// Walk backwards up the ribs in scope and collect candidates.
for rib in ribs[TypeNS].iter().rev() {
Copy link
Member

Choose a reason for hiding this comment

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

Any way these could be merged or share logic? Might be worth leaving a comment at the very least.

compiler/rustc_resolve/src/diagnostics.rs Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented May 21, 2022

☔ The latest upstream changes (presumably #97239) made this pull request unmergeable. Please resolve the merge conflicts.

jackh726
jackh726 reviewed Jun 7, 2022
View reviewed changes
compiler/rustc_hir/src/hir.rs
@@ -2381,7 +2381,7 @@ impl<'hir> Ty<'hir> {
}

/// Not represented directly in the AST; referred to by name through a `ty_path`.
#[derive(Copy, Clone, PartialEq, Eq, Encodable, Decodable, Hash, Debug)]
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Encodable, Decodable, Hash, Debug)]
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need this?

@jackh726 jackh726 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 Jun 26, 2022
@bors
Copy link
Contributor

bors commented Jul 9, 2022

☔ The latest upstream changes (presumably #99078) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon
Copy link
Member

@m-ysk

ping from triage - can you post your status on this PR? Thanks
FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

@JohnCSimon
Copy link
Member

@m-ysk
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you do please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Oct 2, 2022
@JohnCSimon JohnCSimon closed this Oct 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jackh726 jackh726 jackh726 requested changes

Assignees

@jackh726 jackh726

Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Suggest similar names for types, macros and modules Typo MaybeUnint doesn't suggest MaybeUninit
6 participants
@m-ysk @rust-highfive @bors @JohnCSimon @jackh726 @rustbot