Improve missing extern crate diagnostics in Rust 2015 - #161210
Conversation
rustbot
added
the
S-waiting-on-review
rustbot
added
the
T-compiler
|
r? @jackh726 rustbot has assigned @jackh726. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
706a67a to
1367919
Compare
| Relative2018(Span, ast::Path), | ||
| AncestorOnly(Span), | ||
| FailedToResolve(Span, Symbol, String, Option<Suggestion>, String), | ||
| FailedToResolve(Span, Symbol, String, Option<Suggestion>, Option<String>, String), |
There was a problem hiding this comment.
Can you add a comment about what all these fields are?
Or maybe even better, turn these into named fields? At your option :)
| diag.span_suggestions_with_style( | ||
| Span::default(), | ||
| help, | ||
| std::iter::empty::<String>(), | ||
| Applicability::Unspecified, | ||
| SuggestionStyle::HideCodeAlways, | ||
| ); |
There was a problem hiding this comment.
can't this just be diag.help(help)? Same for the other ones.
There was a problem hiding this comment.
if we use diag.help(help), there be a order change on output:
i think the left one seems better, since we first show a possible code suggestion and then a help message for more guidance.
There was a problem hiding this comment.
the option SuggestionStyle::HideCodeAlways is for this order.
There was a problem hiding this comment.
I don't disagree that this output is better, but what you're doing to emit it that way is not OK; the suggestion diagnostics machinery has a bunch of debug assertions for empty suggestions and this is just one particular code path that happens to evade all of them. So to me this feels like a hack, and I don't really want to add any of those here.
I recommend rewriting the help message so it can stand alone and doesn't need to reference another help message for context. Perhaps something like "if you're trying to use a dependency named dep, upgrade your edition to be able to reference it with a use declaration"
There was a problem hiding this comment.
Consider also amending the existing extern crate is required on Rust 2015 message like that;
on Rust 2015, `extern crate` is required to specifiy a dependency on an external crate
There was a problem hiding this comment.
Make sense, I updated it.
rustbot
added
S-waiting-on-author
1367919 to
6b6690e
Compare
|
@rustbot ready |
rustbot
added
S-waiting-on-review
6b6690e to
0152f8a
Compare
rust-bors
Bot
added
S-waiting-on-bors
Rollup of 5 pull requests Successful merges: - #159502 (Enhance suggestions for unresolved links with typos path) - #161052 (Add regression test for generic inference) - #161210 (Improve missing extern crate diagnostics in Rust 2015) - #161818 (Fix long type on diagnostics for conditionally implemented traits) - #161839 (Fix doc link to pointer::addr)
4 participants
Fixes #160026
closes #112809