Use heuristics to recover parsing of missing ;

estebank · estebank · commit d673d0ac8462 · 2019-10-28T10:53:13.000-07:00
- Detect `,` and `:` typos where `;` was intended.
- When the next token could have been the start of a new statement,
  detect a missing semicolon.
diff --git a/src/libsyntax/parse/parser/diagnostics.rs b/src/libsyntax/parse/parser/diagnostics.rs
@@ -6,7 +6,7 @@ use crate::ast::{
     self, Param, BinOpKind, BindingMode, BlockCheckMode, Expr, ExprKind, Ident, Item, ItemKind,
     Mutability, Pat, PatKind, PathSegment, QSelf, Ty, TyKind,
 };
-use crate::parse::token::{self, TokenKind};
+use crate::parse::token::{self, TokenKind, token_can_begin_expr};
 use crate::print::pprust;
 use crate::ptr::P;
 use crate::symbol::{kw, sym};
@@ -326,34 +326,8 @@ impl<'a> Parser<'a> {
             }
         }
 
-        let is_semi_suggestable = expected.iter().any(|t| match t {
-            TokenType::Token(token::Semi) => true, // We expect a `;` here.
-            _ => false,
-        }) && ( // A `;` would be expected before the current keyword.
-            self.token.is_keyword(kw::Break) ||
-            self.token.is_keyword(kw::Continue) ||
-            self.token.is_keyword(kw::For) ||
-            self.token.is_keyword(kw::If) ||
-            self.token.is_keyword(kw::Let) ||
-            self.token.is_keyword(kw::Loop) ||
-            self.token.is_keyword(kw::Match) ||
-            self.token.is_keyword(kw::Return) ||
-            self.token.is_keyword(kw::While)
-        );
         let sm = self.sess.source_map();
         match (sm.lookup_line(self.token.span.lo()), sm.lookup_line(sp.lo())) {
-            (Ok(ref a), Ok(ref b)) if a.line != b.line && is_semi_suggestable => {
-                // The spans are in different lines, expected `;` and found `let` or `return`.
-                // High likelihood that it is only a missing `;`.
-                err.span_suggestion_short(
-                    label_sp,
-                    "a semicolon may be missing here",
-                    ";".to_string(),
-                    Applicability::MaybeIncorrect,
-                );
-                err.emit();
-                return Ok(true);
-            }
             (Ok(ref a), Ok(ref b)) if a.line == b.line => {
                 // When the spans are in the same line, it means that the only content between
                 // them is whitespace, point at the found token in that case:
@@ -902,18 +876,61 @@ impl<'a> Parser<'a> {
             }
         }
         let sm = self.sess.source_map();
-        match (sm.lookup_line(prev_sp.lo()), sm.lookup_line(sp.lo())) {
-            (Ok(ref a), Ok(ref b)) if a.line == b.line => {
-                // When the spans are in the same line, it means that the only content
-                // between them is whitespace, point only at the found token.
-                err.span_label(sp, label_exp);
+        if !sm.is_multiline(prev_sp.until(sp)) {
+            // When the spans are in the same line, it means that the only content
+            // between them is whitespace, point only at the found token.
+            err.span_label(sp, label_exp);
+        } else {
+            err.span_label(prev_sp, label_exp);
+            err.span_label(sp, "unexpected token");
+        }
+        Err(err)
+    }
+
+    pub(super) fn expect_semi(&mut self) -> PResult<'a, ()> {
+        if self.eat(&token::Semi) {
+            return Ok(());
+        }
+        let sm = self.sess.source_map();
+        let msg = format!("expected `;`, found `{}`", self.this_token_descr());
+        let appl = Applicability::MachineApplicable;
+        if self.look_ahead(1, |t| t == &token::CloseDelim(token::Brace)
+            || token_can_begin_expr(t) && t.kind != token::Colon
+        ) && [token::Comma, token::Colon].contains(&self.token.kind) {
+            // Likely typo: `,` → `;` or `:` → `;`. This is triggered if the current token is
+            // either `,` or `:`, and the next token could either start a new statement or is a
+            // block close. For example:
+            //
+            //   let x = 32:
+            //   let y = 42;
+            if sm.is_multiline(self.prev_span.until(self.token.span)) {
+                self.bump();
+                let sp = self.prev_span;
+                self.struct_span_err(sp, &msg)
+                    .span_suggestion(sp, "change this to `;`", ";".to_string(), appl)
+                    .emit();
+                return Ok(())
             }
-            _ => {
-                err.span_label(prev_sp, label_exp);
-                err.span_label(sp, "unexpected token");
+        } else if self.look_ahead(0, |t| t == &token::CloseDelim(token::Brace) || (
+                token_can_begin_expr(t)
+                && t != &token::Semi
+                && t != &token::Pound // Avoid triggering with too many trailing `#` in raw string.
+        )) {
+            // Missing semicolon typo. This is triggered if the next token could either start a
+            // new statement or is a block close. For example:
+            //
+            //   let x = 32
+            //   let y = 42;
+            if sm.is_multiline(self.prev_span.until(self.token.span)) {
+                let sp = self.prev_span.shrink_to_hi();
+                self.struct_span_err(sp, &msg)
+                    .span_label(self.token.span, "unexpected token")
+                    .span_suggestion_short(sp, "add `;` here", ";".to_string(), appl)
+                    .emit();
+                return Ok(())
             }
         }
-        Err(err)
+        self.expect(&token::Semi).map(|_| ()) // Error unconditionally
     }
 
     pub(super) fn parse_semi_or_incorrect_foreign_fn_body(
@@ -943,7 +960,7 @@ impl<'a> Parser<'a> {
                 Err(mut err) => {
                     err.cancel();
                     mem::replace(self, parser_snapshot);
-                    self.expect(&token::Semi)?;
+                    self.expect_semi()?;
                 }
             }
         } else {
diff --git a/src/libsyntax/parse/parser/item.rs b/src/libsyntax/parse/parser/item.rs
@@ -98,7 +98,7 @@ impl<'a> Parser<'a> {
         if self.eat_keyword(kw::Use) {
             // USE ITEM
             let item_ = ItemKind::Use(P(self.parse_use_tree()?));
-            self.expect(&token::Semi)?;
+            self.expect_semi()?;
 
             let span = lo.to(self.prev_span);
             let item = self.mk_item(span, Ident::invalid(), item_, vis, attrs);
@@ -526,7 +526,7 @@ impl<'a> Parser<'a> {
             // eat a matched-delimiter token tree:
             let (delim, tts) = self.expect_delimited_token_tree()?;
             if delim != MacDelimiter::Brace {
-                self.expect(&token::Semi)?;
+                self.expect_semi()?;
             }
 
             Ok(Some(Mac {
@@ -776,7 +776,7 @@ impl<'a> Parser<'a> {
         let typ = self.parse_ty()?;
         self.expect(&token::Eq)?;
         let expr = self.parse_expr()?;
-        self.expect(&token::Semi)?;
+        self.expect_semi()?;
         Ok((name, ImplItemKind::Const(typ, expr), Generics::default()))
     }
 
@@ -813,7 +813,7 @@ impl<'a> Parser<'a> {
 
             let bounds = self.parse_generic_bounds(None)?;
             tps.where_clause = self.parse_where_clause()?;
-            self.expect(&token::Semi)?;
+            self.expect_semi()?;
 
             let whole_span = lo.to(self.prev_span);
             if is_auto == IsAuto::Yes {
@@ -927,7 +927,7 @@ impl<'a> Parser<'a> {
         } else {
             None
         };
-        self.expect(&token::Semi)?;
+        self.expect_semi()?;
         Ok((ident, TraitItemKind::Const(ty, default), Generics::default()))
     }
 
@@ -951,7 +951,7 @@ impl<'a> Parser<'a> {
         } else {
             None
         };
-        self.expect(&token::Semi)?;
+        self.expect_semi()?;
 
         Ok((ident, TraitItemKind::Type(bounds, default), generics))
     }
@@ -1054,7 +1054,7 @@ impl<'a> Parser<'a> {
         } else {
             (orig_name, None)
         };
-        self.expect(&token::Semi)?;
+        self.expect_semi()?;
 
         let span = lo.to(self.prev_span);
         Ok(self.mk_item(span, item_name, ItemKind::ExternCrate(orig_name), visibility, attrs))
@@ -1217,7 +1217,7 @@ impl<'a> Parser<'a> {
         self.expect(&token::Colon)?;
         let ty = self.parse_ty()?;
         let hi = self.token.span;
-        self.expect(&token::Semi)?;
+        self.expect_semi()?;
         Ok(ForeignItem {
             ident,
             attrs,
@@ -1235,7 +1235,7 @@ impl<'a> Parser<'a> {
 
         let ident = self.parse_ident()?;
         let hi = self.token.span;
-        self.expect(&token::Semi)?;
+        self.expect_semi()?;
         Ok(ast::ForeignItem {
             ident,
             attrs,
@@ -1282,7 +1282,7 @@ impl<'a> Parser<'a> {
 
         self.expect(&token::Eq)?;
         let e = self.parse_expr()?;
-        self.expect(&token::Semi)?;
+        self.expect_semi()?;
         let item = match m {
             Some(m) => ItemKind::Static(ty, m, e),
             None => ItemKind::Const(ty, e),
@@ -1344,7 +1344,7 @@ impl<'a> Parser<'a> {
             let ty = self.parse_ty()?;
             AliasKind::Weak(ty)
         };
-        self.expect(&token::Semi)?;
+        self.expect_semi()?;
         Ok((ident, alias, tps))
     }
 
@@ -1468,7 +1468,7 @@ impl<'a> Parser<'a> {
         } else if self.token == token::OpenDelim(token::Paren) {
             let body = VariantData::Tuple(self.parse_tuple_struct_body()?, DUMMY_NODE_ID);
             generics.where_clause = self.parse_where_clause()?;
-            self.expect(&token::Semi)?;
+            self.expect_semi()?;
             body
         } else {
             let token_str = self.this_token_descr();
diff --git a/src/libsyntax/parse/parser/stmt.rs b/src/libsyntax/parse/parser/stmt.rs
@@ -432,6 +432,7 @@ impl<'a> Parser<'a> {
             None => return Ok(None),
         };
 
+        let mut eat_semi = true;
         match stmt.kind {
             StmtKind::Expr(ref expr) if self.token != token::Eof => {
                 // expression without semicolon
@@ -453,13 +454,14 @@ impl<'a> Parser<'a> {
                 if macro_legacy_warnings && self.token != token::Semi {
                     self.warn_missing_semicolon();
                 } else {
-                    self.expect_one_of(&[], &[token::Semi])?;
+                    self.expect_semi()?;
+                    eat_semi = false;
                 }
             }
             _ => {}
         }
 
-        if self.eat(&token::Semi) {
+        if eat_semi && self.eat(&token::Semi) {
             stmt = stmt.add_trailing_semicolon();
         }
         stmt.span = stmt.span.to(self.prev_span);
diff --git a/src/libsyntax/parse/token.rs b/src/libsyntax/parse/token.rs
@@ -143,34 +143,35 @@ impl Lit {
 
 pub(crate) fn ident_can_begin_expr(name: ast::Name, span: Span, is_raw: bool) -> bool {
     let ident_token = Token::new(Ident(name, is_raw), span);
+    token_can_begin_expr(&ident_token)
+}
 
+pub(crate) fn token_can_begin_expr(ident_token: &Token) -> bool {
     !ident_token.is_reserved_ident() ||
     ident_token.is_path_segment_keyword() ||
-    [
-        kw::Async,
-
-        // FIXME: remove when `await!(..)` syntax is removed
-        // https://github.com/rust-lang/rust/issues/60610
-        kw::Await,
-
-        kw::Do,
-        kw::Box,
-        kw::Break,
-        kw::Continue,
-        kw::False,
-        kw::For,
-        kw::If,
-        kw::Let,
-        kw::Loop,
-        kw::Match,
-        kw::Move,
-        kw::Return,
-        kw::True,
-        kw::Unsafe,
-        kw::While,
-        kw::Yield,
-        kw::Static,
-    ].contains(&name)
+    match ident_token.kind {
+        TokenKind::Ident(ident, _) => [
+            kw::Async,
+            kw::Do,
+            kw::Box,
+            kw::Break,
+            kw::Continue,
+            kw::False,
+            kw::For,
+            kw::If,
+            kw::Let,
+            kw::Loop,
+            kw::Match,
+            kw::Move,
+            kw::Return,
+            kw::True,
+            kw::Unsafe,
+            kw::While,
+            kw::Yield,
+            kw::Static,
+        ].contains(&ident),
+        _=> false,
+    }
 }
 
 fn ident_can_begin_type(name: ast::Name, span: Span, is_raw: bool) -> bool {
diff --git a/src/test/ui/parser/import-from-path.stderr b/src/test/ui/parser/import-from-path.stderr
@@ -2,7 +2,7 @@ error: expected `;`, found `::`
   --> $DIR/import-from-path.rs:2:15
    |
 LL | use foo::{bar}::baz
-   |               ^^ expected `;`
+   |               ^^ expected `;` here
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/parser/import-from-rename.stderr b/src/test/ui/parser/import-from-rename.stderr
@@ -1,8 +1,8 @@
-error: expected `;`, found keyword `as`
+error: expected `;`, found `as`
   --> $DIR/import-from-rename.rs:3:16
    |
 LL | use foo::{bar} as baz;
-   |                ^^ expected `;`
+   |                ^^ expected `;` here
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/parser/import-glob-path.stderr b/src/test/ui/parser/import-glob-path.stderr
@@ -2,7 +2,7 @@ error: expected `;`, found `::`
   --> $DIR/import-glob-path.rs:2:11
    |
 LL | use foo::*::bar
-   |           ^^ expected `;`
+   |           ^^ expected `;` here
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/parser/import-glob-rename.stderr b/src/test/ui/parser/import-glob-rename.stderr
@@ -1,8 +1,8 @@
-error: expected `;`, found keyword `as`
+error: expected `;`, found `as`
   --> $DIR/import-glob-rename.rs:3:12
    |
 LL | use foo::* as baz;
-   |            ^^ expected `;`
+   |            ^^ expected `;` here
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/parser/issue-3036.rs b/src/test/ui/parser/issue-3036.rs
@@ -2,5 +2,5 @@
 
 fn main()
 {
-    let x = 3
-} //~ ERROR: expected one of `.`, `;`, `?`, or an operator, found `}`
+    let x = 3 //~ ERROR: expected `;`
+}
diff --git a/src/test/ui/parser/issue-3036.stderr b/src/test/ui/parser/issue-3036.stderr
@@ -1,10 +1,10 @@
-error: expected one of `.`, `;`, `?`, or an operator, found `}`
-  --> $DIR/issue-3036.rs:6:1
+error: expected `;`, found ``}``
+  --> $DIR/issue-3036.rs:5:14
    |
 LL |     let x = 3
-   |              - expected one of `.`, `;`, `?`, or an operator here
+   |              ^ help: add `;` here
 LL | }
-   | ^ unexpected token
+   | - unexpected token
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/parser/recover-missing-semi.rs b/src/test/ui/parser/recover-missing-semi.rs
@@ -1,13 +1,13 @@
 fn main() {
     let _: usize = ()
     //~^ ERROR mismatched types
+    //~| ERROR expected `;`
     let _ = 3;
-    //~^ ERROR expected one of
 }
 
 fn foo() -> usize {
     let _: usize = ()
     //~^ ERROR mismatched types
+    //~| ERROR expected `;`
     return 3;
-    //~^ ERROR expected one of
 }
diff --git a/src/test/ui/parser/recover-missing-semi.stderr b/src/test/ui/parser/recover-missing-semi.stderr

Original file line number	Diff line number	Diff line change
`@@ -432,6 +432,7 @@ impl<'a> Parser<'a> {`
`432`	`432`	`None => return Ok(None),`
`433`	`433`	`};`
`434`	`434`
	`435`	`+ let mut eat_semi = true;`
`435`	`436`	`match stmt.kind {`
`436`	`437`	`StmtKind::Expr(ref expr) if self.token != token::Eof => {`
`437`	`438`	`// expression without semicolon`
`@@ -453,13 +454,14 @@ impl<'a> Parser<'a> {`
`453`	`454`	`if macro_legacy_warnings && self.token != token::Semi {`
`454`	`455`	`self.warn_missing_semicolon();`
`455`	`456`	`} else {`
`456`		`- self.expect_one_of(&[], &[token::Semi])?;`
	`457`	`+ self.expect_semi()?;`
	`458`	`+ eat_semi = false;`
`457`	`459`	`}`
`458`	`460`	`}`
`459`	`461`	`_ => {}`
`460`	`462`	`}`
`461`	`463`
`462`		`- if self.eat(&token::Semi) {`
	`464`	`+ if eat_semi && self.eat(&token::Semi) {`
`463`	`465`	`stmt = stmt.add_trailing_semicolon();`
`464`	`466`	`}`
`465`	`467`	`stmt.span = stmt.span.to(self.prev_span);`
Original file line number	Diff line number	Diff line change
@@ -2,7 +2,7 @@ error: expected `;`, found `::`
`2`	`2`	`--> $DIR/import-from-path.rs:2:15`
`3`	`3`	`\|`
`4`	`4`	`LL \| use foo::{bar}::baz`
`5`		- \| ^^ expected `;`
	`5`	+ \| ^^ expected `;` here
`6`	`6`
`7`	`7`	`error: aborting due to previous error`
`8`	`8`
Original file line number	Diff line number	Diff line change
`@@ -1,8 +1,8 @@`
`1`		-error: expected `;`, found keyword `as`
	`1`	+error: expected `;`, found `as`
`2`	`2`	`--> $DIR/import-from-rename.rs:3:16`
`3`	`3`	`\|`
`4`	`4`	`LL \| use foo::{bar} as baz;`
`5`		- \| ^^ expected `;`
	`5`	+ \| ^^ expected `;` here
`6`	`6`
`7`	`7`	`error: aborting due to previous error`
`8`	`8`
Original file line number	Diff line number	Diff line change
@@ -2,7 +2,7 @@ error: expected `;`, found `::`
`2`	`2`	`--> $DIR/import-glob-path.rs:2:11`
`3`	`3`	`\|`
`4`	`4`	`LL \| use foo::*::bar`
`5`		- \| ^^ expected `;`
	`5`	+ \| ^^ expected `;` here
`6`	`6`
`7`	`7`	`error: aborting due to previous error`
`8`	`8`
Original file line number	Diff line number	Diff line change
`@@ -2,5 +2,5 @@`
`2`	`2`
`3`	`3`	`fn main()`
`4`	`4`	`{`
`5`		`- let x = 3`
`6`		-} //~ ERROR: expected one of `.`, `;`, `?`, or an operator, found `}`
	`5`	+ let x = 3 //~ ERROR: expected `;`
	`6`	`+}`