Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better error messages for missing commas and more #18785

Merged
merged 3 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 60 additions & 22 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 ------------------------------------------- */
Expand Down Expand Up @@ -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
Expand All @@ -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]
*/
Expand Down Expand Up @@ -1839,7 +1874,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())

Expand All @@ -1855,7 +1890,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)) }
Expand Down Expand Up @@ -2008,7 +2043,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 `}'
*/
Expand Down Expand Up @@ -2444,7 +2480,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)
Expand Down Expand Up @@ -2503,7 +2540,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()
Expand Down Expand Up @@ -2601,15 +2638,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
Expand Down Expand Up @@ -2947,7 +2984,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
Expand All @@ -2969,7 +3007,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 =>
Expand Down Expand Up @@ -3015,7 +3053,7 @@ object Parsers {
* | ‘(’ [Patterns ‘,’] PatVar ‘*’ ‘)’
*/
def argumentPatterns(): List[Tree] =
inParens(patternsOpt(Location.InPatternArgs))
inParensWithCommas(patternsOpt(Location.InPatternArgs))

/* -------- MODIFIERS and ANNOTATIONS ------------------------------------------- */

Expand Down Expand Up @@ -3204,7 +3242,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
Expand Down Expand Up @@ -3344,7 +3382,7 @@ object Parsers {
}

// begin termParamClause
inParens {
inParensWithCommas {
if in.token == RPAREN && !prefix && !impliedMods.is(Given) then Nil
else
val clause =
Expand Down
11 changes: 8 additions & 3 deletions compiler/src/dotty/tools/dotc/parsing/Tokens.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,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)
Expand All @@ -1196,7 +1196,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))
Expand Down
4 changes: 2 additions & 2 deletions tests/neg/cc-only-defs.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
21 changes: 4 additions & 17 deletions tests/neg/i14564.check
Original file line number Diff line number Diff line change
@@ -1,17 +1,4 @@
-- [E018] Syntax Error: tests/neg/i14564.scala:5:28 --------------------------------------------------------------------
5 |def test = sum"${ List(42)* }" // error // error
| ^
| expression expected but '}' 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
2 changes: 1 addition & 1 deletion tests/neg/i14564.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

2 changes: 1 addition & 1 deletion tests/neg/i18020.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def foo3 =
val _root_ = "abc" // error

def foo1: Unit =
val _root_: String = "abc" // error // error
val _root_: String = "abc" // error
// _root_: is, technically, a legal name
// so then it tries to construct the infix op pattern
// "_root_ String .." and then throws in a null when it fails
Expand Down
28 changes: 28 additions & 0 deletions tests/neg/i18734.check
Original file line number Diff line number Diff line change
@@ -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
25 changes: 25 additions & 0 deletions tests/neg/i18734.scala
Original file line number Diff line number Diff line change
@@ -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


4 changes: 2 additions & 2 deletions tests/neg/i4453.scala
Original file line number Diff line number Diff line change
@@ -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
26 changes: 19 additions & 7 deletions tests/neg/i5498-postfixOps.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,25 @@
| expression expected but end of statement 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 ')' 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`
2 changes: 1 addition & 1 deletion tests/neg/i5498-postfixOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion tests/neg/i6059.scala
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
def I0(I1: Int ) = I1
val I1 = I0(I0 i2) => // error
val I1 = I0(I0 i2) => // error // error
true
Loading
Loading