Better error recovery in comma-separated lists

Check for trailing commas in parser instead of scanner

Instead of passing down a flag, add a bit to regions to say if we are in a comma-separated region.

delimited instead of expectedToken

Author: adampauls

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -249,8 +249,13 @@ object Parsers {
 
     /** Skip on error to next safe point.
      */
-    protected def skip(stopAtComma: Boolean): Unit =
+    protected def skip(): Unit =
       val lastRegion = in.currentRegion
+      val stopAtComma = lastRegion match {
+        case InParens(_, _, commaSeparated) => commaSeparated
+        case InBraces(_, commaSeparated) => commaSeparated
+        case _ => true
+      }
       def atStop =
         in.token == EOF
         || ((stopAtComma && in.token == COMMA) || skipStopTokens.contains(in.token)) && (in.currentRegion eq lastRegion)
@@ -278,13 +283,13 @@ object Parsers {
       if (in.token == EOF) incompleteInputError(msg)
       else
         syntaxError(msg, offset)
-        skip(stopAtComma = true)
+        skip()
 
     def syntaxErrorOrIncomplete(msg: Message, span: Span): Unit =
       if (in.token == EOF) incompleteInputError(msg)
       else
         syntaxError(msg, span)
-        skip(stopAtComma = true)
+        skip()
 
     /** Consume one token of the specified type, or
       * signal an error if it is not there.
@@ -352,7 +357,7 @@ object Parsers {
             false // it's a statement that might be legal in an outer context
           else
             in.nextToken() // needed to ensure progress; otherwise we might cycle forever
-            skip(stopAtComma=false)
+            skip()
             true
 
       in.observeOutdented()
@@ -559,19 +564,55 @@ object Parsers {
     def inDefScopeBraces[T](body: => T, rewriteWithColon: Boolean = false): T =
       inBracesOrIndented(body, rewriteWithColon)
 
-    /** part { `separator` part }
-     */
-    def tokenSeparated[T](separator: Int, part: () => T): List[T] = {
-      val ts = new ListBuffer[T] += part()
-      while (in.token == separator) {
+    /** part { `,` part }
+      * @param delimited If true, this comma-separated list must surrounded by brackets, parens, or braces.
+      */
+    def commaSeparated[T](part: () => T, delimited: Boolean = true, readFirst: Boolean = true): List[T] = {
+      val expectedEnd = if (delimited) {
+        in.currentRegion match {
+          case InParens(t, outer, _) =>
+            in.currentRegion = InParens(t, outer, commaSeparated = true)
+           t match {
+             case LPAREN => RPAREN
+             case LBRACKET => RBRACKET
+             case _ => EMPTY
+           }
+          case InBraces(outer, _) =>
+            in.currentRegion = InBraces(outer, commaSeparated = true)
+            RBRACE
+          case _ => EMPTY
+        }
+      } else EMPTY
+      val ts = new ListBuffer[T]
+      if (readFirst) ts += part()
+      var done = false
+      while (in.token == COMMA && !done) {
         in.nextToken()
-        ts += part()
+        if (in.isAfterLineEnd && (in.token == OUTDENT || (expectedEnd != EMPTY && in.token == expectedEnd))) {
+          // skip the trailing comma
+          done = true
+        } else {
+          ts += part()
+        }
+      }
+      if (expectedEnd != EMPTY && in.token != expectedEnd) {
+        // As a side effect, will skip to the nearest safe point, which might be a comma
+        syntaxErrorOrIncomplete(ExpectedTokenButFound(expectedEnd, in.token))
+        if (in.token == COMMA) {
+          ts ++= commaSeparated(part, delimited)
+        }
+      }
+      if (delimited) {
+        in.currentRegion match {
+          case InParens(t, outer, true) =>
+            in.currentRegion = InParens(t, outer, commaSeparated = false)
+          case InBraces(outer, true) => in.currentRegion = InBraces(outer, commaSeparated = false)
+          case _ =>
+        }
       }
       ts.toList
     }
 
-    def commaSeparated[T](part: () => T): List[T] = tokenSeparated(COMMA, part)
-
     def inSepRegion[T](f: Region => Region)(op: => T): T =
       val cur = in.currentRegion
       in.currentRegion = f(cur)
@@ -1387,14 +1428,7 @@ object Parsers {
           else
             Function(params, t)
         }
-      def funTypeArgsRest(first: Tree, following: () => Tree) = {
-        val buf = new ListBuffer[Tree] += first
-        while (in.token == COMMA) {
-          in.nextToken()
-          buf += following()
-        }
-        buf.toList
-      }
+
       var isValParamList = false
 
       val t =
@@ -1410,11 +1444,10 @@ object Parsers {
             val ts = funArgType() match {
               case Ident(name) if name != tpnme.WILDCARD && in.isColon() =>
                 isValParamList = true
-                funTypeArgsRest(
-                    typedFunParam(paramStart, name.toTermName, imods),
-                    () => typedFunParam(in.offset, ident(), imods))
+                typedFunParam(paramStart, name.toTermName, imods) :: commaSeparated(
+                    () => typedFunParam(in.offset, ident(), imods), readFirst = false)
               case t =>
-                funTypeArgsRest(t, funArgType)
+                t :: commaSeparated(funArgType,readFirst = false)
             }
             accept(RPAREN)
             if isValParamList || in.isArrow then
@@ -2539,7 +2572,7 @@ object Parsers {
               if (leading == LBRACE || in.token == CASE)
                 enumerators()
               else {
-                val pats = patternsOpt()
+                val pats = patternsOpt(delimited=false)
                 val pat =
                   if (in.token == RPAREN || pats.length > 1) {
                     wrappedEnums = false
@@ -2731,7 +2764,7 @@ object Parsers {
       case USCORE =>
         wildcardIdent()
       case LPAREN =>
-        atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt())) }
+        atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt(delimited=true))) }
       case QUOTE =>
         simpleExpr(Location.InPattern)
       case XMLSTART =>
@@ -2767,11 +2800,11 @@ object Parsers {
 
     /** Patterns          ::=  Pattern [`,' Pattern]
      */
-    def patterns(location: Location = Location.InPattern): List[Tree] =
-      commaSeparated(() => pattern(location))
+    def patterns(delimited: Boolean=false, location: Location = Location.InPattern): List[Tree] =
+      commaSeparated(() => pattern(location), delimited)
 
-    def patternsOpt(location: Location = Location.InPattern): List[Tree] =
-      if (in.token == RPAREN) Nil else patterns(location)
+    def patternsOpt(delimited: Boolean, location: Location = Location.InPattern): List[Tree] =
+      if (in.token == RPAREN) Nil else patterns(delimited, location)
 
     /** ArgumentPatterns  ::=  ‘(’ [Patterns] ‘)’
      *                      |  ‘(’ [Patterns ‘,’] PatVar ‘*’ ‘)’
@@ -3120,7 +3153,7 @@ object Parsers {
      */
     def importClause(leading: Token, mkTree: ImportConstr): List[Tree] = {
       val offset = accept(leading)
-      commaSeparated(importExpr(mkTree)) match {
+      commaSeparated(importExpr(mkTree), delimited=false) match {
         case t :: rest =>
           // The first import should start at the start offset of the keyword.
           val firstPos =
@@ -3197,9 +3230,9 @@ object Parsers {
           }
         else ImportSelector(from)
 
-      def importSelectors(idOK: Boolean): List[ImportSelector] =
+      def importSelector(idOK: Boolean)(): ImportSelector =
         val isWildcard = in.token == USCORE || in.token == GIVEN || isIdent(nme.raw.STAR)
-        val selector = atSpan(in.offset) {
+        atSpan(in.offset) {
           in.token match
             case USCORE => wildcardSelector()
             case GIVEN => givenSelector()
@@ -3209,13 +3242,6 @@ object Parsers {
                 if !idOK then syntaxError(i"named imports cannot follow wildcard imports")
                 namedSelector(termIdent())
         }
-        val rest =
-          if in.token == COMMA then
-            in.nextToken()
-            importSelectors(idOK = idOK && !isWildcard)
-          else
-            Nil
-        selector :: rest
 
       def importSelection(qual: Tree): Tree =
         if in.isIdent(nme.as) && qual.isInstanceOf[RefTree] then
@@ -3233,7 +3259,7 @@ object Parsers {
             case GIVEN =>
               mkTree(qual, givenSelector() :: Nil)
             case LBRACE =>
-              mkTree(qual, inBraces(importSelectors(idOK = true)))
+              mkTree(qual, inBraces(commaSeparated(importSelector(idOK = true))))
             case _ =>
               if isIdent(nme.raw.STAR) then
                 mkTree(qual, wildcardSelector() :: Nil)
@@ -3290,7 +3316,7 @@ object Parsers {
       var lhs = first match {
         case id: Ident if in.token == COMMA =>
           in.nextToken()
-          id :: commaSeparated(() => termIdent())
+          id :: commaSeparated(() => termIdent(), delimited=false)
         case _ =>
           first :: Nil
       }
@@ -3561,7 +3587,7 @@ object Parsers {
         val id = termIdent()
         if (in.token == COMMA) {
           in.nextToken()
-          val ids = commaSeparated(() => termIdent())
+          val ids = commaSeparated(() => termIdent(), delimited=false)
           PatDef(mods1, id :: ids, TypeTree(), EmptyTree)
         }
         else {
@@ -3765,7 +3791,7 @@ object Parsers {
       val derived =
         if (isIdent(nme.derives)) {
           in.nextToken()
-          tokenSeparated(COMMA, () => convertToTypeId(qualId()))
+          commaSeparated(() => convertToTypeId(qualId()), delimited=false)
         }
         else Nil
       possibleTemplateStart()

compiler/src/dotty/tools/dotc/parsing/Scanners.scala

@@ -281,7 +281,7 @@ object Scanners {
     /** Are we in a `${ }` block? such that RBRACE exits back into multiline string. */
     private def inMultiLineInterpolatedExpression =
       currentRegion match {
-        case InBraces(InString(true, _)) => true
+        case InBraces(InString(true, _), _) => true
         case _ => false
       }
 
@@ -315,7 +315,7 @@ object Scanners {
         dropBraces()
       case RPAREN | RBRACKET =>
         currentRegion match {
-          case InParens(prefix, outer) if prefix + 1 == lastToken => currentRegion = outer
+          case InParens(prefix, outer, _) if prefix + 1 == lastToken => currentRegion = outer
           case _ =>
         }
       case STRINGLIT =>
@@ -655,13 +655,6 @@ object Scanners {
               insert(OUTDENT, offset)
               currentRegion = r.outer
             case _ =>
-              lookAhead()
-              if isAfterLineEnd
-                 && (token == RPAREN || token == RBRACKET || token == RBRACE || token == OUTDENT)
-              then
-                () /* skip the trailing comma */
-              else
-                reset()
         case END =>
           if !isEndMarker then token = IDENTIFIER
         case COLON =>
@@ -1451,7 +1444,7 @@ object Scanners {
     def proposeKnownWidth(width: IndentWidth, lastToken: Token) =
       if knownWidth == null then
         this match
-          case InParens(_, _) if lastToken != LPAREN =>
+          case InParens(_, _, _) if lastToken != LPAREN =>
             useOuterWidth()
           case _ =>
             knownWidth = width
@@ -1462,8 +1455,8 @@ object Scanners {
 
     private def delimiter = this match
       case _: InString => "}(in string)"
-      case InParens(LPAREN, _) => ")"
-      case InParens(LBRACKET, _) => "]"
+      case InParens(LPAREN, _, _) => ")"
+      case InParens(LBRACKET, _, _) => "]"
       case _: InBraces => "}"
       case _: InCase => "=>"
       case _: Indented => "UNDENT"
@@ -1478,8 +1471,8 @@ object Scanners {
   end Region
 
   case class InString(multiLine: Boolean, outer: Region) extends Region
-  case class InParens(prefix: Token, outer: Region) extends Region
-  case class InBraces(outer: Region) extends Region
+  case class InParens(prefix: Token, outer: Region, commaSeparated: Boolean = false) extends Region
+  case class InBraces(outer: Region, commaSeparated: Boolean = false) extends Region
   case class InCase(outer: Region) extends Region
 
   /** A class describing an indentation region.

tests/neg/comma-separated-errors.check

@@ -0,0 +1,36 @@
+-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:21 ----------------------------------------------------
+3 |  def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
+  |                     ^
+  |                     ')' expected, but integer literal found
+-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:26 ----------------------------------------------------
+3 |  def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
+  |                          ^^^
+  |                          ':' expected, but identifier found
+-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:42 ----------------------------------------------------
+3 |  def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
+  |                                          ^
+  |                                          ')' expected, but integer literal found
+-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:47 ----------------------------------------------------
+3 |  def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
+  |                                               ^
+  |                                               ':' expected, but '=' found
+-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:11:16 ---------------------------------------------------
+11 |    case Plus(4 1) => // error
+   |                ^
+   |                ')' expected, but integer literal found
+-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:12:16 ---------------------------------------------------
+12 |    case Plus(4 5 6 7, 1, 2 3) => // error // error
+   |                ^
+   |                ')' expected, but integer literal found
+-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:12:28 ---------------------------------------------------
+12 |    case Plus(4 5 6 7, 1, 2 3) => // error // error
+   |                            ^
+   |                            ')' expected, but integer literal found
+-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:14:12 ---------------------------------------------------
+14 |  val x: A[T=Int, T=Int] = ??? // error // error
+   |            ^
+   |            ']' expected, but '=' found
+-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:14:19 ---------------------------------------------------
+14 |  val x: A[T=Int, T=Int] = ??? // error // error
+   |                   ^
+   |                   ']' expected, but '=' found

tests/neg/comma-separated-errors.scala

@@ -0,0 +1,15 @@
+class A[T]
+object o {
+  def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
+
+  case class Plus(a: Int, b: Int)
+
+  object Plus {
+    def unapply(r: Int): Plus = Plus(r - 1, 1)
+  }
+  5 match {
+    case Plus(4 1) => // error
+    case Plus(4 5 6 7, 1, 2 3) => // error // error
+  }
+  val x: A[T=Int, T=Int] = ??? // error // error
+}

tests/neg/i1679.scala

@@ -1,5 +1,5 @@
 class A[T]
 object o {
   // Testing compiler crash, this test should be modified when named type argument are completely implemented
-  val x: A[T=Int, T=Int] = ??? // error: ']' expected, but '=' found // error
+  val x: A[T=Int, T=Int] = ??? // error: ']' expected, but '=' found // error: ']' expected, but '=' found
 }

tests/neg/t11900.check

@@ -0,0 +1,18 @@
+-- Error: tests/neg/t11900.scala:44:16 ---------------------------------------------------------------------------------
+44 |      a => a + 1,   // error: weird comma
+   |                ^
+   |                end of statement expected but ',' found
+-- Error: tests/neg/t11900.scala:48:16 ---------------------------------------------------------------------------------
+48 |    println("a"),   // error: weird comma
+   |                ^
+   |                end of statement expected but ',' found
+-- Error: tests/neg/t11900.scala:52:16 ---------------------------------------------------------------------------------
+52 |    println("b"),   // error: weird comma
+   |                ^
+   |                end of statement expected but ',' found
+-- [E032] Syntax Error: tests/neg/t11900.scala:64:8 --------------------------------------------------------------------
+64 |      _*, // error
+   |        ^
+   |        pattern expected
+   |
+   | longer explanation available when compiling with `-explain`