in which parentheses are suggested for should-have-been-tuple-patterns

Programmers used to working in some other languages (such as Python or
Go) might expect to be able to destructure values with comma-separated
identifiers but no parentheses on the left side of an assignment.

Previously, the first name in such code would get parsed as a
single-indentifier pattern—recognizing, for example, the
`let a` in `let a, b = (1, 2);`—whereupon we would have a fatal syntax
error on seeing an unexpected comma rather than the expected semicolon
(all the way nearer to the end of `parse_full_stmt`).

Instead, let's look for that comma when parsing the pattern, and if we
see it, momentarily make-believe that we're parsing the remaining
elements in a tuple pattern, so that we can suggest wrapping it all in
parentheses. We need to do this in a separate wrapper method called on
the top-level pattern (or `|`-patterns) in a `let` statement, `for`
loop, `if`- or `while let` expression, or match arm rather than within
`parse_pat` itself, because `parse_pat` gets called recursively to parse
the sub-patterns within a tuple pattern.

Resolves #48492.

1 files changed, 39 insertions, 5 deletions

diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index da2a22df997..847733e1e37 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -3318,7 +3318,7 @@ impl<'a> Parser<'a> {
                           mut attrs: ThinVec<Attribute>) -> PResult<'a, P<Expr>> {
         // Parse: `for <src_pat> in <src_expr> <src_loop_block>`
 
-        let pat = self.parse_pat()?;
+        let pat = self.parse_top_level_pat()?;
         if !self.eat_keyword(keywords::In) {
             let in_span = self.prev_span.between(self.span);
             let mut err = self.sess.span_diagnostic
@@ -3528,7 +3528,7 @@ impl<'a> Parser<'a> {
     fn parse_pats(&mut self) -> PResult<'a, Vec<P<Pat>>> {
         let mut pats = Vec::new();
         loop {
-            pats.push(self.parse_pat()?);
+            pats.push(self.parse_top_level_pat()?);
 
             if self.token == token::OrOr {
                 let mut err = self.struct_span_err(self.span,
@@ -3554,7 +3554,12 @@ impl<'a> Parser<'a> {
     // Trailing commas are significant because (p) and (p,) are different patterns.
     fn parse_parenthesized_pat_list(&mut self) -> PResult<'a, (Vec<P<Pat>>, Option<usize>, bool)> {
         self.expect(&token::OpenDelim(token::Paren))?;
+        let result = self.parse_pat_list()?;
+        self.expect(&token::CloseDelim(token::Paren))?;
+        Ok(result)
+    }
 
+    fn parse_pat_list(&mut self) -> PResult<'a, (Vec<P<Pat>>, Option<usize>, bool)> {
         let mut fields = Vec::new();
         let mut ddpos = None;
         let mut trailing_comma = false;
@@ -3584,8 +3589,6 @@ impl<'a> Parser<'a> {
             self.span_err(self.prev_span, "trailing comma is not permitted after `..`");
         }
 
-        self.expect(&token::CloseDelim(token::Paren))?;
-
         Ok((fields, ddpos, trailing_comma))
     }
 
@@ -3767,6 +3770,37 @@ impl<'a> Parser<'a> {
         }))
     }
 
+    /// A wrapper around `parse_pat` with some special error handling for the
+    /// "top-level" patterns in a match arm, `for` loop, `let`, &c. (in contast
+    /// to subpatterns within such).
+    pub fn parse_top_level_pat(&mut self) -> PResult<'a, P<Pat>> {
+        let pat = self.parse_pat()?;
+        if self.token == token::Comma {
+            // An unexpected comma after a top-level pattern is a clue that the
+            // user (perhaps more accustomed to some other language) forgot the
+            // parentheses in what should have been a tuple pattern; return a
+            // suggestion-enhanced error here rather than choking on the comma
+            // later.
+            let comma_span = self.span;
+            self.bump();
+            if let Err(mut err) = self.parse_pat_list() {
+                // We didn't expect this to work anyway; we just wanted
+                // to advance to the end of the comma-sequence so we know
+                // the span to suggest parenthesizing
+                err.cancel();
+            }
+            let seq_span = pat.span.to(self.prev_span);
+            let mut err = self.struct_span_err(comma_span,
+                                               "unexpected `,` in pattern");
+            if let Ok(seq_snippet) = self.sess.codemap().span_to_snippet(seq_span) {
+                err.span_suggestion(seq_span, "try adding parentheses",
+                                    format!("({})", seq_snippet));
+            }
+            return Err(err);
+        }
+        Ok(pat)
+    }
+
     /// Parse a pattern.
     pub fn parse_pat(&mut self) -> PResult<'a, P<Pat>> {
         maybe_whole!(self, NtPat, |x| x);
@@ -3969,7 +4003,7 @@ impl<'a> Parser<'a> {
     /// Parse a local variable declaration
     fn parse_local(&mut self, attrs: ThinVec<Attribute>) -> PResult<'a, P<Local>> {
         let lo = self.prev_span;
-        let pat = self.parse_pat()?;
+        let pat = self.parse_top_level_pat()?;
 
         let (err, ty) = if self.eat(&token::Colon) {
             // Save the state of the parser before parsing type normally, in case there is a `:`