Backport "Better error messages for missing commas and more" to LTS (#…

…20746) Backports #18785 to the LTS branch. PR submitted by the release tooling. [skip ci]
scala · Jun 23, 2024 · a7608fe · a7608fe
2 parents 7efd518 + a96e975
commit a7608fe
Show file tree

Hide file tree

Showing 18 changed files with 174 additions and 87 deletions.
diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
@@ -555,10 +555,29 @@ object Parsers {
       accept(tok)
       try body finally accept(tok + 1)
 
+    /** Same as enclosed, but if closing token is missing, add `,` to the expected tokens
+     *  in the error message provided the next token could have followed a `,`.
+     */
+    def enclosedWithCommas[T](tok: Token, body: => T): T =
+      accept(tok)
+      val closing = tok + 1
+      val isEmpty = in.token == closing
+      val ts = body
+      if in.token != closing then
+        val followComma =
+          if tok == LPAREN then canStartExprTokens3 else canStartTypeTokens
+        val prefix = if !isEmpty && followComma.contains(in.token) then "',' or " else ""
+        syntaxErrorOrIncomplete(ExpectedTokenButFound(closing, in.token, prefix))
+      if in.token == closing then in.nextToken()
+      ts
+
     def inParens[T](body: => T): T = enclosed(LPAREN, body)
     def inBraces[T](body: => T): T = enclosed(LBRACE, body)
     def inBrackets[T](body: => T): T = enclosed(LBRACKET, body)
 
+    def inParensWithCommas[T](body: => T): T = enclosedWithCommas(LPAREN, body)
+    def inBracketsWithCommas[T](body: => T): T = enclosedWithCommas(LBRACKET, body)
+
     def inBracesOrIndented[T](body: => T, rewriteWithColon: Boolean = false): T =
       if in.token == INDENT then
         val rewriteToBraces = in.rewriteNoIndent
@@ -970,6 +989,17 @@ object Parsers {
         isArrowIndent()
       else false
 
+    /** Can the next lookahead token start an operand as defined by
+     *  leadingOperandTokens, or is postfix ops enabled?
+     *  This is used to decide whether the current token can be an infix operator.
+     */
+    def nextCanFollowOperator(leadingOperandTokens: BitSet): Boolean =
+      leadingOperandTokens.contains(in.lookahead.token)
+      || in.postfixOpsEnabled
+      || in.lookahead.token == COLONop
+      || in.lookahead.token == EOF     // important for REPL completions
+      || ctx.mode.is(Mode.Interactive) // in interactive mode the next tokens might be missing
+
   /* --------- OPERAND/OPERATOR STACK --------------------------------------- */
 
     var opStack: List[OpInfo] = Nil
@@ -1050,7 +1080,11 @@ object Parsers {
         then recur(top)
         else top
 
-      recur(first)
+      val res = recur(first)
+      if isIdent(nme.raw.STAR) && !followingIsVararg() then
+        syntaxError(em"spread operator `*` not allowed here; must come last in a parameter list")
+        in.nextToken()
+      res
     end infixOps
 
 /* -------- IDENTIFIERS AND LITERALS ------------------------------------------- */
@@ -1659,7 +1693,7 @@ object Parsers {
     /**  FunParamClause ::=  ‘(’ TypedFunParam {‘,’ TypedFunParam } ‘)’
      */
     def funParamClause(): List[ValDef] =
-      inParens(commaSeparated(() => typedFunParam(in.offset, ident())))
+      inParensWithCommas(commaSeparated(() => typedFunParam(in.offset, ident())))
 
     def funParamClauses(): List[List[ValDef]] =
       if in.token == LPAREN then funParamClause() :: funParamClauses() else Nil
@@ -1671,7 +1705,8 @@ object Parsers {
 
     def infixTypeRest(t: Tree): Tree =
       infixOps(t, canStartInfixTypeTokens, refinedTypeFn, Location.ElseWhere, ParseKind.Type,
-        isOperator = !followingIsVararg() && !isPureArrow)
+        isOperator = !followingIsVararg() && !isPureArrow
+                     && nextCanFollowOperator(canStartInfixTypeTokens))
 
     /** RefinedType   ::=  WithType {[nl] Refinement} [`^` CaptureSet]
      */
@@ -1807,7 +1842,7 @@ object Parsers {
       else
         def singletonArgs(t: Tree): Tree =
           if in.token == LPAREN && in.featureEnabled(Feature.dependent)
-          then singletonArgs(AppliedTypeTree(t, inParens(commaSeparated(singleton))))
+          then singletonArgs(AppliedTypeTree(t, inParensWithCommas(commaSeparated(singleton))))
           else t
         singletonArgs(simpleType1())
 
@@ -1823,7 +1858,7 @@ object Parsers {
     def simpleType1() = simpleTypeRest {
       if in.token == LPAREN then
         atSpan(in.offset) {
-          makeTupleOrParens(inParens(argTypes(namedOK = false, wildOK = true)))
+          makeTupleOrParens(inParensWithCommas(argTypes(namedOK = false, wildOK = true)))
         }
       else if in.token == LBRACE then
         atSpan(in.offset) { RefinedTypeTree(EmptyTree, refinement(indentOK = false)) }
@@ -1976,7 +2011,8 @@ object Parsers {
     /** TypeArgs      ::= `[' Type {`,' Type} `]'
      *  NamedTypeArgs ::= `[' NamedTypeArg {`,' NamedTypeArg} `]'
      */
-    def typeArgs(namedOK: Boolean, wildOK: Boolean): List[Tree] = inBrackets(argTypes(namedOK, wildOK))
+    def typeArgs(namedOK: Boolean, wildOK: Boolean): List[Tree] =
+      inBracketsWithCommas(argTypes(namedOK, wildOK))
 
     /** Refinement ::= `{' RefineStatSeq `}'
      */
@@ -2413,7 +2449,8 @@ object Parsers {
 
     def postfixExprRest(t: Tree, location: Location): Tree =
       infixOps(t, in.canStartExprTokens, prefixExpr, location, ParseKind.Expr,
-        isOperator = !(location.inArgs && followingIsVararg()))
+        isOperator = !(location.inArgs && followingIsVararg())
+                     && nextCanFollowOperator(canStartInfixExprTokens))
 
     /** PrefixExpr       ::= [PrefixOperator'] SimpleExpr
      *  PrefixOperator   ::=  ‘-’ | ‘+’ | ‘~’ | ‘!’ (if not backquoted)
@@ -2472,7 +2509,7 @@ object Parsers {
           placeholderParams = param :: placeholderParams
           atSpan(start) { Ident(pname) }
         case LPAREN =>
-          atSpan(in.offset) { makeTupleOrParens(inParens(exprsInParensOrBindings())) }
+          atSpan(in.offset) { makeTupleOrParens(inParensWithCommas(exprsInParensOrBindings())) }
         case LBRACE | INDENT =>
           canApply = false
           blockExpr()
@@ -2577,15 +2614,15 @@ object Parsers {
     /** ParArgumentExprs ::= `(' [‘using’] [ExprsInParens] `)'
      *                    |  `(' [ExprsInParens `,'] PostfixExpr `*' ')'
      */
-    def parArgumentExprs(): (List[Tree], Boolean) = inParens {
-      if in.token == RPAREN then
-        (Nil, false)
-      else if isIdent(nme.using) then
-        in.nextToken()
-        (commaSeparated(argumentExpr), true)
-      else
-        (commaSeparated(argumentExpr), false)
-    }
+    def parArgumentExprs(): (List[Tree], Boolean) =
+      inParensWithCommas:
+        if in.token == RPAREN then
+          (Nil, false)
+        else if isIdent(nme.using) then
+          in.nextToken()
+          (commaSeparated(argumentExpr), true)
+        else
+          (commaSeparated(argumentExpr), false)
 
     /** ArgumentExprs ::= ParArgumentExprs
      *                 |  [nl] BlockExpr
@@ -2920,7 +2957,8 @@ object Parsers {
     def infixPattern(): Tree =
       infixOps(
         simplePattern(), in.canStartExprTokens, simplePatternFn, Location.InPattern, ParseKind.Pattern,
-        isOperator = in.name != nme.raw.BAR && !followingIsVararg())
+        isOperator = in.name != nme.raw.BAR && !followingIsVararg()
+                     && nextCanFollowOperator(canStartPatternTokens))
 
     /** SimplePattern    ::= PatVar
      *                    |  Literal
@@ -2941,7 +2979,7 @@ object Parsers {
       case USCORE =>
         wildcardIdent()
       case LPAREN =>
-        atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt())) }
+        atSpan(in.offset) { makeTupleOrParens(inParensWithCommas(patternsOpt())) }
       case QUOTE =>
         simpleExpr(Location.InPattern)
       case XMLSTART =>
@@ -2987,7 +3025,7 @@ object Parsers {
      *                      |  ‘(’ [Patterns ‘,’] PatVar ‘*’ ‘)’
      */
     def argumentPatterns(): List[Tree] =
-      inParens(patternsOpt(Location.InPatternArgs))
+      inParensWithCommas(patternsOpt(Location.InPatternArgs))
 
 /* -------- MODIFIERS and ANNOTATIONS ------------------------------------------- */
 
@@ -3176,7 +3214,7 @@ object Parsers {
      *  HkTypeParamClause ::=  ‘[’ HkTypeParam {‘,’ HkTypeParam} ‘]’
      *  HkTypeParam       ::=  {Annotation} [‘+’ | ‘-’] (id [HkTypePamClause] | ‘_’) TypeBounds
      */
-    def typeParamClause(ownerKind: ParamOwner): List[TypeDef] = inBrackets {
+    def typeParamClause(ownerKind: ParamOwner): List[TypeDef] = inBracketsWithCommas {
 
       def checkVarianceOK(): Boolean =
         val ok = ownerKind != ParamOwner.Def && ownerKind != ParamOwner.TypeParam
@@ -3315,7 +3353,7 @@ object Parsers {
       }
 
       // begin termParamClause
-      inParens {
+      inParensWithCommas {
         if in.token == RPAREN && !prefix && !impliedMods.is(Given) then Nil
         else
           val clause =

diff --git a/compiler/src/dotty/tools/dotc/parsing/Tokens.scala b/compiler/src/dotty/tools/dotc/parsing/Tokens.scala
@@ -221,10 +221,13 @@ object Tokens extends TokensCommon {
 
   final val openParensTokens = BitSet(LBRACE, LPAREN, LBRACKET)
 
-  final val canStartExprTokens3: TokenSet =
-      atomicExprTokens
+  final val canStartInfixExprTokens =
+    atomicExprTokens
     | openParensTokens
-    | BitSet(INDENT, QUOTE, IF, WHILE, FOR, NEW, TRY, THROW)
+    | BitSet(QUOTE, NEW)
+
+  final val canStartExprTokens3: TokenSet =
+    canStartInfixExprTokens | BitSet(INDENT, IF, WHILE, FOR, TRY, THROW)
 
   final val canStartExprTokens2: TokenSet = canStartExprTokens3 | BitSet(DO)
 
@@ -233,6 +236,8 @@ object Tokens extends TokensCommon {
 
   final val canStartTypeTokens: TokenSet = canStartInfixTypeTokens | BitSet(LBRACE)
 
+  final val canStartPatternTokens = atomicExprTokens | openParensTokens | BitSet(USCORE, QUOTE)
+
   final val templateIntroTokens: TokenSet = BitSet(CLASS, TRAIT, OBJECT, ENUM, CASECLASS, CASEOBJECT)
 
   final val dclIntroTokens: TokenSet = BitSet(DEF, VAL, VAR, TYPE, GIVEN)

diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala
@@ -1169,7 +1169,7 @@ extends ReferenceMsg(ForwardReferenceExtendsOverDefinitionID) {
         |"""
 }
 
-class ExpectedTokenButFound(expected: Token, found: Token)(using Context)
+class ExpectedTokenButFound(expected: Token, found: Token, prefix: String = "")(using Context)
 extends SyntaxMsg(ExpectedTokenButFoundID) {
 
   private def foundText = Tokens.showToken(found)
@@ -1178,7 +1178,7 @@ extends SyntaxMsg(ExpectedTokenButFoundID) {
     val expectedText =
       if (Tokens.isIdentifier(expected)) "an identifier"
       else Tokens.showToken(expected)
-    i"""${expectedText} expected, but ${foundText} found"""
+    i"""$prefix$expectedText expected, but $foundText found"""
 
   def explain(using Context) =
     if (Tokens.isIdentifier(expected) && Tokens.isKeyword(found))

diff --git a/tests/neg/cc-only-defs.scala b/tests/neg/cc-only-defs.scala
@@ -7,5 +7,5 @@ trait Test {
 
   val b: ImpureFunction1[Int, Int] // now OK
 
-  val a: {z} String // error
-}  // error
+  val a: {z} String // error // error
+}
diff --git a/tests/neg/i14564.check b/tests/neg/i14564.check
@@ -1,17 +1,4 @@
--- [E018] Syntax Error: tests/neg/i14564.scala:5:28 --------------------------------------------------------------------
-5 |def test = sum"${ List(42)* }" // error // error
-  |                            ^
-  |                            expression expected but [31m'}'[0m found
-  |
-  | longer explanation available when compiling with `-explain`
--- [E008] Not Found Error: tests/neg/i14564.scala:5:26 -----------------------------------------------------------------
-5 |def test = sum"${ List(42)* }" // error // error
-  |                  ^^^^^^^^^
-  |                  value * is not a member of List[Int], but could be made available as an extension method.
-  |
-  |                  One of the following imports might make progress towards fixing the problem:
-  |
-  |                    import scala.math.Fractional.Implicits.infixFractionalOps
-  |                    import scala.math.Integral.Implicits.infixIntegralOps
-  |                    import scala.math.Numeric.Implicits.infixNumericOps
-  |
+-- Error: tests/neg/i14564.scala:5:26 ----------------------------------------------------------------------------------
+5 |def test = sum"${ List(42)* }" // error
+  |                          ^
+  |                          spread operator `*` not allowed here; must come last in a parameter list
diff --git a/tests/neg/i14564.scala b/tests/neg/i14564.scala
@@ -2,5 +2,5 @@ import language.postfixOps as _
 
 extension (sc: StringContext) def sum(xs: Int*): String = xs.sum.toString
 
-def test = sum"${ List(42)* }" // error // error
+def test = sum"${ List(42)* }" // error
 
diff --git a/tests/neg/i18734.check b/tests/neg/i18734.check
@@ -0,0 +1,28 @@
+-- [E040] Syntax Error: tests/neg/i18734.scala:7:8 ---------------------------------------------------------------------
+7 |  Foo(1 2) // error
+  |        ^
+  |        ',' or ')' expected, but integer literal found
+-- [E040] Syntax Error: tests/neg/i18734.scala:9:8 ---------------------------------------------------------------------
+9 |  Foo(x y) // error
+  |        ^
+  |        ',' or ')' expected, but identifier found
+-- [E040] Syntax Error: tests/neg/i18734.scala:11:8 --------------------------------------------------------------------
+11 |  Foo(1 b = 2) // error
+   |        ^
+   |        ',' or ')' expected, but identifier found
+-- [E040] Syntax Error: tests/neg/i18734.scala:16:4 --------------------------------------------------------------------
+16 |    b = 2 // error
+   |    ^
+   |    ',' or ')' expected, but identifier found
+-- [E040] Syntax Error: tests/neg/i18734.scala:19:32 -------------------------------------------------------------------
+19 |  val f: (Int, Int) => Int = (x y) => x + y // error
+   |                                ^
+   |                                ',' or ')' expected, but identifier found
+-- [E040] Syntax Error: tests/neg/i18734.scala:23:10 -------------------------------------------------------------------
+23 |  bar[Int String](1 2) // error // error
+   |          ^^^^^^
+   |          ',' or ']' expected, but identifier found
+-- [E040] Syntax Error: tests/neg/i18734.scala:23:20 -------------------------------------------------------------------
+23 |  bar[Int String](1 2) // error // error
+   |                    ^
+   |                    ',' or ')' expected, but integer literal found
diff --git a/tests/neg/i18734.scala b/tests/neg/i18734.scala
@@ -0,0 +1,25 @@
+case class Foo(a: Int, b: Int)
+
+object Bar:
+  val x = 1
+  val y = 2
+
+  Foo(1 2) // error
+
+  Foo(x y) // error
+
+  Foo(1 b = 2) // error
+
+  // Or
+  Foo(
+    a = 1
+    b = 2 // error
+  )
+
+  val f: (Int, Int) => Int = (x y) => x + y // error
+
+  def bar[X, Y](x: X, y: Y) = ???
+
+  bar[Int String](1 2) // error // error
+
+
diff --git a/tests/neg/i4453.scala b/tests/neg/i4453.scala
@@ -1,2 +1,2 @@
-class x0 { var x0 == _ * // error: _* can be used only for last argument // error: == cannot be used as an extractor in a pattern because it lacks an unapply or unapplySeq method
-// error '=' expected, but eof found
+class x0 { var x0 == _ * // error spread operator `*` not allowed here // error '=' expected
+  // error: == cannot be used as an extractor in a pattern because it lacks an unapply or unapplySeq method
diff --git a/tests/neg/i5498-postfixOps.check b/tests/neg/i5498-postfixOps.check
@@ -4,13 +4,25 @@
   |          expression expected but [31mend of statement[0m found
   |
   | longer explanation available when compiling with `-explain`
--- [E018] Syntax Error: tests/neg/i5498-postfixOps.scala:6:37 ----------------------------------------------------------
-6 |  Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error
-  |                                     ^
-  |                                     expression expected but [31m')'[0m found
-  |
-  | longer explanation available when compiling with `-explain`
+-- [E040] Syntax Error: tests/neg/i5498-postfixOps.scala:6:29 ----------------------------------------------------------
+6 |  Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error // error (type error) // error (type error)
+  |                             ^^^^^^^^
+  |                             ',' or ')' expected, but identifier found
 -- [E172] Type Error: tests/neg/i5498-postfixOps.scala:6:0 -------------------------------------------------------------
-6 |  Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error
+6 |  Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error // error (type error) // error (type error)
   |^
   |No given instance of type scala.concurrent.duration.DurationConversions.Classifier[Null] was found for parameter ev of method second in trait DurationConversions
+-- [E007] Type Mismatch Error: tests/neg/i5498-postfixOps.scala:6:24 ---------------------------------------------------
+6 |  Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error // error (type error) // error (type error)
+  |                        ^
+  |                        Found:    (1 : Int)
+  |                        Required: Boolean
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E007] Type Mismatch Error: tests/neg/i5498-postfixOps.scala:6:26 ---------------------------------------------------
+6 |  Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error // error (type error) // error (type error)
+  |                          ^
+  |                          Found:    (2 : Int)
+  |                          Required: Boolean
+  |
+  | longer explanation available when compiling with `-explain`
diff --git a/tests/neg/i5498-postfixOps.scala b/tests/neg/i5498-postfixOps.scala
@@ -3,5 +3,5 @@ import scala.concurrent.duration.*
 def test() = {
   1 second // error: usage of postfix operator
 
-  Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error
+  Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error // error (type error) // error (type error)
 }
diff --git a/tests/neg/i6059.scala b/tests/neg/i6059.scala
@@ -1,3 +1,3 @@
 def I0(I1: Int ) = I1
-val I1 = I0(I0 i2) => // error
+val I1 = I0(I0 i2) => // error // error
   true