From 8f4a5429c2cf7ce1f0a63f821484744024b364f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 29 May 2018 16:37:06 -0700 Subject: [PATCH 1/3] Accept `..` in incorrect position to avoid further errors We currently give a specific message when encountering a `..` anywhere other than the end of a pattern. Modify the parser to accept it (while still emitting the error) so that we don't also trigger "missing fields in pattern" errors afterwards. --- src/libsyntax/parse/parser.rs | 10 ++++++++++ src/test/ui/issue-49257.rs | 1 - src/test/ui/issue-49257.stderr | 8 +------- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 28f93328e9534..5b5ecc4147b67 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3749,6 +3749,16 @@ impl<'a> Parser<'a> { err.span_label(self.span, "`..` must be in the last position, \ and cannot have a trailing comma"); + if self.look_ahead(1, |t| { + t == &token::CloseDelim(token::Brace) || t.is_ident() + }) { + // If the struct looks otherwise well formed, recover and continue. + // This way we avoid "pattern missing fields" errors afterwards. + err.emit(); + self.bump(); + etc = true; + break; + } } else { err.span_label(self.span, "expected `}`"); } diff --git a/src/test/ui/issue-49257.rs b/src/test/ui/issue-49257.rs index a319849223740..edb4037d109e5 100644 --- a/src/test/ui/issue-49257.rs +++ b/src/test/ui/issue-49257.rs @@ -18,5 +18,4 @@ struct Point { x: u8, y: u8 } fn main() { let p = Point { x: 0, y: 0 }; let Point { .., y } = p; //~ ERROR expected `}`, found `,` - //~| ERROR pattern does not mention fields `x`, `y` } diff --git a/src/test/ui/issue-49257.stderr b/src/test/ui/issue-49257.stderr index fec990764bb14..6ca8d80fa2e09 100644 --- a/src/test/ui/issue-49257.stderr +++ b/src/test/ui/issue-49257.stderr @@ -4,12 +4,6 @@ error: expected `}`, found `,` LL | let Point { .., y } = p; //~ ERROR expected `}`, found `,` | ^ `..` must be in the last position, and cannot have a trailing comma -error[E0027]: pattern does not mention fields `x`, `y` - --> $DIR/issue-49257.rs:20:9 - | -LL | let Point { .., y } = p; //~ ERROR expected `}`, found `,` - | ^^^^^^^^^^^^^^^ missing fields `x`, `y` - -error: aborting due to 2 previous errors +error: aborting due to previous error For more information about this error, try `rustc --explain E0027`. From cbc70a0d68bae1d439eb679676c499d543e61396 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 29 May 2018 22:31:00 -0700 Subject: [PATCH 2/3] Improve diagnostics for incorrect `..` usage When using `..` somewhere other than the end, parse the rest of the pattern correctly while still emitting an error. Add suggestions to either remove trailing `,` or moving the `..` to the end. --- src/libsyntax/parse/parser.rs | 214 +++++++++++++++++++++------------ src/test/ui/issue-49257.rs | 3 + src/test/ui/issue-49257.stderr | 35 +++++- 3 files changed, 170 insertions(+), 82 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 5b5ecc4147b67..fb2aa0e9d5afc 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3710,26 +3710,89 @@ impl<'a> Parser<'a> { Ok((before, slice, after)) } + fn parse_pat_field( + &mut self, + lo: Span, + attrs: Vec + ) -> PResult<'a, codemap::Spanned> { + // Check if a colon exists one ahead. This means we're parsing a fieldname. + let hi; + let (subpat, fieldname, is_shorthand) = if self.look_ahead(1, |t| t == &token::Colon) { + // Parsing a pattern of the form "fieldname: pat" + let fieldname = self.parse_field_name()?; + self.bump(); + let pat = self.parse_pat()?; + hi = pat.span; + (pat, fieldname, false) + } else { + // Parsing a pattern of the form "(box) (ref) (mut) fieldname" + let is_box = self.eat_keyword(keywords::Box); + let boxed_span = self.span; + let is_ref = self.eat_keyword(keywords::Ref); + let is_mut = self.eat_keyword(keywords::Mut); + let fieldname = self.parse_ident()?; + hi = self.prev_span; + + let bind_type = match (is_ref, is_mut) { + (true, true) => BindingMode::ByRef(Mutability::Mutable), + (true, false) => BindingMode::ByRef(Mutability::Immutable), + (false, true) => BindingMode::ByValue(Mutability::Mutable), + (false, false) => BindingMode::ByValue(Mutability::Immutable), + }; + let fieldpat = P(Pat { + id: ast::DUMMY_NODE_ID, + node: PatKind::Ident(bind_type, fieldname, None), + span: boxed_span.to(hi), + }); + + let subpat = if is_box { + P(Pat { + id: ast::DUMMY_NODE_ID, + node: PatKind::Box(fieldpat), + span: lo.to(hi), + }) + } else { + fieldpat + }; + (subpat, fieldname, true) + }; + + Ok(codemap::Spanned { + span: lo.to(hi), + node: ast::FieldPat { + ident: fieldname, + pat: subpat, + is_shorthand, + attrs: attrs.into(), + } + }) + } + /// Parse the fields of a struct-like pattern fn parse_pat_fields(&mut self) -> PResult<'a, (Vec>, bool)> { let mut fields = Vec::new(); let mut etc = false; - let mut first = true; - while self.token != token::CloseDelim(token::Brace) { - if first { - first = false; - } else { - self.expect(&token::Comma)?; - // accept trailing commas - if self.check(&token::CloseDelim(token::Brace)) { break } - } + let mut ate_comma = true; + let mut delayed_err: Option> = None; + let mut etc_span = None; + while self.token != token::CloseDelim(token::Brace) { let attrs = self.parse_outer_attributes()?; let lo = self.span; - let hi; + + // check that a comma comes after every field + if !ate_comma { + let err = self.struct_span_err(self.prev_span, "expected `,`"); + return Err(err); + } + ate_comma = false; if self.check(&token::DotDot) || self.token == token::DotDotDot { + etc = true; + let mut etc_sp = self.span; + if self.token == token::DotDotDot { // Issue #46718 + // Accept `...` as if it were `..` to avoid further errors let mut err = self.struct_span_err(self.span, "expected field pattern, found `...`"); err.span_suggestion_with_applicability( @@ -3740,83 +3803,76 @@ impl<'a> Parser<'a> { ); err.emit(); } + self.bump(); // `..` || `...`:w - self.bump(); - if self.token != token::CloseDelim(token::Brace) { - let token_str = self.this_token_to_string(); - let mut err = self.fatal(&format!("expected `{}`, found `{}`", "}", token_str)); - if self.token == token::Comma { // Issue #49257 - err.span_label(self.span, - "`..` must be in the last position, \ - and cannot have a trailing comma"); - if self.look_ahead(1, |t| { - t == &token::CloseDelim(token::Brace) || t.is_ident() - }) { - // If the struct looks otherwise well formed, recover and continue. - // This way we avoid "pattern missing fields" errors afterwards. - err.emit(); - self.bump(); - etc = true; - break; - } + if self.token == token::CloseDelim(token::Brace) { + etc_span = Some(etc_sp); + break; + } + let token_str = self.this_token_to_string(); + let mut err = self.fatal(&format!("expected `}}`, found `{}`", token_str)); + + err.span_label(self.span, "expected `}`"); + let mut comma_sp = None; + if self.token == token::Comma { // Issue #49257 + etc_sp = etc_sp.to(self.sess.codemap().span_until_non_whitespace(self.span)); + err.span_label(etc_sp, + "`..` must be at the end and cannot have a trailing comma"); + comma_sp = Some(self.span); + self.bump(); + ate_comma = true; + } + + etc_span = Some(etc_sp); + if self.token == token::CloseDelim(token::Brace) { + // If the struct looks otherwise well formed, recover and continue. + if let Some(sp) = comma_sp { + err.span_suggestion_short(sp, "remove this comma", "".into()); + } + err.emit(); + break; + } else if self.token.is_ident() && ate_comma { + // Accept fields coming after `..,`. + // This way we avoid "pattern missing fields" errors afterwards. + // We delay this error until the end in order to have a span for a + // suggested fix. + if let Some(mut delayed_err) = delayed_err { + delayed_err.emit(); + return Err(err); } else { - err.span_label(self.span, "expected `}`"); + delayed_err = Some(err); + } + } else { + if let Some(mut err) = delayed_err { + err.emit(); } return Err(err); } - etc = true; - break; } - // Check if a colon exists one ahead. This means we're parsing a fieldname. - let (subpat, fieldname, is_shorthand) = if self.look_ahead(1, |t| t == &token::Colon) { - // Parsing a pattern of the form "fieldname: pat" - let fieldname = self.parse_field_name()?; - self.bump(); - let pat = self.parse_pat()?; - hi = pat.span; - (pat, fieldname, false) - } else { - // Parsing a pattern of the form "(box) (ref) (mut) fieldname" - let is_box = self.eat_keyword(keywords::Box); - let boxed_span = self.span; - let is_ref = self.eat_keyword(keywords::Ref); - let is_mut = self.eat_keyword(keywords::Mut); - let fieldname = self.parse_ident()?; - hi = self.prev_span; - - let bind_type = match (is_ref, is_mut) { - (true, true) => BindingMode::ByRef(Mutability::Mutable), - (true, false) => BindingMode::ByRef(Mutability::Immutable), - (false, true) => BindingMode::ByValue(Mutability::Mutable), - (false, false) => BindingMode::ByValue(Mutability::Immutable), - }; - let fieldpat = P(Pat { - id: ast::DUMMY_NODE_ID, - node: PatKind::Ident(bind_type, fieldname, None), - span: boxed_span.to(hi), - }); - - let subpat = if is_box { - P(Pat { - id: ast::DUMMY_NODE_ID, - node: PatKind::Box(fieldpat), - span: lo.to(hi), - }) - } else { - fieldpat - }; - (subpat, fieldname, true) - }; - - fields.push(codemap::Spanned { span: lo.to(hi), - node: ast::FieldPat { - ident: fieldname, - pat: subpat, - is_shorthand, - attrs: attrs.into(), - } + fields.push(match self.parse_pat_field(lo, attrs) { + Ok(field) => field, + Err(err) => { + if let Some(mut delayed_err) = delayed_err { + delayed_err.emit(); + } + return Err(err); + } }); + ate_comma = self.eat(&token::Comma); + } + + if let Some(mut err) = delayed_err { + if let Some(etc_span) = etc_span { + err.multipart_suggestion( + "move the `..` to the end of the field list", + vec![ + (etc_span, "".into()), + (self.span, ", .. }".into()), + ], + ); + } + err.emit(); } return Ok((fields, etc)); } diff --git a/src/test/ui/issue-49257.rs b/src/test/ui/issue-49257.rs index edb4037d109e5..f288a2b217428 100644 --- a/src/test/ui/issue-49257.rs +++ b/src/test/ui/issue-49257.rs @@ -17,5 +17,8 @@ struct Point { x: u8, y: u8 } fn main() { let p = Point { x: 0, y: 0 }; + let Point { .., y, } = p; //~ ERROR expected `}`, found `,` let Point { .., y } = p; //~ ERROR expected `}`, found `,` + let Point { .., } = p; //~ ERROR expected `}`, found `,` + let Point { .. } = p; } diff --git a/src/test/ui/issue-49257.stderr b/src/test/ui/issue-49257.stderr index 6ca8d80fa2e09..7c3e7df4a1f32 100644 --- a/src/test/ui/issue-49257.stderr +++ b/src/test/ui/issue-49257.stderr @@ -1,9 +1,38 @@ error: expected `}`, found `,` --> $DIR/issue-49257.rs:20:19 | +LL | let Point { .., y, } = p; //~ ERROR expected `}`, found `,` + | --^ + | | | + | | expected `}` + | `..` must be at the end and cannot have a trailing comma +help: move the `..` to the end of the field list + | +LL | let Point { y, , .. } = p; //~ ERROR expected `}`, found `,` + | -- ^^^^^^ + +error: expected `}`, found `,` + --> $DIR/issue-49257.rs:21:19 + | LL | let Point { .., y } = p; //~ ERROR expected `}`, found `,` - | ^ `..` must be in the last position, and cannot have a trailing comma + | --^ + | | | + | | expected `}` + | `..` must be at the end and cannot have a trailing comma +help: move the `..` to the end of the field list + | +LL | let Point { y , .. } = p; //~ ERROR expected `}`, found `,` + | -- ^^^^^^ + +error: expected `}`, found `,` + --> $DIR/issue-49257.rs:22:19 + | +LL | let Point { .., } = p; //~ ERROR expected `}`, found `,` + | --^ + | | | + | | expected `}` + | | help: remove this comma + | `..` must be at the end and cannot have a trailing comma -error: aborting due to previous error +error: aborting due to 3 previous errors -For more information about this error, try `rustc --explain E0027`. From d66d35bb9184b7f3a4e155386ca44410c0d09ff2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 30 May 2018 11:04:39 -0700 Subject: [PATCH 3/3] Account for comma in suggestion --- src/libsyntax/parse/parser.rs | 2 +- src/test/parse-fail/bind-struct-early-modifiers.rs | 2 +- src/test/ui/issue-49257.stderr | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index fb2aa0e9d5afc..d1529347ee0da 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3868,7 +3868,7 @@ impl<'a> Parser<'a> { "move the `..` to the end of the field list", vec![ (etc_span, "".into()), - (self.span, ", .. }".into()), + (self.span, format!("{}.. }}", if ate_comma { "" } else { ", " })), ], ); } diff --git a/src/test/parse-fail/bind-struct-early-modifiers.rs b/src/test/parse-fail/bind-struct-early-modifiers.rs index 25348a3bfd18f..e9e76af11a530 100644 --- a/src/test/parse-fail/bind-struct-early-modifiers.rs +++ b/src/test/parse-fail/bind-struct-early-modifiers.rs @@ -13,7 +13,7 @@ fn main() { struct Foo { x: isize } match (Foo { x: 10 }) { - Foo { ref x: ref x } => {}, //~ ERROR expected `,`, found `:` + Foo { ref x: ref x } => {}, //~ ERROR expected `,` _ => {} } } diff --git a/src/test/ui/issue-49257.stderr b/src/test/ui/issue-49257.stderr index 7c3e7df4a1f32..40179832b49b2 100644 --- a/src/test/ui/issue-49257.stderr +++ b/src/test/ui/issue-49257.stderr @@ -8,8 +8,8 @@ LL | let Point { .., y, } = p; //~ ERROR expected `}`, found `,` | `..` must be at the end and cannot have a trailing comma help: move the `..` to the end of the field list | -LL | let Point { y, , .. } = p; //~ ERROR expected `}`, found `,` - | -- ^^^^^^ +LL | let Point { y, .. } = p; //~ ERROR expected `}`, found `,` + | -- ^^^^ error: expected `}`, found `,` --> $DIR/issue-49257.rs:21:19