From 532dd4475b553212a65a52df6ff511e4822b149a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 8 Mar 2019 14:35:38 -0800 Subject: [PATCH 1/4] Recover from missing comma between enum variants --- src/libsyntax/parse/parser.rs | 22 ++++++++++++++++++++-- src/test/ui/issues/issue-28433.rs | 13 +++++++------ src/test/ui/issues/issue-28433.stderr | 20 ++++++++++++++------ src/test/ui/parser/recover-enum.rs | 8 ++++++-- src/test/ui/parser/recover-enum.stderr | 22 ++++++++++++++++------ 5 files changed, 63 insertions(+), 22 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index aa70c54a1ef8a..8ce29c9f29879 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -7711,11 +7711,29 @@ impl<'a> Parser<'a> { }; variants.push(respan(vlo.to(self.prev_span), vr)); - if !self.eat(&token::Comma) { break; } + if !self.eat(&token::Comma) { + if self.token.is_ident() && + !self.token.is_special_ident() && + !self.token.is_used_keyword() && + !self.token.is_unused_keyword() + { + let sp = self.sess.source_map().next_point(self.prev_span); + let mut err = self.struct_span_err(sp, "missing comma"); + err.span_suggestion_short( + sp, + "missing comma", + ",".to_owned(), + Applicability::MaybeIncorrect, + ); + err.emit(); + } else { + break; + } + } } self.expect(&token::CloseDelim(token::Brace))?; if !any_disr.is_empty() && !all_nullary { - let mut err =self.struct_span_err( + let mut err = self.struct_span_err( any_disr.clone(), "discriminator values can only be used with a field-less enum", ); diff --git a/src/test/ui/issues/issue-28433.rs b/src/test/ui/issues/issue-28433.rs index a87ac63784fc6..be48ade513138 100644 --- a/src/test/ui/issues/issue-28433.rs +++ b/src/test/ui/issues/issue-28433.rs @@ -1,13 +1,14 @@ // compile-flags: -Z continue-parse-after-error -enum bird { - pub duck, - //~^ ERROR: expected identifier, found keyword `pub` - //~| ERROR: expected - goose +enum Bird { + pub Duck, + //~^ ERROR expected identifier, found keyword `pub` + //~| ERROR missing comma + //~| WARN variant `pub` should have an upper camel case name + Goose } fn main() { - let y = bird::goose; + let y = Bird::Goose; } diff --git a/src/test/ui/issues/issue-28433.stderr b/src/test/ui/issues/issue-28433.stderr index d3cba3aae7101..2463969476d4e 100644 --- a/src/test/ui/issues/issue-28433.stderr +++ b/src/test/ui/issues/issue-28433.stderr @@ -1,18 +1,26 @@ error: expected identifier, found keyword `pub` --> $DIR/issue-28433.rs:4:5 | -LL | pub duck, +LL | pub Duck, | ^^^ expected identifier, found keyword help: you can escape reserved keywords to use them as identifiers | -LL | r#pub duck, +LL | r#pub Duck, | ^^^^^ -error: expected one of `(`, `,`, `=`, `{`, or `}`, found `duck` - --> $DIR/issue-28433.rs:4:9 +error: missing comma + --> $DIR/issue-28433.rs:4:8 | -LL | pub duck, - | ^^^^ expected one of `(`, `,`, `=`, `{`, or `}` here +LL | pub Duck, + | ^ help: missing comma + +warning: variant `pub` should have an upper camel case name + --> $DIR/issue-28433.rs:4:5 + | +LL | pub Duck, + | ^^^ help: convert the identifier to upper camel case: `Pub` + | + = note: #[warn(non_camel_case_types)] on by default error: aborting due to 2 previous errors diff --git a/src/test/ui/parser/recover-enum.rs b/src/test/ui/parser/recover-enum.rs index 204ad85716564..da42da84acf2e 100644 --- a/src/test/ui/parser/recover-enum.rs +++ b/src/test/ui/parser/recover-enum.rs @@ -3,7 +3,11 @@ fn main() { enum Test { Very - Bad //~ ERROR found `Bad` - Stuff + //~^ ERROR missing comma + Bad(usize) + //~^ ERROR missing comma + Stuff { a: usize } + //~^ ERROR missing comma + Here } } diff --git a/src/test/ui/parser/recover-enum.stderr b/src/test/ui/parser/recover-enum.stderr index 8c3448d6fbe41..10b4aba4053bd 100644 --- a/src/test/ui/parser/recover-enum.stderr +++ b/src/test/ui/parser/recover-enum.stderr @@ -1,10 +1,20 @@ -error: expected one of `(`, `,`, `=`, `{`, or `}`, found `Bad` - --> $DIR/recover-enum.rs:6:9 +error: missing comma + --> $DIR/recover-enum.rs:5:13 | LL | Very - | - expected one of `(`, `,`, `=`, `{`, or `}` here -LL | Bad - | ^^^ unexpected token + | ^ help: missing comma -error: aborting due to previous error +error: missing comma + --> $DIR/recover-enum.rs:7:19 + | +LL | Bad(usize) + | ^ help: missing comma + +error: missing comma + --> $DIR/recover-enum.rs:9:27 + | +LL | Stuff { a: usize } + | ^ help: missing comma + +error: aborting due to 3 previous errors From b2b9555f95851cd24bcda6801a41ad9a1dfa4ae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 8 Mar 2019 15:12:51 -0800 Subject: [PATCH 2/4] Recover from incorrect `pub` kw in "reasonable" places --- src/libsyntax/parse/parser.rs | 14 ++++++++++- src/test/ui/issues/issue-28433.rs | 4 +--- src/test/ui/issues/issue-28433.stderr | 24 +++---------------- src/test/ui/parser/trait-pub-assoc-const.rs | 2 +- .../ui/parser/trait-pub-assoc-const.stderr | 6 ++--- src/test/ui/parser/trait-pub-assoc-ty.rs | 2 +- src/test/ui/parser/trait-pub-assoc-ty.stderr | 6 ++--- src/test/ui/parser/trait-pub-method.rs | 2 +- src/test/ui/parser/trait-pub-method.stderr | 6 ++--- 9 files changed, 26 insertions(+), 40 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 8ce29c9f29879..ce643545e7118 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1524,7 +1524,7 @@ impl<'a> Parser<'a> { at_end: &mut bool, mut attrs: Vec) -> PResult<'a, TraitItem> { let lo = self.span; - + self.eat_bad_pub(); let (name, node, generics) = if self.eat_keyword(keywords::Type) { self.parse_trait_item_assoc_ty()? } else if self.is_const_item() { @@ -7680,6 +7680,7 @@ impl<'a> Parser<'a> { let struct_def; let mut disr_expr = None; + self.eat_bad_pub(); let ident = self.parse_ident()?; if self.check(&token::OpenDelim(token::Brace)) { // Parse a struct variant. @@ -8618,6 +8619,17 @@ impl<'a> Parser<'a> { Applicability::MaybeIncorrect, ).emit(); } + + /// Recover from `pub` keyword in places where it seems _reasonable_ but isn't valid. + fn eat_bad_pub(&mut self) { + if self.token.is_keyword(keywords::Pub) { + self.bump(); + let mut err = self.diagnostic() + .struct_span_err(self.prev_span, "unnecessary visibility qualifier"); + err.span_label(self.prev_span, "`pub` not permitted here"); + err.emit(); + } + } } pub fn emit_unclosed_delims(unclosed_delims: &mut Vec, handler: &errors::Handler) { diff --git a/src/test/ui/issues/issue-28433.rs b/src/test/ui/issues/issue-28433.rs index be48ade513138..229b334a596cd 100644 --- a/src/test/ui/issues/issue-28433.rs +++ b/src/test/ui/issues/issue-28433.rs @@ -2,9 +2,7 @@ enum Bird { pub Duck, - //~^ ERROR expected identifier, found keyword `pub` - //~| ERROR missing comma - //~| WARN variant `pub` should have an upper camel case name + //~^ ERROR unnecessary visibility qualifier Goose } diff --git a/src/test/ui/issues/issue-28433.stderr b/src/test/ui/issues/issue-28433.stderr index 2463969476d4e..51be15a3e316a 100644 --- a/src/test/ui/issues/issue-28433.stderr +++ b/src/test/ui/issues/issue-28433.stderr @@ -1,26 +1,8 @@ -error: expected identifier, found keyword `pub` +error: unnecessary visibility qualifier --> $DIR/issue-28433.rs:4:5 | LL | pub Duck, - | ^^^ expected identifier, found keyword -help: you can escape reserved keywords to use them as identifiers - | -LL | r#pub Duck, - | ^^^^^ - -error: missing comma - --> $DIR/issue-28433.rs:4:8 - | -LL | pub Duck, - | ^ help: missing comma - -warning: variant `pub` should have an upper camel case name - --> $DIR/issue-28433.rs:4:5 - | -LL | pub Duck, - | ^^^ help: convert the identifier to upper camel case: `Pub` - | - = note: #[warn(non_camel_case_types)] on by default + | ^^^ `pub` not permitted here -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/parser/trait-pub-assoc-const.rs b/src/test/ui/parser/trait-pub-assoc-const.rs index 3ee2dc1ae18fb..219ffa309c254 100644 --- a/src/test/ui/parser/trait-pub-assoc-const.rs +++ b/src/test/ui/parser/trait-pub-assoc-const.rs @@ -1,6 +1,6 @@ trait Foo { pub const Foo: u32; - //~^ ERROR expected one of `async`, `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found + //~^ ERROR unnecessary visibility qualifier } fn main() {} diff --git a/src/test/ui/parser/trait-pub-assoc-const.stderr b/src/test/ui/parser/trait-pub-assoc-const.stderr index 8fc9ae4cf28b4..817692cc82c86 100644 --- a/src/test/ui/parser/trait-pub-assoc-const.stderr +++ b/src/test/ui/parser/trait-pub-assoc-const.stderr @@ -1,10 +1,8 @@ -error: expected one of `async`, `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found `pub` +error: unnecessary visibility qualifier --> $DIR/trait-pub-assoc-const.rs:2:5 | -LL | trait Foo { - | - expected one of 7 possible tokens here LL | pub const Foo: u32; - | ^^^ unexpected token + | ^^^ `pub` not permitted here error: aborting due to previous error diff --git a/src/test/ui/parser/trait-pub-assoc-ty.rs b/src/test/ui/parser/trait-pub-assoc-ty.rs index 042addfd1a434..a78dfbdcddaad 100644 --- a/src/test/ui/parser/trait-pub-assoc-ty.rs +++ b/src/test/ui/parser/trait-pub-assoc-ty.rs @@ -1,6 +1,6 @@ trait Foo { pub type Foo; - //~^ ERROR expected one of `async`, `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found + //~^ ERROR unnecessary visibility qualifier } fn main() {} diff --git a/src/test/ui/parser/trait-pub-assoc-ty.stderr b/src/test/ui/parser/trait-pub-assoc-ty.stderr index b8eab4e87bf0b..400be6af22a82 100644 --- a/src/test/ui/parser/trait-pub-assoc-ty.stderr +++ b/src/test/ui/parser/trait-pub-assoc-ty.stderr @@ -1,10 +1,8 @@ -error: expected one of `async`, `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found `pub` +error: unnecessary visibility qualifier --> $DIR/trait-pub-assoc-ty.rs:2:5 | -LL | trait Foo { - | - expected one of 7 possible tokens here LL | pub type Foo; - | ^^^ unexpected token + | ^^^ `pub` not permitted here error: aborting due to previous error diff --git a/src/test/ui/parser/trait-pub-method.rs b/src/test/ui/parser/trait-pub-method.rs index 9675182c1561e..1f6ee028a174b 100644 --- a/src/test/ui/parser/trait-pub-method.rs +++ b/src/test/ui/parser/trait-pub-method.rs @@ -1,6 +1,6 @@ trait Foo { pub fn foo(); - //~^ ERROR expected one of `async`, `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found + //~^ ERROR unnecessary visibility qualifier } fn main() {} diff --git a/src/test/ui/parser/trait-pub-method.stderr b/src/test/ui/parser/trait-pub-method.stderr index d4db961c3fa9c..b3617a4aa9b01 100644 --- a/src/test/ui/parser/trait-pub-method.stderr +++ b/src/test/ui/parser/trait-pub-method.stderr @@ -1,10 +1,8 @@ -error: expected one of `async`, `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found `pub` +error: unnecessary visibility qualifier --> $DIR/trait-pub-method.rs:2:5 | -LL | trait Foo { - | - expected one of 7 possible tokens here LL | pub fn foo(); - | ^^^ unexpected token + | ^^^ `pub` not permitted here error: aborting due to previous error From 1aa43af37018f285544b234d65b24efb465d18cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 9 Mar 2019 15:14:22 -0800 Subject: [PATCH 3/4] parse full visibility when recovering --- src/libsyntax/parse/parser.rs | 14 +++++++++----- src/test/ui/issues/issue-28433.rs | 4 +++- src/test/ui/issues/issue-28433.stderr | 8 +++++++- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index ce643545e7118..0ec8cb51fa75c 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -8623,11 +8623,15 @@ impl<'a> Parser<'a> { /// Recover from `pub` keyword in places where it seems _reasonable_ but isn't valid. fn eat_bad_pub(&mut self) { if self.token.is_keyword(keywords::Pub) { - self.bump(); - let mut err = self.diagnostic() - .struct_span_err(self.prev_span, "unnecessary visibility qualifier"); - err.span_label(self.prev_span, "`pub` not permitted here"); - err.emit(); + match self.parse_visibility(false) { + Ok(vis) => { + let mut err = self.diagnostic() + .struct_span_err(vis.span, "unnecessary visibility qualifier"); + err.span_label(vis.span, "`pub` not permitted here"); + err.emit(); + } + Err(mut err) => err.emit(), + } } } } diff --git a/src/test/ui/issues/issue-28433.rs b/src/test/ui/issues/issue-28433.rs index 229b334a596cd..2bbb32bf2b372 100644 --- a/src/test/ui/issues/issue-28433.rs +++ b/src/test/ui/issues/issue-28433.rs @@ -3,7 +3,9 @@ enum Bird { pub Duck, //~^ ERROR unnecessary visibility qualifier - Goose + Goose, + pub(crate) Dove + //~^ ERROR unnecessary visibility qualifier } diff --git a/src/test/ui/issues/issue-28433.stderr b/src/test/ui/issues/issue-28433.stderr index 51be15a3e316a..cfdbf6c728726 100644 --- a/src/test/ui/issues/issue-28433.stderr +++ b/src/test/ui/issues/issue-28433.stderr @@ -4,5 +4,11 @@ error: unnecessary visibility qualifier LL | pub Duck, | ^^^ `pub` not permitted here -error: aborting due to previous error +error: unnecessary visibility qualifier + --> $DIR/issue-28433.rs:7:5 + | +LL | pub(crate) Dove + | ^^^^^^^^^^ `pub` not permitted here + +error: aborting due to 2 previous errors From 0a09e76c967cfe0eb43ccdf1bddc715cd3b81f41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 10 Mar 2019 15:04:43 -0700 Subject: [PATCH 4/4] Simplify check --- src/libsyntax/parse/parser.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 0ec8cb51fa75c..1ea9ab87c3f34 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -7713,11 +7713,7 @@ impl<'a> Parser<'a> { variants.push(respan(vlo.to(self.prev_span), vr)); if !self.eat(&token::Comma) { - if self.token.is_ident() && - !self.token.is_special_ident() && - !self.token.is_used_keyword() && - !self.token.is_unused_keyword() - { + if self.token.is_ident() && !self.token.is_reserved_ident() { let sp = self.sess.source_map().next_point(self.prev_span); let mut err = self.struct_span_err(sp, "missing comma"); err.span_suggestion_short(