Skip to content

Commit 89ea0ec

Browse files
committed
Better error recovery in comma-separated lists
1 parent 29f9d33 commit 89ea0ec

File tree

9 files changed

+249
-64
lines changed

9 files changed

+249
-64
lines changed

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

Lines changed: 56 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -553,19 +553,35 @@ object Parsers {
553553
def inDefScopeBraces[T](body: => T, rewriteWithColon: Boolean = false): T =
554554
inBracesOrIndented(body, rewriteWithColon)
555555

556-
/** part { `separator` part }
557-
*/
558-
def tokenSeparated[T](separator: Int, part: () => T): List[T] = {
559-
val ts = new ListBuffer[T] += part()
560-
while (in.token == separator) {
556+
/** part { `,` part }
557+
* @param expectedEnd If set to something other than [[EMPTY]],
558+
* assume this comma separated list must be followed by this token.
559+
* If the parser consumes a `part` that is not followed by a comma or this expected
560+
* token, issue a syntax error and try to recover at the next safe point.
561+
*/
562+
def commaSeparated[T](part: () => T, expectedEnd: Token, readFirst: Boolean = true): List[T] = {
563+
val ts = new ListBuffer[T]
564+
if (readFirst) ts += part()
565+
var done = false
566+
while (in.token == COMMA && !done) {
561567
in.nextToken()
562-
ts += part()
568+
if (in.isAfterLineEnd && (in.token == OUTDENT || (expectedEnd != EMPTY && in.token == expectedEnd))) {
569+
// skip the trailing comma
570+
done = true
571+
} else {
572+
ts += part()
573+
}
574+
}
575+
if (expectedEnd != EMPTY && in.token != expectedEnd) {
576+
// As a side effect, will skip to the nearest safe point, which might be a comma
577+
syntaxErrorOrIncomplete(ExpectedTokenButFound(expectedEnd, in.token))
578+
if (in.token == COMMA) {
579+
ts ++= commaSeparated(part, expectedEnd)
580+
}
563581
}
564582
ts.toList
565583
}
566584

567-
def commaSeparated[T](part: () => T): List[T] = tokenSeparated(COMMA, part)
568-
569585
def inSepRegion[T](f: Region => Region)(op: => T): T =
570586
val cur = in.currentRegion
571587
in.currentRegion = f(cur)
@@ -1381,14 +1397,7 @@ object Parsers {
13811397
else
13821398
Function(params, t)
13831399
}
1384-
def funTypeArgsRest(first: Tree, following: () => Tree) = {
1385-
val buf = new ListBuffer[Tree] += first
1386-
while (in.token == COMMA) {
1387-
in.nextToken()
1388-
buf += following()
1389-
}
1390-
buf.toList
1391-
}
1400+
13921401
var isValParamList = false
13931402

13941403
val t =
@@ -1404,11 +1413,10 @@ object Parsers {
14041413
val ts = funArgType() match {
14051414
case Ident(name) if name != tpnme.WILDCARD && in.isColon() =>
14061415
isValParamList = true
1407-
funTypeArgsRest(
1408-
typedFunParam(paramStart, name.toTermName, imods),
1409-
() => typedFunParam(in.offset, ident(), imods))
1416+
typedFunParam(paramStart, name.toTermName, imods) :: commaSeparated(
1417+
() => typedFunParam(in.offset, ident(), imods), RPAREN, readFirst = false)
14101418
case t =>
1411-
funTypeArgsRest(t, funArgType)
1419+
t :: commaSeparated(funArgType, RPAREN, readFirst = false)
14121420
}
14131421
accept(RPAREN)
14141422
if isValParamList || in.isArrow then
@@ -1509,7 +1517,7 @@ object Parsers {
15091517
/** FunParamClause ::= ‘(’ TypedFunParam {‘,’ TypedFunParam } ‘)’
15101518
*/
15111519
def funParamClause(): List[ValDef] =
1512-
inParens(commaSeparated(() => typedFunParam(in.offset, ident())))
1520+
inParens(commaSeparated(() => typedFunParam(in.offset, ident()), RPAREN))
15131521

15141522
def funParamClauses(): List[List[ValDef]] =
15151523
if in.token == LPAREN then funParamClause() :: funParamClauses() else Nil
@@ -1622,7 +1630,7 @@ object Parsers {
16221630
else
16231631
def singletonArgs(t: Tree): Tree =
16241632
if in.token == LPAREN && in.featureEnabled(Feature.dependent)
1625-
then singletonArgs(AppliedTypeTree(t, inParens(commaSeparated(singleton))))
1633+
then singletonArgs(AppliedTypeTree(t, inParens(commaSeparated(singleton, RPAREN))))
16261634
else t
16271635
singletonArgs(simpleType1())
16281636

@@ -1638,7 +1646,7 @@ object Parsers {
16381646
def simpleType1() = simpleTypeRest {
16391647
if in.token == LPAREN then
16401648
atSpan(in.offset) {
1641-
makeTupleOrParens(inParens(argTypes(namedOK = false, wildOK = true)))
1649+
makeTupleOrParens(inParens(argTypes(namedOK = false, wildOK = true, RPAREN)))
16421650
}
16431651
else if in.token == LBRACE then
16441652
atSpan(in.offset) { RefinedTypeTree(EmptyTree, refinement(indentOK = false)) }
@@ -1722,7 +1730,7 @@ object Parsers {
17221730
* | NamedTypeArg {`,' NamedTypeArg}
17231731
* NamedTypeArg ::= id `=' Type
17241732
*/
1725-
def argTypes(namedOK: Boolean, wildOK: Boolean): List[Tree] = {
1733+
def argTypes(namedOK: Boolean, wildOK: Boolean, expectedEnd: Token): List[Tree] = {
17261734

17271735
def argType() = {
17281736
val t = typ()
@@ -1739,7 +1747,7 @@ object Parsers {
17391747
val rest =
17401748
if (in.token == COMMA) {
17411749
in.nextToken()
1742-
commaSeparated(arg)
1750+
commaSeparated(arg, expectedEnd)
17431751
}
17441752
else Nil
17451753
first :: rest
@@ -1752,7 +1760,7 @@ object Parsers {
17521760
case firstArg =>
17531761
otherArgs(firstArg, () => argType())
17541762
}
1755-
else commaSeparated(() => argType())
1763+
else commaSeparated(() => argType(), expectedEnd)
17561764
}
17571765

17581766
/** FunArgType ::= Type | `=>' Type
@@ -1781,7 +1789,7 @@ object Parsers {
17811789
/** TypeArgs ::= `[' Type {`,' Type} `]'
17821790
* NamedTypeArgs ::= `[' NamedTypeArg {`,' NamedTypeArg} `]'
17831791
*/
1784-
def typeArgs(namedOK: Boolean, wildOK: Boolean): List[Tree] = inBrackets(argTypes(namedOK, wildOK))
1792+
def typeArgs(namedOK: Boolean, wildOK: Boolean): List[Tree] = inBrackets(argTypes(namedOK, wildOK, RBRACKET))
17851793

17861794
/** Refinement ::= `{' RefineStatSeq `}'
17871795
*/
@@ -2145,7 +2153,7 @@ object Parsers {
21452153
var mods1 = mods
21462154
if isErased then mods1 = addModifier(mods1)
21472155
try
2148-
commaSeparated(() => binding(mods1))
2156+
commaSeparated(() => binding(mods1), RPAREN)
21492157
finally
21502158
accept(RPAREN)
21512159
else {
@@ -2376,7 +2384,7 @@ object Parsers {
23762384
/** ExprsInParens ::= ExprInParens {`,' ExprInParens}
23772385
*/
23782386
def exprsInParensOpt(): List[Tree] =
2379-
if (in.token == RPAREN) Nil else commaSeparated(exprInParens)
2387+
if (in.token == RPAREN) Nil else commaSeparated(exprInParens, RPAREN)
23802388

23812389
/** ParArgumentExprs ::= `(' [‘using’] [ExprsInParens] `)'
23822390
* | `(' [ExprsInParens `,'] PostfixExpr `*' ')'
@@ -2386,9 +2394,9 @@ object Parsers {
23862394
(Nil, false)
23872395
else if isIdent(nme.using) then
23882396
in.nextToken()
2389-
(commaSeparated(argumentExpr), true)
2397+
(commaSeparated(argumentExpr, RPAREN), true)
23902398
else
2391-
(commaSeparated(argumentExpr), false)
2399+
(commaSeparated(argumentExpr, RPAREN), false)
23922400
}
23932401

23942402
/** ArgumentExprs ::= ParArgumentExprs
@@ -2532,7 +2540,7 @@ object Parsers {
25322540
if (leading == LBRACE || in.token == CASE)
25332541
enumerators()
25342542
else {
2535-
val pats = patternsOpt()
2543+
val pats = patternsOpt(EMPTY)
25362544
val pat =
25372545
if (in.token == RPAREN || pats.length > 1) {
25382546
wrappedEnums = false
@@ -2724,7 +2732,7 @@ object Parsers {
27242732
case USCORE =>
27252733
wildcardIdent()
27262734
case LPAREN =>
2727-
atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt())) }
2735+
atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt(RPAREN))) }
27282736
case QUOTE =>
27292737
simpleExpr(Location.InPattern)
27302738
case XMLSTART =>
@@ -2759,17 +2767,17 @@ object Parsers {
27592767

27602768
/** Patterns ::= Pattern [`,' Pattern]
27612769
*/
2762-
def patterns(location: Location = Location.InPattern): List[Tree] =
2763-
commaSeparated(() => pattern(location))
2770+
def patterns(expectedEnd: Token = EMPTY, location: Location = Location.InPattern): List[Tree] =
2771+
commaSeparated(() => pattern(location), expectedEnd)
27642772

2765-
def patternsOpt(location: Location = Location.InPattern): List[Tree] =
2766-
if (in.token == RPAREN) Nil else patterns(location)
2773+
def patternsOpt(expectedEnd: Token, location: Location = Location.InPattern): List[Tree] =
2774+
if (in.token == RPAREN) Nil else patterns(expectedEnd, location)
27672775

27682776
/** ArgumentPatterns ::= ‘(’ [Patterns] ‘)’
27692777
* | ‘(’ [Patterns ‘,’] PatVar ‘*’ ‘)’
27702778
*/
27712779
def argumentPatterns(): List[Tree] =
2772-
inParens(patternsOpt(Location.InPatternArgs))
2780+
inParens(patternsOpt(RPAREN, Location.InPatternArgs))
27732781

27742782
/* -------- MODIFIERS and ANNOTATIONS ------------------------------------------- */
27752783

@@ -2950,7 +2958,7 @@ object Parsers {
29502958
TypeDef(name, lambdaAbstract(hkparams, bounds)).withMods(mods)
29512959
}
29522960
}
2953-
commaSeparated(() => typeParam())
2961+
commaSeparated(() => typeParam(), RBRACKET)
29542962
}
29552963

29562964
def typeParamClauseOpt(ownerKind: ParamOwner.Value): List[TypeDef] =
@@ -2959,7 +2967,7 @@ object Parsers {
29592967
/** ContextTypes ::= FunArgType {‘,’ FunArgType}
29602968
*/
29612969
def contextTypes(ofClass: Boolean, nparams: Int, impliedMods: Modifiers): List[ValDef] =
2962-
val tps = commaSeparated(funArgType)
2970+
val tps = commaSeparated(funArgType, RPAREN)
29632971
var counter = nparams
29642972
def nextIdx = { counter += 1; counter }
29652973
val paramFlags = if ofClass then Private | Local | ParamAccessor else Param
@@ -3063,7 +3071,7 @@ object Parsers {
30633071
!impliedMods.is(Given)
30643072
|| startParamTokens.contains(in.token)
30653073
|| isIdent && (in.name == nme.inline || in.lookahead.isColon())
3066-
if isParams then commaSeparated(() => param())
3074+
if isParams then commaSeparated(() => param(), RPAREN)
30673075
else contextTypes(ofClass, nparams, impliedMods)
30683076
checkVarArgsRules(clause)
30693077
clause
@@ -3112,7 +3120,7 @@ object Parsers {
31123120
*/
31133121
def importClause(leading: Token, mkTree: ImportConstr): List[Tree] = {
31143122
val offset = accept(leading)
3115-
commaSeparated(importExpr(mkTree)) match {
3123+
commaSeparated(importExpr(mkTree), EMPTY) match {
31163124
case t :: rest =>
31173125
// The first import should start at the start offset of the keyword.
31183126
val firstPos =
@@ -3189,9 +3197,9 @@ object Parsers {
31893197
}
31903198
else ImportSelector(from)
31913199

3192-
def importSelectors(idOK: Boolean): List[ImportSelector] =
3200+
def importSelector(idOK: Boolean)(): ImportSelector =
31933201
val isWildcard = in.token == USCORE || in.token == GIVEN || isIdent(nme.raw.STAR)
3194-
val selector = atSpan(in.offset) {
3202+
atSpan(in.offset) {
31953203
in.token match
31963204
case USCORE => wildcardSelector()
31973205
case GIVEN => givenSelector()
@@ -3201,13 +3209,6 @@ object Parsers {
32013209
if !idOK then syntaxError(i"named imports cannot follow wildcard imports")
32023210
namedSelector(termIdent())
32033211
}
3204-
val rest =
3205-
if in.token == COMMA then
3206-
in.nextToken()
3207-
importSelectors(idOK = idOK && !isWildcard)
3208-
else
3209-
Nil
3210-
selector :: rest
32113212

32123213
def importSelection(qual: Tree): Tree =
32133214
if in.isIdent(nme.as) && qual.isInstanceOf[RefTree] then
@@ -3225,7 +3226,7 @@ object Parsers {
32253226
case GIVEN =>
32263227
mkTree(qual, givenSelector() :: Nil)
32273228
case LBRACE =>
3228-
mkTree(qual, inBraces(importSelectors(idOK = true)))
3229+
mkTree(qual, inBraces(commaSeparated(importSelector(idOK = true), RBRACE)))
32293230
case _ =>
32303231
if isIdent(nme.raw.STAR) then
32313232
mkTree(qual, wildcardSelector() :: Nil)
@@ -3282,7 +3283,7 @@ object Parsers {
32823283
var lhs = first match {
32833284
case id: Ident if in.token == COMMA =>
32843285
in.nextToken()
3285-
id :: commaSeparated(() => termIdent())
3286+
id :: commaSeparated(() => termIdent(), EMPTY)
32863287
case _ =>
32873288
first :: Nil
32883289
}
@@ -3551,7 +3552,7 @@ object Parsers {
35513552
val id = termIdent()
35523553
if (in.token == COMMA) {
35533554
in.nextToken()
3554-
val ids = commaSeparated(() => termIdent())
3555+
val ids = commaSeparated(() => termIdent(), EMPTY)
35553556
PatDef(mods1, id :: ids, TypeTree(), EmptyTree)
35563557
}
35573558
else {
@@ -3755,7 +3756,7 @@ object Parsers {
37553756
val derived =
37563757
if (isIdent(nme.derives)) {
37573758
in.nextToken()
3758-
tokenSeparated(COMMA, () => convertToTypeId(qualId()))
3759+
commaSeparated(() => convertToTypeId(qualId()), EMPTY)
37593760
}
37603761
else Nil
37613762
possibleTemplateStart()

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -654,13 +654,6 @@ object Scanners {
654654
insert(OUTDENT, offset)
655655
currentRegion = r.outer
656656
case _ =>
657-
lookAhead()
658-
if isAfterLineEnd
659-
&& (token == RPAREN || token == RBRACKET || token == RBRACE || token == OUTDENT)
660-
then
661-
() /* skip the trailing comma */
662-
else
663-
reset()
664657
case END =>
665658
if !isEndMarker then token = IDENTIFIER
666659
case COLON =>
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:21 ----------------------------------------------------
2+
3 | def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
3+
| ^
4+
| ')' expected, but integer literal found
5+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:26 ----------------------------------------------------
6+
3 | def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
7+
| ^^^
8+
| ':' expected, but identifier found
9+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:42 ----------------------------------------------------
10+
3 | def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
11+
| ^
12+
| ')' expected, but integer literal found
13+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:47 ----------------------------------------------------
14+
3 | def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
15+
| ^
16+
| ':' expected, but '=' found
17+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:11:16 ---------------------------------------------------
18+
11 | case Plus(4 1) => // error
19+
| ^
20+
| ')' expected, but integer literal found
21+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:12:16 ---------------------------------------------------
22+
12 | case Plus(4 5 6 7, 1, 2 3) => // error // error
23+
| ^
24+
| ')' expected, but integer literal found
25+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:12:28 ---------------------------------------------------
26+
12 | case Plus(4 5 6 7, 1, 2 3) => // error // error
27+
| ^
28+
| ')' expected, but integer literal found
29+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:14:12 ---------------------------------------------------
30+
14 | val x: A[T=Int, T=Int] = ??? // error // error
31+
| ^
32+
| ']' expected, but '=' found
33+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:14:19 ---------------------------------------------------
34+
14 | val x: A[T=Int, T=Int] = ??? // error // error
35+
| ^
36+
| ']' expected, but '=' found
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
class A[T]
2+
object o {
3+
def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
4+
5+
case class Plus(a: Int, b: Int)
6+
7+
object Plus {
8+
def unapply(r: Int): Plus = Plus(r - 1, 1)
9+
}
10+
5 match {
11+
case Plus(4 1) => // error
12+
case Plus(4 5 6 7, 1, 2 3) => // error // error
13+
}
14+
val x: A[T=Int, T=Int] = ??? // error // error
15+
}

tests/neg/i1679.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
class A[T]
22
object o {
33
// Testing compiler crash, this test should be modified when named type argument are completely implemented
4-
val x: A[T=Int, T=Int] = ??? // error: ']' expected, but '=' found // error
4+
val x: A[T=Int, T=Int] = ??? // error: ']' expected, but '=' found // error: ']' expected, but '=' found
55
}

0 commit comments

Comments
 (0)