Check for trailing commas in parser instead of scanner

Author: adampauls

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

@@ -568,11 +568,18 @@ object Parsers {
       *                    If the parser consumes a `part` that is not followed by a comma or this expected
       *                    token, issue a syntax error and try to recover at the next safe point.
       */
-    def commaSeparated[T](part: () => T, expectedEnd: Token = EMPTY): List[T] = {
-      val ts = new ListBuffer[T] += part()
-      while (in.token == COMMA) {
+    def commaSeparated[T](part: () => T, expectedEnd: Token, readFirst: Boolean = true): List[T] = {
+      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
@@ -1400,14 +1407,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 =
@@ -1423,11 +1423,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), RPAREN, readFirst = false)
               case t =>
-                funTypeArgsRest(t, funArgType)
+                t :: commaSeparated(funArgType, RPAREN, readFirst = false)
             }
             accept(RPAREN)
             if isValParamList || in.isArrow then
@@ -3130,7 +3129,7 @@ object Parsers {
      */
     def importClause(leading: Token, mkTree: ImportConstr): List[Tree] = {
       val offset = accept(leading)
-      commaSeparated(importExpr(mkTree)) match {
+      commaSeparated(importExpr(mkTree), EMPTY) match {
         case t :: rest =>
           // The first import should start at the start offset of the keyword.
           val firstPos =
@@ -3207,9 +3206,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()
@@ -3219,13 +3218,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
@@ -3243,7 +3235,7 @@ object Parsers {
             case GIVEN =>
               mkTree(qual, givenSelector() :: Nil)
             case LBRACE =>
-              mkTree(qual, inBraces(importSelectors(idOK = true)))
+              mkTree(qual, inBraces(commaSeparated(importSelector(idOK = true), RBRACE)))
             case _ =>
               if isIdent(nme.raw.STAR) then
                 mkTree(qual, wildcardSelector() :: Nil)
@@ -3300,7 +3292,7 @@ object Parsers {
       var lhs = first match {
         case id: Ident if in.token == COMMA =>
           in.nextToken()
-          id :: commaSeparated(() => termIdent())
+          id :: commaSeparated(() => termIdent(), EMPTY)
         case _ =>
           first :: Nil
       }
@@ -3571,7 +3563,7 @@ object Parsers {
         val id = termIdent()
         if (in.token == COMMA) {
           in.nextToken()
-          val ids = commaSeparated(() => termIdent())
+          val ids = commaSeparated(() => termIdent(), EMPTY)
           PatDef(mods1, id :: ids, TypeTree(), EmptyTree)
         }
         else {
@@ -3775,7 +3767,7 @@ object Parsers {
       val derived =
         if (isIdent(nme.derives)) {
           in.nextToken()
-          commaSeparated(() => convertToTypeId(qualId()))
+          commaSeparated(() => convertToTypeId(qualId()), EMPTY)
         }
         else Nil
       possibleTemplateStart()

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

@@ -660,13 +660,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 =>

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`

tests/neg/t11900.scala

@@ -0,0 +1,79 @@
+
+trait t11900 {
+  // cf pos/trailing-commas
+  //
+  import scala.collection.{
+    immutable,
+    mutable,
+  }
+
+  def h[A,
+  ]: List[A] = Nil
+
+  def u(
+      x: Int,
+      y: Int,
+  )(using List[Int],
+      Set[Int],
+  )(using l: List[Int],
+    s : Set[Int],
+  ): Int = 1
+
+  def g = List(
+    1,
+    2,
+    3,
+  )
+
+  def star =
+    List(1, 2, 3, 4, 5) match {
+      case List(
+      1,
+      2,
+      3,
+      ) => false
+      case List(
+      1,
+      2,
+      _*,
+      ) => true
+    }
+
+  def f =
+    List(1, 2, 3).map {
+      a => a + 1,   // error: weird comma
+    }
+
+  class A() {
+    println("a"),   // error: weird comma
+  }
+
+  def b() = {
+    println("b"),   // error: weird comma
+  }
+
+  def starcrossed =
+    List(1, 2, 3, 4, 5) match {
+      case List(
+      1,
+      2,
+      3,
+      ) => false
+      case List(
+      1,
+      _*, // error
+      2,
+      ) => true
+    }
+
+  def p(p: (Int,
+      String,
+      )
+  ): Unit
+
+  def q: (Int,
+      String,
+      )
+
+  val z = 42
+}

tests/neg/trailingCommas.scala

@@ -56,3 +56,27 @@ object `package` {
   case class Foo(foo: Any)
   case class Bar(foo: Any)
 }
+
+// Unparenthesized lists
+trait Deriv1[T]
+object Deriv1 {
+  def derived[T]: Deriv1[T] = new Deriv1[T] {}
+}
+
+trait Deriv2[T]
+object Deriv2 {
+  def derived[T]: Deriv2[T] = new Deriv2[T] {}
+}
+
+class Derives1 derives Deriv1, Deriv2,
+object End // error: an identifier expected, but 'object' found
+
+class Derives2 derives Deriv1,
+     Deriv2,
+object End2 // error: an identifier expected, but 'object' found
+
+val a,
+    b,
+    c,
+    = (1, 2, 3) // error
+val x, y, z, = (1, 2, 3) // error // error

tests/pos/comma-separated.scala

@@ -0,0 +1,19 @@
+trait Bar[T]
+object Bar {
+  def derived[T]: Bar[T] = new Bar[T] {}
+}
+
+trait Baz[T]
+object Baz {
+  def derived[T]: Baz[T] = new Baz[T] {}
+}
+
+class Foo derives Bar, Baz
+
+class Foo2 derives Bar,
+   Baz
+
+val x, y, z = (1, 2, 3)
+val a,
+    b,
+    c = (1, 2, 3)