Github Improve suggestion for tuple struct pattern matching errors. by reese · P...
source link: https://github.com/rust-lang/rust/pull/81235
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Some small tweaks requested, otherwise r=me.
match self.tcx.sess.source_map().span_to_snippet(field.pat.span) {
Ok(f) => {
// Field names are numbers, but numbers
// are not valid identifiers
if variant_field_idents.contains(&field.ident) {
String::from("_")
} else {
f
}
}
Err(_) => rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| {
s.print_pat(field.pat)
}),
}
LL | if let S { a, b, c, d } = S(1, 2, 3, 4) {
| ^^^^^^^^^^^^^^^^ help: use the tuple variant pattern syntax instead: `S(a, b, c, d)`
| -^^^^^^^^^^^^^^^
| |
| help: use the tuple variant pattern syntax instead: `(a, b, c, d)`
Comment on lines
4 to 7
estebank 28 days ago
Contributor
The span in all of the tests' suggestions seems to now be wrong. The resulting code would be (a, b, c, d)
instead of the desired S(a, b, c, d)
.
reese 28 days ago
Author
Contributor
The suggestion here seemed like you were suggesting to remove the path from the help text. It seems like I misunderstood the intent there, what were you suggesting to use instead of the path
?
estebank 28 days ago
Contributor
My proposal is to use span_suggestion_verbose
to have a separate suggestion subwindow, but for the suggestion to be correctly displayed you need to make a span trimming the S
from the beginning. The suggestion has to have the correct span for it to be rendered correctly.
In the case highlighted above, you had a span pointing at S { a, b, c, d }
. You now have a span pointing at S
. Creating a new span that starts at the end of S
's span, you'll have a span pointing at { a, b, c, d }
which would do then render correctly.
reese 22 days ago
Author
Contributor
Gotcha, having that suggestion with span_suggestion_verbose
makes a lot of sense. Thanks for the clarification, still figuring out all the options for these diagnostics
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK