Skip to content

Commit 10a2b83

Browse files
oderskyodersky
authored
Better error messages for missing commas and more (#18785)
Three measures: 1. Classify an id as infix operator only if following can start an operand 2. Detect and report spread operators in illegal positions 3. Mention `,` in addition to `)` or `]` in error messages when a `,` could have been missing Fixes #18734
2 parents 22aaabb + fca7d06 commit 10a2b83

19 files changed

+175
-88
lines changed

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

Lines changed: 60 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -555,10 +555,29 @@ object Parsers {
555555
accept(tok)
556556
try body finally accept(tok + 1)
557557

558+
/** Same as enclosed, but if closing token is missing, add `,` to the expected tokens
559+
* in the error message provided the next token could have followed a `,`.
560+
*/
561+
def enclosedWithCommas[T](tok: Token, body: => T): T =
562+
accept(tok)
563+
val closing = tok + 1
564+
val isEmpty = in.token == closing
565+
val ts = body
566+
if in.token != closing then
567+
val followComma =
568+
if tok == LPAREN then canStartExprTokens3 else canStartTypeTokens
569+
val prefix = if !isEmpty && followComma.contains(in.token) then "',' or " else ""
570+
syntaxErrorOrIncomplete(ExpectedTokenButFound(closing, in.token, prefix))
571+
if in.token == closing then in.nextToken()
572+
ts
573+
558574
def inParens[T](body: => T): T = enclosed(LPAREN, body)
559575
def inBraces[T](body: => T): T = enclosed(LBRACE, body)
560576
def inBrackets[T](body: => T): T = enclosed(LBRACKET, body)
561577

578+
def inParensWithCommas[T](body: => T): T = enclosedWithCommas(LPAREN, body)
579+
def inBracketsWithCommas[T](body: => T): T = enclosedWithCommas(LBRACKET, body)
580+
562581
def inBracesOrIndented[T](body: => T, rewriteWithColon: Boolean = false): T =
563582
if in.token == INDENT then
564583
val rewriteToBraces = in.rewriteNoIndent
@@ -970,6 +989,17 @@ object Parsers {
970989
isArrowIndent()
971990
else false
972991

992+
/** Can the next lookahead token start an operand as defined by
993+
* leadingOperandTokens, or is postfix ops enabled?
994+
* This is used to decide whether the current token can be an infix operator.
995+
*/
996+
def nextCanFollowOperator(leadingOperandTokens: BitSet): Boolean =
997+
leadingOperandTokens.contains(in.lookahead.token)
998+
|| in.postfixOpsEnabled
999+
|| in.lookahead.token == COLONop
1000+
|| in.lookahead.token == EOF // important for REPL completions
1001+
|| ctx.mode.is(Mode.Interactive) // in interactive mode the next tokens might be missing
1002+
9731003
/* --------- OPERAND/OPERATOR STACK --------------------------------------- */
9741004

9751005
var opStack: List[OpInfo] = Nil
@@ -1050,7 +1080,11 @@ object Parsers {
10501080
then recur(top)
10511081
else top
10521082

1053-
recur(first)
1083+
val res = recur(first)
1084+
if isIdent(nme.raw.STAR) && !followingIsVararg() then
1085+
syntaxError(em"spread operator `*` not allowed here; must come last in a parameter list")
1086+
in.nextToken()
1087+
res
10541088
end infixOps
10551089

10561090
/* -------- IDENTIFIERS AND LITERALS ------------------------------------------- */
@@ -1659,7 +1693,7 @@ object Parsers {
16591693
/** FunParamClause ::= ‘(’ TypedFunParam {‘,’ TypedFunParam } ‘)’
16601694
*/
16611695
def funParamClause(): List[ValDef] =
1662-
inParens(commaSeparated(() => typedFunParam(in.offset, ident())))
1696+
inParensWithCommas(commaSeparated(() => typedFunParam(in.offset, ident())))
16631697

16641698
def funParamClauses(): List[List[ValDef]] =
16651699
if in.token == LPAREN then funParamClause() :: funParamClauses() else Nil
@@ -1671,7 +1705,8 @@ object Parsers {
16711705

16721706
def infixTypeRest(t: Tree): Tree =
16731707
infixOps(t, canStartInfixTypeTokens, refinedTypeFn, Location.ElseWhere, ParseKind.Type,
1674-
isOperator = !followingIsVararg() && !isPureArrow)
1708+
isOperator = !followingIsVararg() && !isPureArrow
1709+
&& nextCanFollowOperator(canStartInfixTypeTokens))
16751710

16761711
/** RefinedType ::= WithType {[nl] Refinement} [`^` CaptureSet]
16771712
*/
@@ -1839,7 +1874,7 @@ object Parsers {
18391874
else
18401875
def singletonArgs(t: Tree): Tree =
18411876
if in.token == LPAREN && in.featureEnabled(Feature.dependent)
1842-
then singletonArgs(AppliedTypeTree(t, inParens(commaSeparated(singleton))))
1877+
then singletonArgs(AppliedTypeTree(t, inParensWithCommas(commaSeparated(singleton))))
18431878
else t
18441879
singletonArgs(simpleType1())
18451880

@@ -1855,7 +1890,7 @@ object Parsers {
18551890
def simpleType1() = simpleTypeRest {
18561891
if in.token == LPAREN then
18571892
atSpan(in.offset) {
1858-
makeTupleOrParens(inParens(argTypes(namedOK = false, wildOK = true)))
1893+
makeTupleOrParens(inParensWithCommas(argTypes(namedOK = false, wildOK = true)))
18591894
}
18601895
else if in.token == LBRACE then
18611896
atSpan(in.offset) { RefinedTypeTree(EmptyTree, refinement(indentOK = false)) }
@@ -2008,7 +2043,8 @@ object Parsers {
20082043
/** TypeArgs ::= `[' Type {`,' Type} `]'
20092044
* NamedTypeArgs ::= `[' NamedTypeArg {`,' NamedTypeArg} `]'
20102045
*/
2011-
def typeArgs(namedOK: Boolean, wildOK: Boolean): List[Tree] = inBrackets(argTypes(namedOK, wildOK))
2046+
def typeArgs(namedOK: Boolean, wildOK: Boolean): List[Tree] =
2047+
inBracketsWithCommas(argTypes(namedOK, wildOK))
20122048

20132049
/** Refinement ::= `{' RefineStatSeq `}'
20142050
*/
@@ -2444,7 +2480,8 @@ object Parsers {
24442480

24452481
def postfixExprRest(t: Tree, location: Location): Tree =
24462482
infixOps(t, in.canStartExprTokens, prefixExpr, location, ParseKind.Expr,
2447-
isOperator = !(location.inArgs && followingIsVararg()))
2483+
isOperator = !(location.inArgs && followingIsVararg())
2484+
&& nextCanFollowOperator(canStartInfixExprTokens))
24482485

24492486
/** PrefixExpr ::= [PrefixOperator'] SimpleExpr
24502487
* PrefixOperator ::= ‘-’ | ‘+’ | ‘~’ | ‘!’ (if not backquoted)
@@ -2503,7 +2540,7 @@ object Parsers {
25032540
placeholderParams = param :: placeholderParams
25042541
atSpan(start) { Ident(pname) }
25052542
case LPAREN =>
2506-
atSpan(in.offset) { makeTupleOrParens(inParens(exprsInParensOrBindings())) }
2543+
atSpan(in.offset) { makeTupleOrParens(inParensWithCommas(exprsInParensOrBindings())) }
25072544
case LBRACE | INDENT =>
25082545
canApply = false
25092546
blockExpr()
@@ -2601,15 +2638,15 @@ object Parsers {
26012638
/** ParArgumentExprs ::= `(' [‘using’] [ExprsInParens] `)'
26022639
* | `(' [ExprsInParens `,'] PostfixExpr `*' ')'
26032640
*/
2604-
def parArgumentExprs(): (List[Tree], Boolean) = inParens {
2605-
if in.token == RPAREN then
2606-
(Nil, false)
2607-
else if isIdent(nme.using) then
2608-
in.nextToken()
2609-
(commaSeparated(argumentExpr), true)
2610-
else
2611-
(commaSeparated(argumentExpr), false)
2612-
}
2641+
def parArgumentExprs(): (List[Tree], Boolean) =
2642+
inParensWithCommas:
2643+
if in.token == RPAREN then
2644+
(Nil, false)
2645+
else if isIdent(nme.using) then
2646+
in.nextToken()
2647+
(commaSeparated(argumentExpr), true)
2648+
else
2649+
(commaSeparated(argumentExpr), false)
26132650

26142651
/** ArgumentExprs ::= ParArgumentExprs
26152652
* | [nl] BlockExpr
@@ -2947,7 +2984,8 @@ object Parsers {
29472984
def infixPattern(): Tree =
29482985
infixOps(
29492986
simplePattern(), in.canStartExprTokens, simplePatternFn, Location.InPattern, ParseKind.Pattern,
2950-
isOperator = in.name != nme.raw.BAR && !followingIsVararg())
2987+
isOperator = in.name != nme.raw.BAR && !followingIsVararg()
2988+
&& nextCanFollowOperator(canStartPatternTokens))
29512989

29522990
/** SimplePattern ::= PatVar
29532991
* | Literal
@@ -2969,7 +3007,7 @@ object Parsers {
29693007
case USCORE =>
29703008
wildcardIdent()
29713009
case LPAREN =>
2972-
atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt())) }
3010+
atSpan(in.offset) { makeTupleOrParens(inParensWithCommas(patternsOpt())) }
29733011
case QUOTE =>
29743012
simpleExpr(Location.InPattern)
29753013
case XMLSTART =>
@@ -3015,7 +3053,7 @@ object Parsers {
30153053
* | ‘(’ [Patterns ‘,’] PatVar ‘*’ ‘)’
30163054
*/
30173055
def argumentPatterns(): List[Tree] =
3018-
inParens(patternsOpt(Location.InPatternArgs))
3056+
inParensWithCommas(patternsOpt(Location.InPatternArgs))
30193057

30203058
/* -------- MODIFIERS and ANNOTATIONS ------------------------------------------- */
30213059

@@ -3204,7 +3242,7 @@ object Parsers {
32043242
* HkTypeParamClause ::= ‘[’ HkTypeParam {‘,’ HkTypeParam} ‘]’
32053243
* HkTypeParam ::= {Annotation} [‘+’ | ‘-’] (id [HkTypePamClause] | ‘_’) TypeBounds
32063244
*/
3207-
def typeParamClause(ownerKind: ParamOwner): List[TypeDef] = inBrackets {
3245+
def typeParamClause(ownerKind: ParamOwner): List[TypeDef] = inBracketsWithCommas {
32083246

32093247
def checkVarianceOK(): Boolean =
32103248
val ok = ownerKind != ParamOwner.Def && ownerKind != ParamOwner.TypeParam
@@ -3344,7 +3382,7 @@ object Parsers {
33443382
}
33453383

33463384
// begin termParamClause
3347-
inParens {
3385+
inParensWithCommas {
33483386
if in.token == RPAREN && !prefix && !impliedMods.is(Given) then Nil
33493387
else
33503388
val clause =

‎compiler/src/dotty/tools/dotc/parsing/Tokens.scala

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -221,10 +221,13 @@ object Tokens extends TokensCommon {
221221

222222
final val openParensTokens = BitSet(LBRACE, LPAREN, LBRACKET)
223223

224-
final val canStartExprTokens3: TokenSet =
225-
atomicExprTokens
224+
final val canStartInfixExprTokens =
225+
atomicExprTokens
226226
| openParensTokens
227-
| BitSet(INDENT, QUOTE, IF, WHILE, FOR, NEW, TRY, THROW)
227+
| BitSet(QUOTE, NEW)
228+
229+
final val canStartExprTokens3: TokenSet =
230+
canStartInfixExprTokens | BitSet(INDENT, IF, WHILE, FOR, TRY, THROW)
228231

229232
final val canStartExprTokens2: TokenSet = canStartExprTokens3 | BitSet(DO)
230233

@@ -233,6 +236,8 @@ object Tokens extends TokensCommon {
233236

234237
final val canStartTypeTokens: TokenSet = canStartInfixTypeTokens | BitSet(LBRACE)
235238

239+
final val canStartPatternTokens = atomicExprTokens | openParensTokens | BitSet(USCORE, QUOTE)
240+
236241
final val templateIntroTokens: TokenSet = BitSet(CLASS, TRAIT, OBJECT, ENUM, CASECLASS, CASEOBJECT)
237242

238243
final val dclIntroTokens: TokenSet = BitSet(DEF, VAL, VAR, TYPE, GIVEN)

‎compiler/src/dotty/tools/dotc/reporting/messages.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,7 +1169,7 @@ extends ReferenceMsg(ForwardReferenceExtendsOverDefinitionID) {
11691169
|"""
11701170
}
11711171

1172-
class ExpectedTokenButFound(expected: Token, found: Token)(using Context)
1172+
class ExpectedTokenButFound(expected: Token, found: Token, prefix: String = "")(using Context)
11731173
extends SyntaxMsg(ExpectedTokenButFoundID) {
11741174

11751175
private def foundText = Tokens.showToken(found)
@@ -1178,7 +1178,7 @@ extends SyntaxMsg(ExpectedTokenButFoundID) {
11781178
val expectedText =
11791179
if (Tokens.isIdentifier(expected)) "an identifier"
11801180
else Tokens.showToken(expected)
1181-
i"""${expectedText} expected, but ${foundText} found"""
1181+
i"""$prefix$expectedText expected, but $foundText found"""
11821182

11831183
def explain(using Context) =
11841184
if (Tokens.isIdentifier(expected) && Tokens.isKeyword(found))

‎tests/neg/cc-only-defs.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ trait Test {
77

88
val b: ImpureFunction1[Int, Int] // now OK
99

10-
val a: {z} String // error
11-
} // error
10+
val a: {z} String // error // error
11+
}

‎tests/neg/i14564.check

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,4 @@
1-
-- [E018] Syntax Error: tests/neg/i14564.scala:5:28 --------------------------------------------------------------------
2-
5 |def test = sum"${ List(42)* }" // error // error
3-
| ^
4-
| expression expected but '}' found
5-
|
6-
| longer explanation available when compiling with `-explain`
7-
-- [E008] Not Found Error: tests/neg/i14564.scala:5:26 -----------------------------------------------------------------
8-
5 |def test = sum"${ List(42)* }" // error // error
9-
| ^^^^^^^^^
10-
| value * is not a member of List[Int], but could be made available as an extension method.
11-
|
12-
| One of the following imports might make progress towards fixing the problem:
13-
|
14-
| import scala.math.Fractional.Implicits.infixFractionalOps
15-
| import scala.math.Integral.Implicits.infixIntegralOps
16-
| import scala.math.Numeric.Implicits.infixNumericOps
17-
|
1+
-- Error: tests/neg/i14564.scala:5:26 ----------------------------------------------------------------------------------
2+
5 |def test = sum"${ List(42)* }" // error
3+
| ^
4+
| spread operator `*` not allowed here; must come last in a parameter list

‎tests/neg/i14564.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@ import language.postfixOps as _
22

33
extension (sc: StringContext) def sum(xs: Int*): String = xs.sum.toString
44

5-
def test = sum"${ List(42)* }" // error // error
5+
def test = sum"${ List(42)* }" // error
66

‎tests/neg/i18020.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ def foo3 =
1616
val _root_ = "abc" // error
1717

1818
def foo1: Unit =
19-
val _root_: String = "abc" // error // error
19+
val _root_: String = "abc" // error
2020
// _root_: is, technically, a legal name
2121
// so then it tries to construct the infix op pattern
2222
// "_root_ String .." and then throws in a null when it fails

‎tests/neg/i18734.check

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
-- [E040] Syntax Error: tests/neg/i18734.scala:7:8 ---------------------------------------------------------------------
2+
7 | Foo(1 2) // error
3+
| ^
4+
| ',' or ')' expected, but integer literal found
5+
-- [E040] Syntax Error: tests/neg/i18734.scala:9:8 ---------------------------------------------------------------------
6+
9 | Foo(x y) // error
7+
| ^
8+
| ',' or ')' expected, but identifier found
9+
-- [E040] Syntax Error: tests/neg/i18734.scala:11:8 --------------------------------------------------------------------
10+
11 | Foo(1 b = 2) // error
11+
| ^
12+
| ',' or ')' expected, but identifier found
13+
-- [E040] Syntax Error: tests/neg/i18734.scala:16:4 --------------------------------------------------------------------
14+
16 | b = 2 // error
15+
| ^
16+
| ',' or ')' expected, but identifier found
17+
-- [E040] Syntax Error: tests/neg/i18734.scala:19:32 -------------------------------------------------------------------
18+
19 | val f: (Int, Int) => Int = (x y) => x + y // error
19+
| ^
20+
| ',' or ')' expected, but identifier found
21+
-- [E040] Syntax Error: tests/neg/i18734.scala:23:10 -------------------------------------------------------------------
22+
23 | bar[Int String](1 2) // error // error
23+
| ^^^^^^
24+
| ',' or ']' expected, but identifier found
25+
-- [E040] Syntax Error: tests/neg/i18734.scala:23:20 -------------------------------------------------------------------
26+
23 | bar[Int String](1 2) // error // error
27+
| ^
28+
| ',' or ')' expected, but integer literal found

‎tests/neg/i18734.scala

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
case class Foo(a: Int, b: Int)
2+
3+
object Bar:
4+
val x = 1
5+
val y = 2
6+
7+
Foo(1 2) // error
8+
9+
Foo(x y) // error
10+
11+
Foo(1 b = 2) // error
12+
13+
// Or
14+
Foo(
15+
a = 1
16+
b = 2 // error
17+
)
18+
19+
val f: (Int, Int) => Int = (x y) => x + y // error
20+
21+
def bar[X, Y](x: X, y: Y) = ???
22+
23+
bar[Int String](1 2) // error // error
24+
25+

‎tests/neg/i4453.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
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
2-
// error '=' expected, but eof found
1+
class x0 { var x0 == _ * // error spread operator `*` not allowed here // error '=' expected
2+
// error: == cannot be used as an extractor in a pattern because it lacks an unapply or unapplySeq method

‎tests/neg/i5498-postfixOps.check

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,25 @@
44
| expression expected but end of statement found
55
|
66
| longer explanation available when compiling with `-explain`
7-
-- [E018] Syntax Error: tests/neg/i5498-postfixOps.scala:6:37 ----------------------------------------------------------
8-
6 | Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error
9-
| ^
10-
| expression expected but ')' found
11-
|
12-
| longer explanation available when compiling with `-explain`
7+
-- [E040] Syntax Error: tests/neg/i5498-postfixOps.scala:6:29 ----------------------------------------------------------
8+
6 | Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error // error (type error) // error (type error)
9+
| ^^^^^^^^
10+
| ',' or ')' expected, but identifier found
1311
-- [E172] Type Error: tests/neg/i5498-postfixOps.scala:6:0 -------------------------------------------------------------
14-
6 | Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error
12+
6 | Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error // error (type error) // error (type error)
1513
|^
1614
|No given instance of type scala.concurrent.duration.DurationConversions.Classifier[Null] was found for parameter ev of method second in trait DurationConversions
15+
-- [E007] Type Mismatch Error: tests/neg/i5498-postfixOps.scala:6:24 ---------------------------------------------------
16+
6 | Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error // error (type error) // error (type error)
17+
| ^
18+
| Found: (1 : Int)
19+
| Required: Boolean
20+
|
21+
| longer explanation available when compiling with `-explain`
22+
-- [E007] Type Mismatch Error: tests/neg/i5498-postfixOps.scala:6:26 ---------------------------------------------------
23+
6 | Seq(1, 2).filter(List(1,2) contains) // error: usage of postfix operator // error // error (type error) // error (type error)
24+
| ^
25+
| Found: (2 : Int)
26+
| Required: Boolean
27+
|
28+
| longer explanation available when compiling with `-explain`

‎tests/neg/i5498-postfixOps.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,5 @@ import scala.concurrent.duration.*
33
def test() = {
44
1 second // error: usage of postfix operator
55

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)
77
}

‎tests/neg/i6059.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
def I0(I1: Int ) = I1
2-
val I1 = I0(I0 i2) => // error
2+
val I1 = I0(I0 i2) => // error // error
33
true

0 commit comments

Comments
 (0)