use structured suggestions for E0432 #58498
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
src/librustc_resolve/lib.rs
Outdated
| String::from("unresolved import"), | ||
| Some(( | ||
| ident.span, | ||
| String::from("a similar path resolves"), |
There was a problem hiding this comment.
| String::from("a similar path resolves"), | |
| String::from("a similar path exists"), |
Word choice: I think "exists" feels less stilted than "resolves" here.
| note, | ||
| suggestion: Some(( | ||
| span, | ||
| String::from("a similar path resolves"), |
|
r? @zackmdavis @bors delegate+ |
|
✌️ @euclio can now approve this pull request |
| | ^^^^^--- | ||
| | | | | ||
| | | help: a similar name exists in the module: `bar` | ||
| | no `baz` in `zed` |
There was a problem hiding this comment.
Is it possible to move the "no baz in zed" message into the main error message and have the help end up in the position that it was previously? Not sure if our emitters will do that though.
There was a problem hiding this comment.
Do you mean an error message like:
-> $DIR/import.rs:2:5
|
LL | use zed::baz; //~ ERROR unresolved import: no `baz` in `zed` [E0432]
| --- help: a similar name exists in the module: `bar`
|
There was a problem hiding this comment.
oh yea, that's even better than what I had in mind.
There was a problem hiding this comment.
error[E0432]: unresolved imports `zed::baz`, `zed::bay`
--> src/test/ui/import.rs:7:11
|
7 | use zed::{baz, bay};
| ^^^ ^^^ no `bay` in `zed`
| |
| no `baz` in `zed`
help: a similar name exists in the module
|
7 | use zed::{bar, bay};
| ^^^
help: a similar name exists in the module
|
7 | use zed::{baz, bar};
| ^^^
Well, my idea won't work with the multi-import case, though.
There was a problem hiding this comment.
That message still has the no bay in zed messages, if we eliminate them entirely, at least one of the help messages should show up in the correct space
There was a problem hiding this comment.
Right, that's the output of my code as it stands. I think that removing the label entirely would regress the error message if there's no suggestion. I'm not sure that making them mutually exclusive would be better, either. @estebank, what do you think?
There was a problem hiding this comment.
I think that for the last case shown here it is best to be conservative (and verbose). An option you have @euclio is to use span_suggestion_hidden (#58296) to display only the message without the code snippet, but you would have to explicitly write the found identifier in the message to be usable from the cli output (as opposed to VSCode):
error[E0432]: unresolved imports `zed::baz`, `zed::bay`
--> src/test/ui/import.rs:7:11
|
7 | use zed::{baz, bay};
| ^^^ ^^^ no `bay` in `zed`
| |
| no `baz` in `zed`
= help: `bar`, which is similar to `baz`, exists in `zed`
= help: `bar`, which is similar to `bay`, exists in `zed`
It'd be also neat to collect the suggestions to avoid suggesting the same name multiple times, but that's gold plating.
There was a problem hiding this comment.
Feel free not to do this, I think that the output as is is already pretty good (modulo Zack's wording comments).
|
r? @estebank |
|
ping from triage @estebank waiting for your review on this |
|
Beyond @zackmdavis' comments and mine above, it looks good! |
euclio
force-pushed
the
e0432-suggestions
branch
from
c929d5a
to
4bbe883
Compare
|
@estebank I've fixed the wording suggestions. I think I'll leave the label improvements to a future PR. |
|
@bors r+ |
|
📌 Commit 4bbe883 has been approved by |
bors
added
S-waiting-on-bors
use structured suggestions for E0432
|
☀️ Test successful - checks-travis, status-appveyor |
No description provided.