Suggest case insensitive import suggestions

[GTimothy]

This PR proposes case insensitive import suggestions.
Previous discussion of this topic here: #72641 and in this PR: #72988
If that is still of interest, a discussion may be needed to limit or expand the included list.
I initially followed the suggestions in #72988.

The tests are based on the tests by @chrissimpkins in #72988.

I added a is_exact_match field to ImportSuggestion to modify the suggestion text accordingly.

Only when no other suggestion is made, do I check for case insensitive import suggestion.

fn test_layout(_x: LayOut){}
//~^ ERROR: cannot find type `LayOut` in this scope

error[E0425]: cannot find type `LayOut` in this scope
  --> $DIR/libstd.rs:3:20
   |
LL | fn test_layout(_x: LayOut){}
   |                    ^^^^^^ not found in this scope
   |
help: consider importing this similarly named struct
   |
LL + use std::alloc::Layout;
   |

for a more complex case:
SystemTime exists in the stdlib, but I only suggest case insensitive import when nothing else is suggested

fn test_systemtime(_x: Systemtime){}
//~^ ERROR: cannot find type `Systemtime` in this scope

struct SystemTome{}
mod st{
    struct SystemTame{}
}

error[E0425]: cannot find type `Systemtime` in this scope
  --> $DIR/libstd.rs:428:24
   |
LL | fn test_systemtime(_x: Systemtime){}
   |                        ^^^^^^^^^^
...
LL | struct SystemTome{}
   | ----------------- similarly named struct `SystemTome` defined here
   |
help: a struct with a similar name exists
   |
LL - fn test_systemtime(_x: Systemtime){}
LL + fn test_systemtime(_x: SystemTome){}
   |

During this PR, I ended up adding two other things which could arguably be two separate PRs:

• filtering out typos suggestions that do not have parameters when the typo has some
  motivation: It suggested Clone (no parameter) for the std cloned<(),()> test when I expected Cloned<(),()> to be suggested.

• when suggesting that a missing binding is available in a pattern but not used because behind a .., suggest a MaybeIncorrect fix.
  motivation: this way, I can avoid looking for imports if there is a suggestion.

• custom import message for case insensitive suggestion.

• only suggest case insensitive when no other suggestion is made

• check for enum variant match before suggesting an enum

• check for enum variant parameter requirement before suggesting an enum

• check with reviewers where to place the test, and whether to rename them

• squash and split commits for a clean PR

[GTimothy]

I see from the 247 failed tests that 1-edit difference is too big for small names. 'net' should probably NOT be suggested to replace 'new'...

Edit: i have now switched to case insensitive match only.

[rustbot]

r? @TaKO8Ki

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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 17 candidates

[GTimothy]

upstream change #157991 contains commit cd2d10a and 4f6a600 from #157974 which do some weird file swapping instead of renamings that completely confuses and breaks my rebase attempts.
I may have some skill issue here, does anyone know how to fix this? Is there a proper git workflow to resolve this or do I need to copy my changes over to the swapped file and delete the changes in the original file by hand?

If this is not a skill issue on my part, maybe avoiding this kind of file swapping and splitting this in multiple rename commits would help git not getting confused ?

[GTimothy]

For anyone having trouble with the file swap, my cleanest solution is:

• assuming I have 7 commits on top of my out of date branch.
• assuming no existing *.patch files present.

# make patches out of my commits
git format-patch HEAD~7..HEAD

# edit the patch files to change the file path
sed -i 's|a/compiler/rustc_resolve/src/diagnostics.rs|a/compiler/rustc_resolve/src/error_helper.rs|g; s|b/compiler/rustc_resolve/src/diagnostics.rs|b/compiler/rustc
_resolve/src/error_helper.rs|g' *.patch  

# create new 'fix-swapped-files' branch from upstream latest
git checkout -b fix-swapped-files <upstream>/main

# apply updated patches
git am --committer-date-is-author-date *.patch 

[rustbot]

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.

[rust-bors]

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

[apiraino]

@rustbot reroll

[mu001999]

During this PR, I ended up adding two other things which could arguably be two separate PRs

I think it would be good to extract them as separate PRs. Let's keep this PR only related to case-insensitive import suggestions.

And, please squash the commits to three commits, which would be helpful to review:

1. Adding tests and make a snapshot of current compiler behavior
2. The new implementation
3. Bless tests

@rustbot author

View changes since this review

[mu001999]

I think such suggestions may not be what we expect.

Considering let x = HashMap;, we won't suggest importing HashMap because it is in the type namespace but we expect value namespace.

[mu001999]

Basically we want the suggested imports to be in the same namespace.

[GTimothy]

Ok, so I had to dive deep; there are two similar but distinct cases here.
I am writing this down as much for myself as for anyone else.

std::fmt::Error

Error here is a struct. It gets to live in two namespaces: TypeNS and ValueNS.
In this case, Error is a unit struct, it gets both a type and a const constructor with a DefKind of Ctor( Struct, Const,) that gets matched to ValueNS

DefKind::Fn
 | DefKind::Const { .. }
 | DefKind::ConstParam
 | DefKind::Static { .. }
 | DefKind::Ctor(..)
 | DefKind::AssocFn
 | DefKind::AssocConst { .. } => Some(Namespace::ValueNS),

std::fs::TryLock::Error

Error is an enum variant. It also get to be in the same two namespaces because of two distinct DefKinds:
a Variant DefKind gets mapped to the TypeNS namespace and the variant constructor gets a Ctor( Variant, Fn, ) DefKind (it is not a unit variant).
The same mapping as above places the constructor in the ValueNS namespace.

Therefore, fmt::Error is a valid suggestion.
For fs::TryLock::Error, I am looking into filtering the suggestion if it requires more parameters than provided.

[mu001999]

I test this on nightly, i.e, let x = Error;. it will emit the same suggestion.

So I think it is good enough to keep the behavior have implemented here.

And for the filter, I think don't need to implement it. IMO, we could assume user even don't know it requires parameters or not, so we could still emit them.

But we could sort them (I'm not sure we have done it or not) based on the these conditions. And also this could be a separate PR.

Sorry for the confusing, I should test the same case instead of different ones firstly. And thanks for your investigation.

[mu001999]

I think it would be good if this could be consistent with what we suggest for Range, currently we will get:

error[E0425]: cannot find type `Range` in this scope
 --> src/lib.rs:1:11
  |
1 | fn foo(x: Range) {}
  |           ^^^^^ not found in this scope
  |
help: consider importing one of these items
  |
1 + use std::collections::btree_map::Range;
  |
1 + use std::collections::btree_set::Range;
  |
1 + use std::range::Range;
  |
1 + use std::range::legacy::Range;
  |
  = and 10 other candidates

[GTimothy]

I am not seeing these 10 other candidates suggestions when running the same test on the current nightly or sable releases:

error[E0425]: cannot find type `Range` in this scope
 --> src/main.rs:8:16
  |
8 |     fn range(r:Range){}
  |                ^^^^^ not found in this scope
  |
help: consider importing one of these structs
  |
8 +     use std::collections::btree_map::Range;
  |
8 +     use std::collections::btree_set::Range;
  |
8 +     use std::range::Range;
  |
8 +     use std::range::legacy::Range;
  |
  = and 2 other candidates

on latest main (no other candidates):

error[E0425]: cannot find type `Range` in this scope
  --> $DIR/libstd.rs:272:19
   |
LL | fn test_Range(_x: Range){}
   |                   ^^^^^ not found in this scope
   |
help: consider importing one of these structs
   |
LL + use std::collections::btree_map::Range;
   |
LL + use std::collections::btree_set::Range;
   |
LL + use std::range::Range;
   |
LL + use std::range::legacy::Range;
   |

I reporting the same things for range (lowercase) as the main branch reports for Range (uppercase).
Unless I am missing something?

[mu001999]

Oh, sorry for this, I found it on stable (play), it would emit:

   Compiling playground v0.0.1 (/playground)
error[E0425]: cannot find type `Range` in this scope
 --> src/lib.rs:1:11
  |
1 | fn foo(x: Range) {}
  |           ^^^^^ not found in this scope
  |
help: consider importing one of these structs
  |
1 + use std::collections::btree_map::Range;
  |
1 + use std::collections::btree_set::Range;
  |
1 + use std::ops::Range;
  |
1 + use std::range::Range;
  |
  = and 2 other candidates

But on nightly, it suggests the same to this PR. So just ignore this :)

[mu001999]

Seems #125687 has stabilized recently, so this change in the last commit is because rebasing, I guess, right?

Just want to make sure I don't miss something.

[mu001999]

Use E0425 instead here

[mu001999]

Could we re-use try_lookup_name_relaxed? E.g., adding a mode param for that function and re-use most logic in it, instead of adding a new function here.

[rustbot]

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