-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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.
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
Expand libstd struct case misspelling diagnostics #72988
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
// checks case typos with libstd::alloc structs | ||
fn main(){} | ||
|
||
fn test_layout(_x: LayOut){} | ||
//~^ ERROR: cannot find type `LayOut` in this scope | ||
fn test_system(_x: system){} | ||
//~^ ERROR: cannot find type `system` in this scope |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
error[E0412]: cannot find type `LayOut` in this scope | ||
--> $DIR/alloc.rs:4:20 | ||
| | ||
LL | fn test_layout(_x: LayOut){} | ||
| ^^^^^^ not found in this scope | ||
| | ||
help: did you mean `std::alloc::Layout`? | ||
| | ||
LL | fn test_layout(_x: std::alloc::Layout){} | ||
| ^^^^^^^^^^^^^^^^^^ | ||
|
||
error[E0412]: cannot find type `system` in this scope | ||
--> $DIR/alloc.rs:6:20 | ||
| | ||
LL | fn test_system(_x: system){} | ||
| ^^^^^^ not found in this scope | ||
| | ||
help: did you mean `std::alloc::System`? | ||
| | ||
LL | fn test_system(_x: std::alloc::System){} | ||
davidtwco marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ^^^^^^^^^^^^^^^^^^ | ||
|
||
error: aborting due to 2 previous errors | ||
|
||
For more information about this error, try `rustc --explain E0412`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
// checks case typos with libstd::any structs | ||
fn main(){} | ||
|
||
fn test_typeid(_x: Typeid){} | ||
//~^ ERROR: cannot find type `Typeid` in this scope |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
error[E0412]: cannot find type `Typeid` in this scope | ||
--> $DIR/any.rs:4:20 | ||
| | ||
LL | fn test_typeid(_x: Typeid){} | ||
| ^^^^^^ not found in this scope | ||
| | ||
help: did you mean `std::any::TypeId`? | ||
| | ||
LL | fn test_typeid(_x: std::any::TypeId){} | ||
| ^^^^^^^^^^^^^^^^ | ||
|
||
error: aborting due to previous error | ||
|
||
For more information about this error, try `rustc --explain E0412`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
// checks case typos with libstd::ascii structs | ||
fn main(){} | ||
|
||
fn test_escdef(_x: Escapedefault){} | ||
//~^ ERROR: cannot find type `Escapedefault` in this scope |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
error[E0412]: cannot find type `Escapedefault` in this scope | ||
--> $DIR/ascii.rs:4:20 | ||
| | ||
LL | fn test_escdef(_x: Escapedefault){} | ||
| ^^^^^^^^^^^^^ not found in this scope | ||
| | ||
help: did you mean one of these?: | ||
| | ||
LL | fn test_escdef(_x: std::ascii::EscapeDefault){} | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
LL | fn test_escdef(_x: std::char::EscapeDefault){} | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^ | ||
LL | fn test_escdef(_x: std::str::EscapeDefault){} | ||
| ^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
error: aborting due to previous error | ||
|
||
For more information about this error, try `rustc --explain E0412`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
// checks case typos with libstd::cell structs | ||
fn main(){} | ||
|
||
fn test_cell(_x: cell<()>){} | ||
//~^ ERROR: cannot find type `cell` in this scope |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
error[E0412]: cannot find type `cell` in this scope | ||
--> $DIR/cell.rs:4:18 | ||
| | ||
LL | fn test_cell(_x: cell<()>){} | ||
| ^^^^ not found in this scope | ||
| | ||
help: did you mean `std::cell::Cell`? | ||
| | ||
LL | fn test_cell(_x: std::cell::Cell<()>){} | ||
| ^^^^^^^^^^^^^^^ | ||
|
||
error: aborting due to previous error | ||
|
||
For more information about this error, try `rustc --explain E0412`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// checks case typos with libstd::char structs | ||
fn main(){} | ||
|
||
fn test_du16(_x: DecodeUTF16<()>){} | ||
//~^ ERROR: cannot find type `DecodeUTF16` in this scope | ||
|
||
fn test_edflt(_x: Escapedefault){} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd combine all of these tests into a single file - we're not gaining anything by having them separate - if we change the approach, it's very likely that if one fails then they all would and so having different tests just means we have more tests to run (likely to be marginally slower). Also, feel free to |
||
//~^ ERROR: cannot find type `Escapedefault` in this scope | ||
|
||
fn test_euni(_x: Escapeunicode){} | ||
//~^ ERROR: cannot find type `Escapeunicode` in this scope | ||
|
||
fn test_tolow(_x: Tolowercase){} | ||
//~^ ERROR: cannot find type `Tolowercase` in this scope | ||
|
||
fn test_toupper(_x: Touppercase){} | ||
//~^ ERROR: cannot find type `Touppercase` in this scope |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should take a different approach - I'd rather we avoid hard-coding
std
type names in the compiler (the compiler shouldn't need to be updated whenstd
is changed for this diagnostic to continue to work) and I think we can make this more general, so it works for more than juststd
.As you note in the PR description, we do Levenshtein matching on things that are already in-scope and suggest fixes in that case. We also already have suggestions for things that aren't in scope but where the name matches exactly.
The snippet below is where the compiler decides to make "did you mean to import this" suggestions:
rust/src/librustc_resolve/diagnostics.rs
Lines 660 to 693 in 50c0192
On line 660 is where it checks for an exact match - I'd experiment with doing a exact match or checking for a Levenshtein distance from that name that's under some threshold (look into
lev_distance
andfind_best_match_for_name
to see what can be re-used). I'd hope that would detect cases like this where we can import a name that's very similar. Alternatively, we could just change the comparison to compare the names without considering the case (as I understand it, that would be equivalent to what you're doing here?).What we might also want to do is thread this information through in some way - e.g. changing
ImportSuggestion
to be an enum withExact
andAlmost
(you can come up with a better name than this) variants so that we can make the diagnostic separate these two types of suggestions:This should be easier to maintain because we don't have to keep a list of types like you've got here, and it'd work for non-
std
types too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tyvm David! I will look into it this week. I really appreciate the review and will be in touch when I have updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nitpick on wording:
help: consider importing one of these similarly named items
→help: you might have meant to use {one of }the following similarly named item{s}
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chrissimpkins How are you getting with this? Feel free to open a work-in-progress PR if you’re unsure how to proceed or want some early feedback!
There was a problem hiding this 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! Busy week last week. I will get back to this during the week and let you know if I run into any issues.