Hint use of use crate:: on invalid mod
#71288
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
kornelski
force-pushed
the
E0583
branch
2 times, most recently
from
ec54700
to
5e89c5c
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
I've rebased it. It's ready to merge. |
joelpalmer
added
S-waiting-on-review
|
r? @dtolnay |
|
oops this is compiler.. |
estebank
reviewed
May 1, 2020
Comment on lines
+287
to
+290
| err.help(&format!( | ||
| "if there is `mod {}` elsewhere in the crate already, import it with `use crate::` instead", | ||
| mod_name, | ||
| )); |
There was a problem hiding this comment.
You can do the following instead
err.span_suggestion_verbose(
span,
"if the module exists elsewhere in the crate already, you can import instead",
format!("use crate::{};", mod_name),
Applicability::MaybeIncorrect,
)That being said, it'd be nice to actually check that the module exists in the crate and provide the suggestion with higher certainty. Do not have the time to check what the code would have to be to verify that crate::mod_name exists, but @petrochenkov might know of the top of their head. Otherwise, would you mind exploring how to do that, @kornelski? We want to minimize the cases where we provide incorrect suggestions, as those can be worse than terse ones. If we can make that check, then we can provide the help to create the file exclusively if the mod doesn't exist alredy.
There was a problem hiding this comment.
I'm happy to dig in further, but I'd prefer to land this change in the meantime.
The message starts with a disclaimer, it intentionally only suggests use instead of showing a potentially-incorrect path. So I think risk of confusion is low, and mentioning use is an important improvement.
This check currently happens very early in the parsing, so a naive mod existence checks at this stage would be unreliable and sensitive to order of definitions. A proper fix may be quite involved, e.g. create a "broken mod" AST node for a later pass to pick it up and scan modules after they're all parsed. That might take longer to implement, so the current PR is IMHO good to have in the meantime.
|
@estebank you happy with this? |
joelpalmer
added
S-waiting-on-review
Elinvynia
added
S-waiting-on-review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Next step in #69492
It's not feasible to know at this stage of parsing what other modules exists, so the help string is speculative unfortunately.
The goal is to help users who assumed
mod foo;works the same way as#include "foo"orrequire("foo"), and should have useduse crate::fooinstead.