-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Suggest correct enum variant on typo #56204
Conversation
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
5c666df
to
6b338e0
Compare
(&ty::Adt(adt_def, _substs), Def::Enum(_did)) => { | ||
let ty_str = ty.to_string(); | ||
// Incorrect enum variant | ||
let mut err = tcx.sess.struct_span_err( |
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.
Do we have criteria written down anywhere for which errors deserve a dedicated E0XXX code?
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.
My personal criteria has been when splitting existing errors to just continue what was there: create a new code when the existing one has one, don't bother when the existing doesn't, with the thinking of going back at a later time after the diagnostic has been seen in the wild for a bit. That being said, it'd probably be a good idea to require adding codes more aggressively than we do now. My only concern there would be when the error would be so all-encompassing that it's help text would be almost useless.
) { | ||
err.span_suggestion_with_applicability( | ||
span, | ||
"did you mean", |
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.
The span_suggestion
doc-coment advises against phrasing as a question, but I'm not feeling zealous/dogmatic enough to bikeshed it here.
I remember you said something once about doing a grand error-message style-and-consistency copyediting session, at which time we might come back and rephrase this and others?
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.
That's what I'm thinking yes. I don't like it either, but it is the wording used for this family of errors. I'd really like to come up with a wording that would work equally well for rustc
output and inline in VSCode, "did you mean" is not it.
r=me conditional on Travis success |
r? @zackmdavis @bors r+ |
📌 Commit 6b338e0 has been approved by |
Suggest correct enum variant on typo Fix rust-lang#37992.
Rollup of 14 pull requests Successful merges: - #56024 (Don't auto-inline const functions) - #56045 (Check arg/ret sizedness at ExprKind::Path) - #56072 (Stabilize macro_literal_matcher) - #56075 (Encode a custom "producers" section in wasm files) - #56100 (generator fields are not necessarily initialized) - #56101 (Incorporate `dyn` into more comments and docs.) - #56144 (Fix BTreeSet and BTreeMap gdb pretty-printers) - #56151 (Move a flaky process test out of libstd) - #56170 (Fix self profiler ICE on Windows) - #56176 (Panic setup msg) - #56204 (Suggest correct enum variant on typo) - #56207 (Stabilize the int_to_from_bytes feature) - #56210 (read_c_str should call the AllocationExtra hooks) - #56211 ([master] Forward-ports from beta) Failed merges: r? @ghost
Fix #37992.