From 317b08c937e7b55e48d0541a4a752db2c67e0c44 Mon Sep 17 00:00:00 2001 From: adampauls Date: Thu, 17 Feb 2022 08:24:25 -0800 Subject: [PATCH 1/3] Better error recovery in comma-separated lists --- .../dotty/tools/dotc/parsing/Parsers.scala | 65 +++++++++++-------- tests/neg/comma-separated-errors.check | 36 ++++++++++ tests/neg/comma-separated-errors.scala | 15 +++++ tests/neg/i1679.scala | 2 +- 4 files changed, 89 insertions(+), 29 deletions(-) create mode 100644 tests/neg/comma-separated-errors.check create mode 100644 tests/neg/comma-separated-errors.scala diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 7a5620be0415..0422583183a7 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -562,19 +562,28 @@ 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] = { + /** part { `,` part } + * @param expectedEnd If set to something other than [[EMPTY]], + * assume this comma separated list must be followed by this token. + * 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 == separator) { + while (in.token == COMMA) { in.nextToken() 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, expectedEnd) + } + } 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) @@ -1519,7 +1528,7 @@ object Parsers { /** FunParamClause ::= ‘(’ TypedFunParam {‘,’ TypedFunParam } ‘)’ */ def funParamClause(): List[ValDef] = - inParens(commaSeparated(() => typedFunParam(in.offset, ident()))) + inParens(commaSeparated(() => typedFunParam(in.offset, ident()), RPAREN)) def funParamClauses(): List[List[ValDef]] = if in.token == LPAREN then funParamClause() :: funParamClauses() else Nil @@ -1631,7 +1640,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, inParens(commaSeparated(singleton, RPAREN)))) else t singletonArgs(simpleType1()) @@ -1647,7 +1656,7 @@ object Parsers { def simpleType1() = simpleTypeRest { if in.token == LPAREN then atSpan(in.offset) { - makeTupleOrParens(inParens(argTypes(namedOK = false, wildOK = true))) + makeTupleOrParens(inParens(argTypes(namedOK = false, wildOK = true, RPAREN))) } else if in.token == LBRACE then atSpan(in.offset) { RefinedTypeTree(EmptyTree, refinement(indentOK = false)) } @@ -1731,7 +1740,7 @@ object Parsers { * | NamedTypeArg {`,' NamedTypeArg} * NamedTypeArg ::= id `=' Type */ - def argTypes(namedOK: Boolean, wildOK: Boolean): List[Tree] = { + def argTypes(namedOK: Boolean, wildOK: Boolean, expectedEnd: Token): List[Tree] = { def argType() = { val t = typ() @@ -1748,7 +1757,7 @@ object Parsers { val rest = if (in.token == COMMA) { in.nextToken() - commaSeparated(arg) + commaSeparated(arg, expectedEnd) } else Nil first :: rest @@ -1761,7 +1770,7 @@ object Parsers { case firstArg => otherArgs(firstArg, () => argType()) } - else commaSeparated(() => argType()) + else commaSeparated(() => argType(), expectedEnd) } /** FunArgType ::= Type | `=>' Type @@ -1790,7 +1799,7 @@ 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] = inBrackets(argTypes(namedOK, wildOK, RBRACKET)) /** Refinement ::= `{' RefineStatSeq `}' */ @@ -2154,7 +2163,7 @@ object Parsers { var mods1 = mods if isErased then mods1 = addModifier(mods1) try - commaSeparated(() => binding(mods1)) + commaSeparated(() => binding(mods1), RPAREN) finally accept(RPAREN) else { @@ -2384,7 +2393,7 @@ object Parsers { /** ExprsInParens ::= ExprInParens {`,' ExprInParens} */ def exprsInParensOpt(): List[Tree] = - if (in.token == RPAREN) Nil else commaSeparated(exprInParens) + if (in.token == RPAREN) Nil else commaSeparated(exprInParens, RPAREN) /** ParArgumentExprs ::= `(' [‘using’] [ExprsInParens] `)' * | `(' [ExprsInParens `,'] PostfixExpr `*' ')' @@ -2394,9 +2403,9 @@ object Parsers { (Nil, false) else if isIdent(nme.using) then in.nextToken() - (commaSeparated(argumentExpr), true) + (commaSeparated(argumentExpr, RPAREN), true) else - (commaSeparated(argumentExpr), false) + (commaSeparated(argumentExpr, RPAREN), false) } /** ArgumentExprs ::= ParArgumentExprs @@ -2540,7 +2549,7 @@ object Parsers { if (leading == LBRACE || in.token == CASE) enumerators() else { - val pats = patternsOpt() + val pats = patternsOpt(EMPTY) val pat = if (in.token == RPAREN || pats.length > 1) { wrappedEnums = false @@ -2732,7 +2741,7 @@ object Parsers { case USCORE => wildcardIdent() case LPAREN => - atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt())) } + atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt(RPAREN))) } case QUOTE => simpleExpr(Location.InPattern) case XMLSTART => @@ -2768,17 +2777,17 @@ object Parsers { /** Patterns ::= Pattern [`,' Pattern] */ - def patterns(location: Location = Location.InPattern): List[Tree] = - commaSeparated(() => pattern(location)) + def patterns(expectedEnd: Token = EMPTY, location: Location = Location.InPattern): List[Tree] = + commaSeparated(() => pattern(location), expectedEnd) - def patternsOpt(location: Location = Location.InPattern): List[Tree] = - if (in.token == RPAREN) Nil else patterns(location) + def patternsOpt(expectedEnd: Token, location: Location = Location.InPattern): List[Tree] = + if (in.token == RPAREN) Nil else patterns(expectedEnd, location) /** ArgumentPatterns ::= ‘(’ [Patterns] ‘)’ * | ‘(’ [Patterns ‘,’] PatVar ‘*’ ‘)’ */ def argumentPatterns(): List[Tree] = - inParens(patternsOpt(Location.InPatternArgs)) + inParens(patternsOpt(RPAREN, Location.InPatternArgs)) /* -------- MODIFIERS and ANNOTATIONS ------------------------------------------- */ @@ -2959,7 +2968,7 @@ object Parsers { TypeDef(name, lambdaAbstract(hkparams, bounds)).withMods(mods) } } - commaSeparated(() => typeParam()) + commaSeparated(() => typeParam(), RBRACKET) } def typeParamClauseOpt(ownerKind: ParamOwner): List[TypeDef] = @@ -2968,7 +2977,7 @@ object Parsers { /** ContextTypes ::= FunArgType {‘,’ FunArgType} */ def contextTypes(ofClass: Boolean, nparams: Int, impliedMods: Modifiers): List[ValDef] = - val tps = commaSeparated(funArgType) + val tps = commaSeparated(funArgType, RPAREN) var counter = nparams def nextIdx = { counter += 1; counter } val paramFlags = if ofClass then Private | Local | ParamAccessor else Param @@ -3072,7 +3081,7 @@ object Parsers { !impliedMods.is(Given) || startParamTokens.contains(in.token) || isIdent && (in.name == nme.inline || in.lookahead.isColon()) - if isParams then commaSeparated(() => param()) + if isParams then commaSeparated(() => param(), RPAREN) else contextTypes(ofClass, nparams, impliedMods) checkVarArgsRules(clause) clause @@ -3766,7 +3775,7 @@ object Parsers { val derived = if (isIdent(nme.derives)) { in.nextToken() - tokenSeparated(COMMA, () => convertToTypeId(qualId())) + commaSeparated(() => convertToTypeId(qualId())) } else Nil possibleTemplateStart() diff --git a/tests/neg/comma-separated-errors.check b/tests/neg/comma-separated-errors.check new file mode 100644 index 000000000000..3b74c2ab29c2 --- /dev/null +++ b/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 diff --git a/tests/neg/comma-separated-errors.scala b/tests/neg/comma-separated-errors.scala new file mode 100644 index 000000000000..8eb7965cd3e9 --- /dev/null +++ b/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 +} diff --git a/tests/neg/i1679.scala b/tests/neg/i1679.scala index cadeb85dc8db..6ca81cea6406 100644 --- a/tests/neg/i1679.scala +++ b/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 } From fe885986c081a50a293d762e536a8a1ff4e69344 Mon Sep 17 00:00:00 2001 From: adampauls Date: Thu, 17 Feb 2022 08:24:25 -0800 Subject: [PATCH 2/3] Check for trailing commas in parser instead of scanner --- .../dotty/tools/dotc/parsing/Parsers.scala | 52 ++++++------ .../dotty/tools/dotc/parsing/Scanners.scala | 7 -- tests/neg/t11900.check | 18 +++++ tests/neg/t11900.scala | 79 +++++++++++++++++++ tests/neg/trailingCommas.scala | 24 ++++++ tests/pos/comma-separated.scala | 19 +++++ 6 files changed, 162 insertions(+), 37 deletions(-) create mode 100644 tests/neg/t11900.check create mode 100644 tests/neg/t11900.scala create mode 100644 tests/pos/comma-separated.scala diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 0422583183a7..7aa0119b7c3c 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/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() diff --git a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala index 8407ac5f2089..c2dfcec7defb 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala +++ b/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 => diff --git a/tests/neg/t11900.check b/tests/neg/t11900.check new file mode 100644 index 000000000000..531a1b8417fd --- /dev/null +++ b/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` \ No newline at end of file diff --git a/tests/neg/t11900.scala b/tests/neg/t11900.scala new file mode 100644 index 000000000000..d45f06bf180b --- /dev/null +++ b/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 +} \ No newline at end of file diff --git a/tests/neg/trailingCommas.scala b/tests/neg/trailingCommas.scala index 2a24fc83c79e..c3a2c98c65a7 100644 --- a/tests/neg/trailingCommas.scala +++ b/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 \ No newline at end of file diff --git a/tests/pos/comma-separated.scala b/tests/pos/comma-separated.scala new file mode 100644 index 000000000000..d97b7dd9e2ee --- /dev/null +++ b/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) \ No newline at end of file From 8a09cb5d494f9b81843d7c27e785c4141e76f672 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 16 Mar 2022 12:34:13 +0100 Subject: [PATCH 3/3] fix trailing commas in community build --- community-build/community-projects/protoquill | 2 +- community-build/community-projects/specs2 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/community-build/community-projects/protoquill b/community-build/community-projects/protoquill index 12b3649dfe93..16d26fcb3072 160000 --- a/community-build/community-projects/protoquill +++ b/community-build/community-projects/protoquill @@ -1 +1 @@ -Subproject commit 12b3649dfe93229d1e19fd698db4ee1b8a1ffddd +Subproject commit 16d26fcb30720b9aa81d29f08b9da10916e269a2 diff --git a/community-build/community-projects/specs2 b/community-build/community-projects/specs2 index 0652daeefb57..e1ae96e7a55f 160000 --- a/community-build/community-projects/specs2 +++ b/community-build/community-projects/specs2 @@ -1 +1 @@ -Subproject commit 0652daeefb57c2d51e3f16ea5c44929bdba722bf +Subproject commit e1ae96e7a55fed2268f9ccd391687a5ac96ee4df