From 8a83c8f64f717f7cf569f589b42c6535591f8854 Mon Sep 17 00:00:00 2001 From: Reese Williams Date: Wed, 20 Jan 2021 21:49:11 -0500 Subject: [PATCH 1/3] Improve suggestion for tuple struct pattern matching errors. Currently, when a user uses a struct pattern to pattern match on a tuple struct, the errors we emit generally suggest adding fields using their field names, which are numbers. However, numbers are not valid identifiers, so the suggestions, which use the shorthand notation, are not valid syntax. This commit changes those errors to suggest using the actual tuple struct pattern syntax instead, which is a more actionable suggestion. --- compiler/rustc_parse/src/parser/mod.rs | 2 +- compiler/rustc_parse/src/parser/pat.rs | 2 +- compiler/rustc_typeck/src/check/pat.rs | 98 ++++++++++++++++--- .../ui/structs/struct-tuple-field-names.rs | 15 +++ .../structs/struct-tuple-field-names.stderr | 15 +++ 5 files changed, 118 insertions(+), 14 deletions(-) create mode 100644 src/test/ui/structs/struct-tuple-field-names.rs create mode 100644 src/test/ui/structs/struct-tuple-field-names.stderr diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 45964b1c988ed..d8be90cd8452f 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -932,7 +932,7 @@ impl<'a> Parser<'a> { self.bump(); Ok(Ident::new(symbol, self.prev_token.span)) } else { - self.parse_ident_common(false) + self.parse_ident_common(true) } } diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs index 456e32680fe50..15b67ca9fafb7 100644 --- a/compiler/rustc_parse/src/parser/pat.rs +++ b/compiler/rustc_parse/src/parser/pat.rs @@ -999,7 +999,7 @@ impl<'a> Parser<'a> { let boxed_span = self.token.span; let is_ref = self.eat_keyword(kw::Ref); let is_mut = self.eat_keyword(kw::Mut); - let fieldname = self.parse_ident()?; + let fieldname = self.parse_field_name()?; hi = self.prev_token.span; let bind_type = match (is_ref, is_mut) { diff --git a/compiler/rustc_typeck/src/check/pat.rs b/compiler/rustc_typeck/src/check/pat.rs index 5fc573a57ad0b..f16f8c9caf2dd 100644 --- a/compiler/rustc_typeck/src/check/pat.rs +++ b/compiler/rustc_typeck/src/check/pat.rs @@ -16,6 +16,7 @@ use rustc_span::lev_distance::find_best_match_for_name; use rustc_span::source_map::{Span, Spanned}; use rustc_span::symbol::Ident; use rustc_trait_selection::traits::{ObligationCause, Pattern}; +use ty::VariantDef; use std::cmp; use std::collections::hash_map::Entry::{Occupied, Vacant}; @@ -1209,14 +1210,68 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { u.emit(); } } - (None, Some(mut err)) | (Some(mut err), None) => { + (None, Some(mut u)) => { + if let Some(mut e) = self.error_tuple_variant_as_struct_pat(pat, fields, variant) { + u.delay_as_bug(); + e.emit(); + } else { + u.emit(); + } + } + (Some(mut err), None) => { err.emit(); } - (None, None) => {} + (None, None) => { + if let Some(mut err) = + self.error_tuple_variant_index_shorthand(variant, pat, fields) + { + err.emit(); + } + } } no_field_errors } + fn error_tuple_variant_index_shorthand( + &self, + variant: &VariantDef, + pat: &'_ Pat<'_>, + fields: &[hir::FieldPat<'_>], + ) -> Option> { + // if this is a tuple struct, then all field names will be numbers + // so if any fields in a struct pattern use shorthand syntax, they will + // be invalid identifiers (for example, Foo { 0, 1 }). + if let (CtorKind::Fn, PatKind::Struct(qpath, field_patterns, ..)) = + (variant.ctor_kind, &pat.kind) + { + let has_shorthand_field_name = field_patterns.iter().any(|field| field.is_shorthand); + if has_shorthand_field_name { + let path = rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| { + s.print_qpath(qpath, false) + }); + let mut err = struct_span_err!( + self.tcx.sess, + pat.span, + E0769, + "tuple variant `{}` uses a bare index in a struct pattern", + path + ); + err.span_suggestion( + pat.span, + "use the tuple variant pattern syntax instead", + format!( + "{}({})", + path, + self.get_suggested_tuple_struct_pattern(fields, variant) + ), + Applicability::MaybeIncorrect, + ); + return Some(err); + } + } + None + } + fn error_foreign_non_exhaustive_spat(&self, pat: &Pat<'_>, descr: &str, no_fields: bool) { let sess = self.tcx.sess; let sm = sess.source_map(); @@ -1356,16 +1411,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); let (sugg, appl) = if fields.len() == variant.fields.len() { ( - fields - .iter() - .map(|f| match self.tcx.sess.source_map().span_to_snippet(f.pat.span) { - Ok(f) => f, - Err(_) => rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| { - s.print_pat(f.pat) - }), - }) - .collect::>() - .join(", "), + self.get_suggested_tuple_struct_pattern(fields, variant), Applicability::MachineApplicable, ) } else { @@ -1385,6 +1431,34 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { None } + fn get_suggested_tuple_struct_pattern( + &self, + fields: &[hir::FieldPat<'_>], + variant: &VariantDef, + ) -> String { + let variant_field_idents = variant.fields.iter().map(|f| f.ident).collect::>(); + fields + .iter() + .map(|field| { + 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) + }), + } + }) + .collect::>() + .join(", ") + } + /// Returns a diagnostic reporting a struct pattern which is missing an `..` due to /// inaccessible fields. /// diff --git a/src/test/ui/structs/struct-tuple-field-names.rs b/src/test/ui/structs/struct-tuple-field-names.rs new file mode 100644 index 0000000000000..0ebbff75e59ff --- /dev/null +++ b/src/test/ui/structs/struct-tuple-field-names.rs @@ -0,0 +1,15 @@ +struct S(i32, f32); +enum E { + S(i32, f32), +} +fn main() { + let x = E::S(1, 2.2); + match x { + E::S { 0, 1 } => {} + //~^ ERROR tuple variant `E::S` uses a bare index in a struct pattern [E0769] + } + let y = S(1, 2.2); + match y { + S { } => {} //~ ERROR: tuple variant `S` written as struct variant [E0769] + } +} diff --git a/src/test/ui/structs/struct-tuple-field-names.stderr b/src/test/ui/structs/struct-tuple-field-names.stderr new file mode 100644 index 0000000000000..2021aa9d70e39 --- /dev/null +++ b/src/test/ui/structs/struct-tuple-field-names.stderr @@ -0,0 +1,15 @@ +error[E0769]: tuple variant `E::S` uses a bare index in a struct pattern + --> $DIR/struct-tuple-field-names.rs:8:9 + | +LL | E::S { 0, 1 } => {} + | ^^^^^^^^^^^^^ help: use the tuple variant pattern syntax instead: `E::S(_, _)` + +error[E0769]: tuple variant `S` written as struct variant + --> $DIR/struct-tuple-field-names.rs:13:9 + | +LL | S { } => {} + | ^^^^^ help: use the tuple variant pattern syntax instead: `S(_, _)` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0769`. From 7879099ad3ef6c84d8df96521c30b7bf4c573615 Mon Sep 17 00:00:00 2001 From: Reese Williams Date: Sun, 14 Feb 2021 13:14:11 -0500 Subject: [PATCH 2/3] Clarify error message and remove pretty printing in help suggestions. --- compiler/rustc_typeck/src/check/pat.rs | 14 +++++--------- src/test/ui/issues/issue-17800.stderr | 4 +++- .../missing-fields-in-struct-pattern.stderr | 4 +++- src/test/ui/parser/recover-from-bad-variant.stderr | 4 +++- src/test/ui/structs/struct-tuple-field-names.rs | 2 +- .../ui/structs/struct-tuple-field-names.stderr | 10 +++++++--- src/test/ui/type/type-check/issue-41314.stderr | 4 +++- 7 files changed, 25 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_typeck/src/check/pat.rs b/compiler/rustc_typeck/src/check/pat.rs index f16f8c9caf2dd..cfcd0d673d015 100644 --- a/compiler/rustc_typeck/src/check/pat.rs +++ b/compiler/rustc_typeck/src/check/pat.rs @@ -1253,17 +1253,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.tcx.sess, pat.span, E0769, - "tuple variant `{}` uses a bare index in a struct pattern", + "tuple variant `{}` written as struct variant", path ); err.span_suggestion( - pat.span, + qpath.span().shrink_to_hi().until(pat.span), "use the tuple variant pattern syntax instead", - format!( - "{}({})", - path, - self.get_suggested_tuple_struct_pattern(fields, variant) - ), + format!("({})", self.get_suggested_tuple_struct_pattern(fields, variant)), Applicability::MaybeIncorrect, ); return Some(err); @@ -1421,9 +1417,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) }; err.span_suggestion( - pat.span, + qpath.span().shrink_to_hi().until(pat.span), "use the tuple variant pattern syntax instead", - format!("{}({})", path, sugg), + format!("({})", sugg), appl, ); return Some(err); diff --git a/src/test/ui/issues/issue-17800.stderr b/src/test/ui/issues/issue-17800.stderr index fc034a0cbf3b8..4f9618232313f 100644 --- a/src/test/ui/issues/issue-17800.stderr +++ b/src/test/ui/issues/issue-17800.stderr @@ -2,7 +2,9 @@ error[E0769]: tuple variant `MyOption::MySome` written as struct variant --> $DIR/issue-17800.rs:8:9 | LL | MyOption::MySome { x: 42 } => (), - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the tuple variant pattern syntax instead: `MyOption::MySome(42)` + | ----------------^^^^^^^^^^ + | | + | help: use the tuple variant pattern syntax instead: `(42)` error: aborting due to previous error diff --git a/src/test/ui/missing/missing-fields-in-struct-pattern.stderr b/src/test/ui/missing/missing-fields-in-struct-pattern.stderr index 6583524aad18f..81d208e4bc3f3 100644 --- a/src/test/ui/missing/missing-fields-in-struct-pattern.stderr +++ b/src/test/ui/missing/missing-fields-in-struct-pattern.stderr @@ -2,7 +2,9 @@ error[E0769]: tuple variant `S` written as struct variant --> $DIR/missing-fields-in-struct-pattern.rs:4:12 | 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)` error: aborting due to previous error diff --git a/src/test/ui/parser/recover-from-bad-variant.stderr b/src/test/ui/parser/recover-from-bad-variant.stderr index 89232a519d7b2..86086cf97ecce 100644 --- a/src/test/ui/parser/recover-from-bad-variant.stderr +++ b/src/test/ui/parser/recover-from-bad-variant.stderr @@ -22,7 +22,9 @@ error[E0769]: tuple variant `Enum::Bar` written as struct variant --> $DIR/recover-from-bad-variant.rs:12:9 | LL | Enum::Bar { a, b } => {} - | ^^^^^^^^^^^^^^^^^^ help: use the tuple variant pattern syntax instead: `Enum::Bar(a, b)` + | ---------^^^^^^^^^ + | | + | help: use the tuple variant pattern syntax instead: `(a, b)` error: aborting due to 3 previous errors diff --git a/src/test/ui/structs/struct-tuple-field-names.rs b/src/test/ui/structs/struct-tuple-field-names.rs index 0ebbff75e59ff..7bd54af1dbee5 100644 --- a/src/test/ui/structs/struct-tuple-field-names.rs +++ b/src/test/ui/structs/struct-tuple-field-names.rs @@ -6,7 +6,7 @@ fn main() { let x = E::S(1, 2.2); match x { E::S { 0, 1 } => {} - //~^ ERROR tuple variant `E::S` uses a bare index in a struct pattern [E0769] + //~^ ERROR tuple variant `E::S` written as struct variant [E0769] } let y = S(1, 2.2); match y { diff --git a/src/test/ui/structs/struct-tuple-field-names.stderr b/src/test/ui/structs/struct-tuple-field-names.stderr index 2021aa9d70e39..80c6187cbbef4 100644 --- a/src/test/ui/structs/struct-tuple-field-names.stderr +++ b/src/test/ui/structs/struct-tuple-field-names.stderr @@ -1,14 +1,18 @@ -error[E0769]: tuple variant `E::S` uses a bare index in a struct pattern +error[E0769]: tuple variant `E::S` written as struct variant --> $DIR/struct-tuple-field-names.rs:8:9 | LL | E::S { 0, 1 } => {} - | ^^^^^^^^^^^^^ help: use the tuple variant pattern syntax instead: `E::S(_, _)` + | ----^^^^^^^^^ + | | + | help: use the tuple variant pattern syntax instead: `(_, _)` error[E0769]: tuple variant `S` written as struct variant --> $DIR/struct-tuple-field-names.rs:13:9 | LL | S { } => {} - | ^^^^^ help: use the tuple variant pattern syntax instead: `S(_, _)` + | -^^^^ + | | + | help: use the tuple variant pattern syntax instead: `(_, _)` error: aborting due to 2 previous errors diff --git a/src/test/ui/type/type-check/issue-41314.stderr b/src/test/ui/type/type-check/issue-41314.stderr index bd4d2071c2059..78c14d37518c0 100644 --- a/src/test/ui/type/type-check/issue-41314.stderr +++ b/src/test/ui/type/type-check/issue-41314.stderr @@ -2,7 +2,9 @@ error[E0769]: tuple variant `X::Y` written as struct variant --> $DIR/issue-41314.rs:7:9 | LL | X::Y { number } => {} - | ^^^^^^^^^^^^^^^ help: use the tuple variant pattern syntax instead: `X::Y(number)` + | ----^^^^^^^^^^^ + | | + | help: use the tuple variant pattern syntax instead: `(number)` error: aborting due to previous error From d8540ae5a98b6135253521cdbf34c5953494a5bf Mon Sep 17 00:00:00 2001 From: Reese Williams Date: Sat, 20 Feb 2021 15:33:08 -0500 Subject: [PATCH 3/3] Fix suggestion span and move suggestions into new subwindow. --- compiler/rustc_typeck/src/check/pat.rs | 8 ++++---- src/test/ui/issues/issue-17800.stderr | 9 ++++++--- .../missing-fields-in-struct-pattern.stderr | 9 ++++++--- .../ui/parser/recover-from-bad-variant.stderr | 9 ++++++--- .../ui/structs/struct-tuple-field-names.stderr | 18 ++++++++++++------ src/test/ui/type/type-check/issue-41314.stderr | 9 ++++++--- 6 files changed, 40 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_typeck/src/check/pat.rs b/compiler/rustc_typeck/src/check/pat.rs index cfcd0d673d015..c21e7d8aebc55 100644 --- a/compiler/rustc_typeck/src/check/pat.rs +++ b/compiler/rustc_typeck/src/check/pat.rs @@ -1256,8 +1256,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { "tuple variant `{}` written as struct variant", path ); - err.span_suggestion( - qpath.span().shrink_to_hi().until(pat.span), + err.span_suggestion_verbose( + qpath.span().shrink_to_hi().to(pat.span.shrink_to_hi()), "use the tuple variant pattern syntax instead", format!("({})", self.get_suggested_tuple_struct_pattern(fields, variant)), Applicability::MaybeIncorrect, @@ -1416,8 +1416,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Applicability::MaybeIncorrect, ) }; - err.span_suggestion( - qpath.span().shrink_to_hi().until(pat.span), + err.span_suggestion_verbose( + qpath.span().shrink_to_hi().to(pat.span.shrink_to_hi()), "use the tuple variant pattern syntax instead", format!("({})", sugg), appl, diff --git a/src/test/ui/issues/issue-17800.stderr b/src/test/ui/issues/issue-17800.stderr index 4f9618232313f..7df86d7326bc6 100644 --- a/src/test/ui/issues/issue-17800.stderr +++ b/src/test/ui/issues/issue-17800.stderr @@ -2,9 +2,12 @@ error[E0769]: tuple variant `MyOption::MySome` written as struct variant --> $DIR/issue-17800.rs:8:9 | LL | MyOption::MySome { x: 42 } => (), - | ----------------^^^^^^^^^^ - | | - | help: use the tuple variant pattern syntax instead: `(42)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use the tuple variant pattern syntax instead + | +LL | MyOption::MySome(42) => (), + | ^^^^ error: aborting due to previous error diff --git a/src/test/ui/missing/missing-fields-in-struct-pattern.stderr b/src/test/ui/missing/missing-fields-in-struct-pattern.stderr index 81d208e4bc3f3..a95b5bb94d24a 100644 --- a/src/test/ui/missing/missing-fields-in-struct-pattern.stderr +++ b/src/test/ui/missing/missing-fields-in-struct-pattern.stderr @@ -2,9 +2,12 @@ error[E0769]: tuple variant `S` written as struct variant --> $DIR/missing-fields-in-struct-pattern.rs:4:12 | LL | if let S { a, b, c, d } = S(1, 2, 3, 4) { - | -^^^^^^^^^^^^^^^ - | | - | help: use the tuple variant pattern syntax instead: `(a, b, c, d)` + | ^^^^^^^^^^^^^^^^ + | +help: use the tuple variant pattern syntax instead + | +LL | if let S(a, b, c, d) = S(1, 2, 3, 4) { + | ^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/parser/recover-from-bad-variant.stderr b/src/test/ui/parser/recover-from-bad-variant.stderr index 86086cf97ecce..9b9d2bc4972b4 100644 --- a/src/test/ui/parser/recover-from-bad-variant.stderr +++ b/src/test/ui/parser/recover-from-bad-variant.stderr @@ -22,9 +22,12 @@ error[E0769]: tuple variant `Enum::Bar` written as struct variant --> $DIR/recover-from-bad-variant.rs:12:9 | LL | Enum::Bar { a, b } => {} - | ---------^^^^^^^^^ - | | - | help: use the tuple variant pattern syntax instead: `(a, b)` + | ^^^^^^^^^^^^^^^^^^ + | +help: use the tuple variant pattern syntax instead + | +LL | Enum::Bar(a, b) => {} + | ^^^^^^ error: aborting due to 3 previous errors diff --git a/src/test/ui/structs/struct-tuple-field-names.stderr b/src/test/ui/structs/struct-tuple-field-names.stderr index 80c6187cbbef4..29e721465215d 100644 --- a/src/test/ui/structs/struct-tuple-field-names.stderr +++ b/src/test/ui/structs/struct-tuple-field-names.stderr @@ -2,17 +2,23 @@ error[E0769]: tuple variant `E::S` written as struct variant --> $DIR/struct-tuple-field-names.rs:8:9 | LL | E::S { 0, 1 } => {} - | ----^^^^^^^^^ - | | - | help: use the tuple variant pattern syntax instead: `(_, _)` + | ^^^^^^^^^^^^^ + | +help: use the tuple variant pattern syntax instead + | +LL | E::S(_, _) => {} + | ^^^^^^ error[E0769]: tuple variant `S` written as struct variant --> $DIR/struct-tuple-field-names.rs:13:9 | LL | S { } => {} - | -^^^^ - | | - | help: use the tuple variant pattern syntax instead: `(_, _)` + | ^^^^^ + | +help: use the tuple variant pattern syntax instead + | +LL | S(_, _) => {} + | ^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/type/type-check/issue-41314.stderr b/src/test/ui/type/type-check/issue-41314.stderr index 78c14d37518c0..c3d41ae68cd6a 100644 --- a/src/test/ui/type/type-check/issue-41314.stderr +++ b/src/test/ui/type/type-check/issue-41314.stderr @@ -2,9 +2,12 @@ error[E0769]: tuple variant `X::Y` written as struct variant --> $DIR/issue-41314.rs:7:9 | LL | X::Y { number } => {} - | ----^^^^^^^^^^^ - | | - | help: use the tuple variant pattern syntax instead: `(number)` + | ^^^^^^^^^^^^^^^ + | +help: use the tuple variant pattern syntax instead + | +LL | X::Y(number) => {} + | ^^^^^^^^ error: aborting due to previous error