From 767eb9e93306aa6acee9ed1d10f6276e056bc30e Mon Sep 17 00:00:00 2001 From: adampauls Date: Thu, 17 Feb 2022 08:24:25 -0800 Subject: [PATCH] Better error recovery in comma-separated lists Check for trailing commas in parser instead of scanner Instead of passing down a flag, add a bit to regions to say if we are in a comma-separated region. delimited instead of expectedToken --- .../dotty/tools/dotc/parsing/Parsers.scala | 114 +++++++++++------- .../dotty/tools/dotc/parsing/Scanners.scala | 21 ++-- tests/neg/comma-separated-errors.check | 36 ++++++ tests/neg/comma-separated-errors.scala | 15 +++ tests/neg/i1679.scala | 2 +- tests/neg/t11900.check | 18 +++ tests/neg/t11900.scala | 79 ++++++++++++ tests/neg/trailingCommas.scala | 24 ++++ tests/pos/comma-separated.scala | 19 +++ 9 files changed, 269 insertions(+), 59 deletions(-) create mode 100644 tests/neg/comma-separated-errors.check create mode 100644 tests/neg/comma-separated-errors.scala 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 5543ebc0b798..5f7180cf86bd 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -249,8 +249,13 @@ object Parsers { /** Skip on error to next safe point. */ - protected def skip(stopAtComma: Boolean): Unit = + protected def skip(): Unit = val lastRegion = in.currentRegion + val stopAtComma = lastRegion match { + case InParens(_, _, commaSeparated) => commaSeparated + case InBraces(_, commaSeparated) => commaSeparated + case _ => true + } def atStop = in.token == EOF || ((stopAtComma && in.token == COMMA) || skipStopTokens.contains(in.token)) && (in.currentRegion eq lastRegion) @@ -278,13 +283,13 @@ object Parsers { if (in.token == EOF) incompleteInputError(msg) else syntaxError(msg, offset) - skip(stopAtComma = true) + skip() def syntaxErrorOrIncomplete(msg: Message, span: Span): Unit = if (in.token == EOF) incompleteInputError(msg) else syntaxError(msg, span) - skip(stopAtComma = true) + skip() /** Consume one token of the specified type, or * signal an error if it is not there. @@ -352,7 +357,7 @@ object Parsers { false // it's a statement that might be legal in an outer context else in.nextToken() // needed to ensure progress; otherwise we might cycle forever - skip(stopAtComma=false) + skip() true in.observeOutdented() @@ -559,19 +564,55 @@ 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] = { - val ts = new ListBuffer[T] += part() - while (in.token == separator) { + /** part { `,` part } + * @param delimited If true, this comma-separated list must surrounded by brackets, parens, or braces. + */ + def commaSeparated[T](part: () => T, delimited: Boolean = true, readFirst: Boolean = true): List[T] = { + val expectedEnd = if (delimited) { + in.currentRegion match { + case InParens(t, outer, _) => + in.currentRegion = InParens(t, outer, commaSeparated = true) + t match { + case LPAREN => RPAREN + case LBRACKET => RBRACKET + case _ => EMPTY + } + case InBraces(outer, _) => + in.currentRegion = InBraces(outer, commaSeparated = true) + RBRACE + case _ => EMPTY + } + } else EMPTY + 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 + syntaxErrorOrIncomplete(ExpectedTokenButFound(expectedEnd, in.token)) + if (in.token == COMMA) { + ts ++= commaSeparated(part, delimited) + } + } + if (delimited) { + in.currentRegion match { + case InParens(t, outer, true) => + in.currentRegion = InParens(t, outer, commaSeparated = false) + case InBraces(outer, true) => in.currentRegion = InBraces(outer, commaSeparated = false) + case _ => + } } 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) @@ -1387,14 +1428,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 = @@ -1410,11 +1444,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), readFirst = false) case t => - funTypeArgsRest(t, funArgType) + t :: commaSeparated(funArgType,readFirst = false) } accept(RPAREN) if isValParamList || in.isArrow then @@ -2539,7 +2572,7 @@ object Parsers { if (leading == LBRACE || in.token == CASE) enumerators() else { - val pats = patternsOpt() + val pats = patternsOpt(delimited=false) val pat = if (in.token == RPAREN || pats.length > 1) { wrappedEnums = false @@ -2731,7 +2764,7 @@ object Parsers { case USCORE => wildcardIdent() case LPAREN => - atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt())) } + atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt(delimited=true))) } case QUOTE => simpleExpr(Location.InPattern) case XMLSTART => @@ -2767,11 +2800,11 @@ object Parsers { /** Patterns ::= Pattern [`,' Pattern] */ - def patterns(location: Location = Location.InPattern): List[Tree] = - commaSeparated(() => pattern(location)) + def patterns(delimited: Boolean=false, location: Location = Location.InPattern): List[Tree] = + commaSeparated(() => pattern(location), delimited) - def patternsOpt(location: Location = Location.InPattern): List[Tree] = - if (in.token == RPAREN) Nil else patterns(location) + def patternsOpt(delimited: Boolean, location: Location = Location.InPattern): List[Tree] = + if (in.token == RPAREN) Nil else patterns(delimited, location) /** ArgumentPatterns ::= ‘(’ [Patterns] ‘)’ * | ‘(’ [Patterns ‘,’] PatVar ‘*’ ‘)’ @@ -3120,7 +3153,7 @@ object Parsers { */ def importClause(leading: Token, mkTree: ImportConstr): List[Tree] = { val offset = accept(leading) - commaSeparated(importExpr(mkTree)) match { + commaSeparated(importExpr(mkTree), delimited=false) match { case t :: rest => // The first import should start at the start offset of the keyword. val firstPos = @@ -3197,9 +3230,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() @@ -3209,13 +3242,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 @@ -3233,7 +3259,7 @@ object Parsers { case GIVEN => mkTree(qual, givenSelector() :: Nil) case LBRACE => - mkTree(qual, inBraces(importSelectors(idOK = true))) + mkTree(qual, inBraces(commaSeparated(importSelector(idOK = true)))) case _ => if isIdent(nme.raw.STAR) then mkTree(qual, wildcardSelector() :: Nil) @@ -3290,7 +3316,7 @@ object Parsers { var lhs = first match { case id: Ident if in.token == COMMA => in.nextToken() - id :: commaSeparated(() => termIdent()) + id :: commaSeparated(() => termIdent(), delimited=false) case _ => first :: Nil } @@ -3561,7 +3587,7 @@ object Parsers { val id = termIdent() if (in.token == COMMA) { in.nextToken() - val ids = commaSeparated(() => termIdent()) + val ids = commaSeparated(() => termIdent(), delimited=false) PatDef(mods1, id :: ids, TypeTree(), EmptyTree) } else { @@ -3765,7 +3791,7 @@ object Parsers { val derived = if (isIdent(nme.derives)) { in.nextToken() - tokenSeparated(COMMA, () => convertToTypeId(qualId())) + commaSeparated(() => convertToTypeId(qualId()), delimited=false) } else Nil possibleTemplateStart() diff --git a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala index 72b05ad073c2..74068751455d 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Scanners.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Scanners.scala @@ -281,7 +281,7 @@ object Scanners { /** Are we in a `${ }` block? such that RBRACE exits back into multiline string. */ private def inMultiLineInterpolatedExpression = currentRegion match { - case InBraces(InString(true, _)) => true + case InBraces(InString(true, _), _) => true case _ => false } @@ -315,7 +315,7 @@ object Scanners { dropBraces() case RPAREN | RBRACKET => currentRegion match { - case InParens(prefix, outer) if prefix + 1 == lastToken => currentRegion = outer + case InParens(prefix, outer, _) if prefix + 1 == lastToken => currentRegion = outer case _ => } case STRINGLIT => @@ -655,13 +655,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 => @@ -1451,7 +1444,7 @@ object Scanners { def proposeKnownWidth(width: IndentWidth, lastToken: Token) = if knownWidth == null then this match - case InParens(_, _) if lastToken != LPAREN => + case InParens(_, _, _) if lastToken != LPAREN => useOuterWidth() case _ => knownWidth = width @@ -1462,8 +1455,8 @@ object Scanners { private def delimiter = this match case _: InString => "}(in string)" - case InParens(LPAREN, _) => ")" - case InParens(LBRACKET, _) => "]" + case InParens(LPAREN, _, _) => ")" + case InParens(LBRACKET, _, _) => "]" case _: InBraces => "}" case _: InCase => "=>" case _: Indented => "UNDENT" @@ -1478,8 +1471,8 @@ object Scanners { end Region case class InString(multiLine: Boolean, outer: Region) extends Region - case class InParens(prefix: Token, outer: Region) extends Region - case class InBraces(outer: Region) extends Region + case class InParens(prefix: Token, outer: Region, commaSeparated: Boolean = false) extends Region + case class InBraces(outer: Region, commaSeparated: Boolean = false) extends Region case class InCase(outer: Region) extends Region /** A class describing an indentation region. 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 } 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