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

Parser simple expression error recovery change from null to ??? #19103

Merged
merged 2 commits into from
Dec 6, 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
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ object Parsers {
false
}

def errorTermTree(start: Offset): Literal = atSpan(start, in.offset, in.offset) { Literal(Constant(null)) }
def errorTermTree(start: Offset): Tree = atSpan(start, in.offset, in.offset) { unimplementedExpr }

private var inFunReturnType = false
private def fromWithinReturnType[T](body: => T): T = {
Expand Down
38 changes: 29 additions & 9 deletions compiler/src/dotty/tools/dotc/util/Signatures.scala
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,17 @@ object Signatures {
case tp @ TypeApply(fun, types) => applyCallInfo(span, types, fun, true)
case _ => (0, 0, Nil)


def isEnclosingApply(tree: tpd.Tree, span: Span)(using Context): Boolean =
tree match
case apply @ Apply(fun, _) => !fun.span.contains(span) && isValid(apply)
case unapply @ UnApply(fun, _, _) =>
!fun.span.contains(span) && !ctx.definitions.isFunctionNType(tree.tpe) // we want to show tuples in unapply
case typeTree @ AppliedTypeTree(fun, _) => !fun.span.contains(span) && isValid(typeTree)
case typeApply @ TypeApply(fun, _) => !fun.span.contains(span) && isValid(typeApply)
case _ => false


/**
* Finds enclosing application from given `path` for `span`.
*
Expand All @@ -108,17 +119,26 @@ object Signatures {
* next subsequent application exists, it returns the latter
*/
private def findEnclosingApply(path: List[tpd.Tree], span: Span)(using Context): tpd.Tree =
path.filterNot {
case apply @ Apply(fun, _) => fun.span.contains(span) || isValid(apply)
case unapply @ UnApply(fun, _, _) => fun.span.contains(span) || isValid(unapply)
case typeTree @ AppliedTypeTree(fun, _) => fun.span.contains(span) || isValid(typeTree)
case typeApply @ TypeApply(fun, _) => fun.span.contains(span) || isValid(typeApply)
case _ => true
} match {
import tpd.TreeOps

val filteredPath = path.filter:
case block @ Block(stats, expr) =>
block.existsSubTree(tree => isEnclosingApply(tree, span) && tree.span.contains(span))
case other => isEnclosingApply(other, span)

filteredPath match
case Nil => tpd.EmptyTree
case tpd.Block(stats, expr) :: _ => // potential block containing lifted args

val enclosingFunction = stats.collectFirst:
case defdef: tpd.DefDef if defdef.rhs.span.contains(span) => defdef

val enclosingTree = enclosingFunction.getOrElse(expr)
findEnclosingApply(Interactive.pathTo(enclosingTree, span), span)

case direct :: enclosing :: _ if isClosingSymbol(direct.source(span.end -1)) => enclosing
case direct :: _ => direct
}


private def isClosingSymbol(ch: Char) = ch == ')' || ch == ']'

Expand Down Expand Up @@ -303,7 +323,7 @@ object Signatures {
* @param tree tree to validate
*/
private def isValid(tree: tpd.Tree)(using Context): Boolean =
ctx.definitions.isTupleNType(tree.tpe) || ctx.definitions.isFunctionNType(tree.tpe)
!ctx.definitions.isTupleNType(tree.tpe) && !ctx.definitions.isFunctionNType(tree.tpe)

/**
* Get unapply method result type omiting unknown types and another method calls.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -516,13 +516,14 @@ class SignatureHelpTest {
.signatureHelp(m2, List(signature), Some(0), 1)
}

@Test def noUnapplyForTuple: Unit = {
@Test def unapplyForTuple: Unit = {
val signature = S("", Nil, List(List(P("", "Int"), P("", "Int"))), None)
code"""object Main {
| (1, 2) match
| case (x${m1}, ${m2}) =>
|}"""
.signatureHelp(m1, Nil, Some(0), 0)
.signatureHelp(m2, Nil, Some(0), 0)
.signatureHelp(m1, List(signature), Some(0), 0)
.signatureHelp(m2, List(signature), Some(0), 1)
}

@Test def unapplyCaseClass: Unit = {
Expand Down Expand Up @@ -693,7 +694,7 @@ class SignatureHelpTest {
.signatureHelp(m1, sigs, None, 0)
.signatureHelp(m2, sigs, None, 0)
.signatureHelp(m3, sigs, Some(2), 0)
.signatureHelp(m4, sigs, None, 1)
.signatureHelp(m4, List(sig0, sig1), None, 1)
.signatureHelp(m5, sigs, Some(2), 1)
.signatureHelp(m6, sigs, Some(0), 1)
.signatureHelp(m7, sigs, Some(1), 1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ object SignatureHelpProvider:
val pos = driver.sourcePosition(params)
val trees = driver.openedTrees(uri.nn)

val path =
Interactive.pathTo(trees, pos).dropWhile(t => notCurrentApply(t, pos))
val path = Interactive.pathTo(trees, pos)

val (paramN, callableN, alternatives) =
Signatures.signatureHelp(path, pos.span)
Expand Down Expand Up @@ -70,32 +69,6 @@ object SignatureHelpProvider:
)
end signatureHelp

private def isValid(tree: tpd.Tree)(using Context): Boolean =
ctx.definitions.isTupleClass(
tree.symbol.owner.companionClass
) || ctx.definitions.isFunctionType(tree.tpe)

private def notCurrentApply(
tree: tpd.Tree,
pos: SourcePosition
)(using Context): Boolean =
tree match
case unapply: tpd.UnApply =>
unapply.fun.span.contains(pos.span) || isValid(unapply)
case typeTree @ AppliedTypeTree(fun, _) =>
fun.span.contains(pos.span) || isValid(typeTree)
case typeApply @ TypeApply(fun, _) =>
fun.span.contains(pos.span) || isValid(typeApply)
case appl: tpd.GenericApply =>
/* find first apply that the cursor is located in arguments and not at function name
* for example in:
* `Option(1).fold(2)(@@_ + 1)`
* we want to find the tree responsible for the entire location, not just `_ + 1`
*/
appl.fun.span.contains(pos.span)

case _ => true

private def withDocumentation(
info: SymbolDocumentation,
signature: Signatures.Signature,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ class SignatureHelpPatternSuite extends BaseSignatureHelpSuite:
| }
|}
|""".stripMargin,
"""|map[B](f: ((Int, Int)) => B): List[B]
| ^^^^^^^^^^^^^^^^^^^^
"""|(Int, Int)
| ^^^
|""".stripMargin
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite:
|""".stripMargin
)

@Test def `local-method` =
@Test def `local-method` =
check(
"""
|object Main {
Expand All @@ -721,7 +721,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite:
|""".stripMargin,
)

@Test def `local-method2` =
@Test def `local-method2` =
check(
"""
|object Main {
Expand All @@ -739,7 +739,7 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite:
|""".stripMargin,
)

@Test def `local-method3` =
@Test def `local-method3` =
check(
"""
|object Main {
Expand All @@ -756,4 +756,76 @@ class SignatureHelpSuite extends BaseSignatureHelpSuite:
| ^^^^^^^^^
|apply(a: Int): Int
|""".stripMargin
)
)

@Test def `instantiated-type-var-old-ext-1` =
check(
"""|object O:
| implicit class Test[T](xs: List[T]):
| def test(x: T): List[T] = ???
| List(1,2,3).test(@@""".stripMargin,
"""|test(x: Int): List[Int]
| ^^^^^^
|""".stripMargin
)

@Test def `instantiated-type-var-old-ext-2` =
check(
"""|object O:
| implicit class Test[T](xs: List[T]):
| def test(x: T): List[T] = ???
| List(1,2,3).test(s@@""".stripMargin,
"""|test(x: Int): List[Int]
| ^^^^^^
|""".stripMargin
)

@Test def `instantiated-type-var-old-ext-3` =
check(
"""|object O:
| implicit class Test[T](xs: List[T]):
| def test(x: T): List[T] = ???
| List(1,2,3).test(@@)""".stripMargin,
"""|test(x: Int): List[Int]
| ^^^^^^
|""".stripMargin
)

@Test def `instantiated-type-var-old-ext-4` =
check(
"""|object O:
| implicit class Test[T](xs: List[T]):
| def test(x: T): List[T] = ???
| List(1,2,3).test(
| @@
| )""".stripMargin,
"""|test(x: Int): List[Int]
| ^^^^^^
|""".stripMargin
)

@Test def `instantiated-type-var-old-ext-5` =
check(
"""|object O:
| implicit class Test[T](xs: List[T]):
| def test(x: T): List[T] = ???
| List(1,2,3).test(
| @@
| println("test")
|""".stripMargin,
"""|test(x: Int | Unit): List[Int | Unit]
| ^^^^^^^^^^^^^
|""".stripMargin
)

@Test def `instantiated-type-var-old-ext-6` =
check(
"""|object O:
| implicit class Test[T](xs: List[T]):
| def test(y: String, x: T): List[T] = ???
| List(1,2,3).test("", @@)
|""".stripMargin,
"""|test(y: String, x: Int): List[Int]
| ^^^^^^
|""".stripMargin
)
4 changes: 2 additions & 2 deletions tests/neg/i5004.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
object i0 {
1 match {
def this(): Int // error
def this() // error
}
def this()
} // error
}
2 changes: 1 addition & 1 deletion tests/neg/i5498-postfixOps.check
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
-- [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 // 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
|Ambiguous given instances: both object spanConvert in object DurationConversions and object fromNowConvert in object DurationConversions match type scala.concurrent.duration.DurationConversions.Classifier[C] of 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)
| ^
Expand Down
1 change: 1 addition & 0 deletions tests/neg/parser-stability-1.scala
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
object x0 {
x1 match // error
def this // error
// error
42 changes: 0 additions & 42 deletions tests/neg/syntax-error-recovery.check
Original file line number Diff line number Diff line change
Expand Up @@ -94,48 +94,6 @@
| Not found: bam
|
| longer explanation available when compiling with `-explain`
-- [E190] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:7:2 -------------------------------------------
6 | 2
7 | }
| ^
| Discarded non-Unit value of type Null. You may want to use `()`.
|
| longer explanation available when compiling with `-explain`
-- [E190] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:15:2 ------------------------------------------
14 | 2
15 | println(baz) // error
| ^
| Discarded non-Unit value of type Null. You may want to use `()`.
|
| longer explanation available when compiling with `-explain`
-- [E190] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:27:2 ------------------------------------------
26 | 2
27 | println(bam) // error
| ^
| Discarded non-Unit value of type Null. You may want to use `()`.
|
| longer explanation available when compiling with `-explain`
-- [E190] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:36:2 ------------------------------------------
35 | 2
36 | }
| ^
| Discarded non-Unit value of type Null. You may want to use `()`.
|
| longer explanation available when compiling with `-explain`
-- [E190] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:44:2 ------------------------------------------
43 | 2
44 | println(baz) // error
| ^
| Discarded non-Unit value of type Null. You may want to use `()`.
|
| longer explanation available when compiling with `-explain`
-- [E190] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:56:2 ------------------------------------------
55 | 2
56 | println(bam) // error
| ^
| Discarded non-Unit value of type Null. You may want to use `()`.
|
| longer explanation available when compiling with `-explain`
-- Warning: tests/neg/syntax-error-recovery.scala:61:2 -----------------------------------------------------------------
61 | println(bam)
| ^^^^^^^
Expand Down
Loading