Skip to content

Commit

Permalink
Auto merge of #51201 - estebank:dotdot, r=petrochenkov
Browse files Browse the repository at this point in the history 
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.

Add suggestions to either remove trailing `,` or moving the `..` to the
end.

Follow up to #49268.
  • Loading branch information
@bors
bors committed Jun 6, 2018
2 parents 9ac3725 + d66d35b commit 35aeecb
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 78 deletions.
204 changes: 135 additions & 69 deletions src/libsyntax/parse/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3710,26 +3710,89 @@ impl<'a> Parser<'a> {
Ok((before, slice, after))
}

fn parse_pat_field(
&mut self,
lo: Span,
attrs: Vec<Attribute>
) -> PResult<'a, codemap::Spanned<ast::FieldPat>> {
// 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<codemap::Spanned<ast::FieldPat>>, 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<DiagnosticBuilder<'a>> = 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(
Expand All @@ -3740,73 +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.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, format!("{}.. }}", if ate_comma { "" } else { ", " })),
],
);
}
err.emit();
}
return Ok((fields, etc));
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/parse-fail/bind-struct-early-modifiers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 `,`
_ => {}
}
}
4 changes: 3 additions & 1 deletion src/test/ui/issue-49257.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +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 `,`
//~| ERROR pattern does not mention fields `x`, `y`
let Point { .., } = p; //~ ERROR expected `}`, found `,`
let Point { .. } = p;
}
37 changes: 30 additions & 7 deletions src/test/ui/issue-49257.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,38 @@
error: expected `}`, found `,`
--> $DIR/issue-49257.rs:20:19
|
LL | let Point { .., y } = p; //~ ERROR expected `}`, found `,`
| ^ `..` must be in the last position, and cannot have a trailing comma
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[E0027]: pattern does not mention fields `x`, `y`
--> $DIR/issue-49257.rs:20:9
error: expected `}`, found `,`
--> $DIR/issue-49257.rs:21:19
|
LL | let Point { .., y } = p; //~ ERROR expected `}`, found `,`
| ^^^^^^^^^^^^^^^ missing fields `x`, `y`
| --^
| | |
| | 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 2 previous errors
error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0027`.

0 comments on commit 35aeecb

Please sign in to comment.