Skip to content

Commit a967a25

Browse files
DuhemmDuhemm
authored
Merge pull request #5484 from dotty-staging/fix-error-pos-2
 Report the correct errors in the IDE and add test infrastructure for diagnostics
2 parents 3730250 + d180287 commit a967a25

File tree

17 files changed

+93
-33
lines changed

17 files changed

+93
-33
lines changed

‎compiler/src/dotty/tools/dotc/reporting/Reporter.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,8 +238,10 @@ abstract class Reporter extends interfaces.ReporterResult {
238238
def isHidden(m: MessageContainer)(implicit ctx: Context): Boolean =
239239
ctx.mode.is(Mode.Printing)
240240

241-
/** Does this reporter contain not yet reported errors or warnings? */
242-
def hasPendingErrors: Boolean = false
241+
/** Does this reporter contain errors that have yet to be reported by its outer reporter ?
242+
* Note: this is always false when there is no outer reporter.
243+
*/
244+
def hasUnreportedErrors: Boolean = false
243245

244246
/** If this reporter buffers messages, remove and return all buffered messages. */
245247
def removeBufferedMessages(implicit ctx: Context): List[MessageContainer] = Nil

‎compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ class StoreReporter(outer: Reporter) extends Reporter {
2828
infos += m
2929
}
3030

31-
override def hasPendingErrors: Boolean =
32-
infos != null && infos.exists(_.isInstanceOf[Error])
31+
override def hasUnreportedErrors: Boolean =
32+
outer != null && infos != null && infos.exists(_.isInstanceOf[Error])
3333

3434
override def removeBufferedMessages(implicit ctx: Context): List[MessageContainer] =
3535
if (infos != null) try infos.toList finally infos = null

‎compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -291,11 +291,10 @@ object messages {
291291
val kind: String = "Type Mismatch"
292292
val msg: String = {
293293
val (where, printCtx) = Formatting.disambiguateTypes(found, expected)
294+
val whereSuffix = if (where.isEmpty) where else s"\n\n$where"
294295
val (fnd, exp) = Formatting.typeDiff(found, expected)(printCtx)
295296
s"""|found: $fnd
296-
|required: $exp
297-
|
298-
|$where""".stripMargin + whyNoMatch + implicitFailure
297+
|required: $exp""".stripMargin + whereSuffix + whyNoMatch + implicitFailure
299298
}
300299

301300
val explanation: String = ""

‎compiler/src/dotty/tools/dotc/typer/Inferencing.scala

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -432,10 +432,7 @@ trait Inferencing { this: Typer =>
432432
// found : Int(1)
433433
// required: String
434434
// val y: List[List[String]] = List(List(1))
435-
val hasUnreportedErrors = state.reporter match {
436-
case r: StoreReporter if r.hasErrors => true
437-
case _ => false
438-
}
435+
val hasUnreportedErrors = state.reporter.hasUnreportedErrors
439436
def constraint = state.constraint
440437
for (tvar <- qualifying)
441438
if (!tvar.isInstantiated && state.constraint.contains(tvar)) {

‎compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ object ProtoTypes {
264264
targ = arg.withType(WildcardType)
265265
else {
266266
targ = typerFn(arg)
267-
if (!ctx.reporter.hasPendingErrors) {
267+
if (!ctx.reporter.hasUnreportedErrors) {
268268
// FIXME: This can swallow warnings by updating the typerstate from a nested
269269
// context that gets discarded later. But we do have to update the
270270
// typerstate if there are no errors. If we also omitted the next two lines

‎compiler/src/dotty/tools/dotc/util/ParsedComment.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ object ParsedComment {
117117
"@throws" -> TagFormatter("Throws", toDescriptionList),
118118
"@see" -> TagFormatter("See Also", toMarkdownList),
119119
"@example" -> TagFormatter("Examples", toCodeFences("scala")),
120-
"@usecase" -> TagFormatter("Usecases", toCodeFences("scala")),
121120
"@note" -> TagFormatter("Note", toMarkdownList),
122121
"@author" -> TagFormatter("Authors", toMarkdownList),
123122
"@since" -> TagFormatter("Since", toMarkdownList),

‎compiler/test-resources/repl/errmsgs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,36 +5,30 @@ scala> val x: List[String] = List(1)
55
| ^
66
| found: Int(1)
77
| required: String
8-
|
98
scala> val y: List[List[String]] = List(List(1))
109
1 | val y: List[List[String]] = List(List(1))
1110
| ^
1211
| found: Int(1)
1312
| required: String
14-
|
1513
scala> val z: (List[String], List[Int]) = (List(1), List("a"))
1614
1 | val z: (List[String], List[Int]) = (List(1), List("a"))
1715
| ^
1816
| found: Int(1)
1917
| required: String
20-
|
2118
1 | val z: (List[String], List[Int]) = (List(1), List("a"))
2219
| ^^^
2320
| found: String("a")
2421
| required: Int
25-
|
2622
scala> val a: Inv[String] = new Inv(new Inv(1))
2723
1 | val a: Inv[String] = new Inv(new Inv(1))
2824
| ^^^^^^^^^^
2925
| found: Inv[Int]
3026
| required: String
31-
|
3227
scala> val b: Inv[String] = new Inv(1)
3328
1 | val b: Inv[String] = new Inv(1)
3429
| ^
3530
| found: Int(1)
3631
| required: String
37-
|
3832
scala> abstract class C { type T; val x: T; val s: Unit = { type T = String; var y: T = x; locally { def f() = { type T = Int; val z: T = y }; f() } }; }
3933
1 | abstract class C { type T; val x: T; val s: Unit = { type T = String; var y: T = x; locally { def f() = { type T = Int; val z: T = y }; f() } }; }
4034
| ^
@@ -59,7 +53,6 @@ scala> val x: List[Int] = "foo" :: List(1)
5953
| ^^^^^
6054
| found: String("foo")
6155
| required: Int
62-
|
6356
scala> { def f: Int = g; val x: Int = 1; def g: Int = 5; }
6457
1 | { def f: Int = g; val x: Int = 1; def g: Int = 5; }
6558
| ^

‎compiler/test-resources/repl/importFromObj

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ scala> buf += xs
99
| ^^
1010
| found: List[Int](o.xs)
1111
| required: Int
12-
|
1312
scala> buf ++= xs
1413
val res0: scala.collection.mutable.ListBuffer[Int] = ListBuffer(1, 2, 3)
1514
scala> import util.foo

‎compiler/test-resources/type-printer/type-mismatch

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,3 @@ scala> val x: Foo[String] = res0
77
| ^^^^
88
| found: Foo[Int](res0)
99
| required: Foo[String]
10-
|

‎language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package dotty.tools.languageserver
2+
3+
import org.junit.Test
4+
5+
import dotty.tools.dotc.reporting.diagnostic.ErrorMessageID._
6+
import dotty.tools.languageserver.util.Code._
7+
import org.eclipse.lsp4j.DiagnosticSeverity._
8+
9+
class DiagnosticsTest {
10+
@Test def diagnosticWrongType: Unit =
11+
code"""object Test {
12+
| val x: Int = $m1"foo"$m2
13+
|}""".withSource
14+
.diagnostics(m1,
15+
(m1 to m2, """found: String("foo")
16+
|required: Int""".stripMargin, Error, Some(TypeMismatchID))
17+
)
18+
19+
@Test def diagnosticMissingLambdaBody: Unit =
20+
code"""object Test {
21+
| Nil.map(x => x).filter(x$m1 =>$m2)
22+
|$m3}""".withSource
23+
.diagnostics(m1,
24+
(m2 to m3, "illegal start of simple expression", Error, Some(IllegalStartSimpleExprID)),
25+
(m1 to m1, """found: Null
26+
|required: Boolean""".stripMargin, Error, Some(TypeMismatchID))
27+
)
28+
29+
@Test def diagnosticPureExpression: Unit =
30+
code"""object Test {
31+
| ${m1}1$m2
32+
|}""".withSource
33+
.diagnostics(m1,
34+
(m1 to m2,
35+
"a pure expression does nothing in statement position; you may be omitting necessary parentheses",
36+
Warning, Some(PureExpressionInStatementPositionID)))
37+
38+
@Test def diagnosticWorksheetPureExpression: Unit =
39+
ws"""${m1}1""".withSource
40+
.diagnostics(m1 /* no "pure expression" warning because this is a worksheet */)
41+
}

‎language-server/test/dotty/tools/languageserver/HoverTest.scala

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,11 +153,6 @@ class HoverTest {
153153
| myFoo.bar[Int, String](0, "hello, world")
154154
| ```
155155
|
156-
|**Usecases**
157-
| - ```scala
158-
| def bar(fizz: Int, buzz: String): Any
159-
| ```
160-
|
161156
|**Note**
162157
| - A note
163158
|

‎language-server/test/dotty/tools/languageserver/SymbolTest.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ class SymbolTest {
2121
code"class $Bar",
2222
code"""package foo
2323
|class $fooFoo {
24-
| class Bar
24+
| class ${m7}Bar$m8
2525
|}
2626
"""
2727
) .symbol("Foo", Foo.range.symInfo("Foo", SymbolKind.Class), fooFoo.range.symInfo("Foo", SymbolKind.Class, "foo"))
28-
.symbol("Bar", Bar.range.symInfo("Bar", SymbolKind.Class))
28+
.symbol("Bar", Bar.range.symInfo("Bar", SymbolKind.Class), (m7 to m8).symInfo("Bar", SymbolKind.Class, "Foo"))
2929
}
3030

3131
@Test def symbolShowModule: Unit = {

‎language-server/test/dotty/tools/languageserver/util/Code.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ object Code {
106106
}
107107

108108
if (pi.hasNext)
109-
stringBuilder.append(pi.next())
109+
stringBuilder.append(pi.next().stripMargin)
110110

111111
(stringBuilder.result(), positions.result())
112112
}

‎language-server/test/dotty/tools/languageserver/util/CodeRange.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ case class CodeRange(start: CodeMarker, end: CodeMarker) {
1919
if (!checked) {
2020
assert(start.file == end.file, s"$start and $end where not in the same file")
2121
assert(start.line <= end.line, s"Expected $end to be after $start")
22-
assert(start.line != end.line || start.character < end.character, s"Expected $end to be after $start")
22+
assert(start.line != end.line || start.character <= end.character, s"Expected $end to be at or after $start")
2323
checked = true
2424
}
2525
}

‎language-server/test/dotty/tools/languageserver/util/CodeTester.scala

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@ import dotty.tools.languageserver.util.actions._
55
import dotty.tools.languageserver.util.embedded.CodeMarker
66
import dotty.tools.languageserver.util.server.{TestFile, TestServer}
77

8+
import dotty.tools.dotc.reporting.diagnostic.ErrorMessageID
89
import dotty.tools.dotc.util.Signatures.Signature
910

10-
import org.eclipse.lsp4j.{CompletionItemKind, DocumentHighlightKind}
11+
import org.eclipse.lsp4j.{ CompletionItemKind, DocumentHighlightKind, Diagnostic, DiagnosticSeverity }
12+
13+
import org.junit.Assert.assertEquals
1114

1215
/**
1316
* Simulates an LSP client for test in a project defined by `sources`.
@@ -30,6 +33,31 @@ class CodeTester(projects: List[Project]) {
3033

3134
private val positions: PositionContext = getPositions(files)
3235

36+
/** Check that the last diagnostics that have been published so far by the server
37+
* for a given file match `expected`.
38+
*
39+
* @param marker The marker defining the source file from which to query.
40+
* @param expected The expected diagnostics to be found
41+
*/
42+
def diagnostics(marker: CodeMarker,
43+
expected: (CodeRange, String, DiagnosticSeverity, Option[ErrorMessageID])*): this.type = {
44+
implicit val posCtx = positions
45+
46+
def toDiagnostic(range: CodeRange, message: String, severity: DiagnosticSeverity,
47+
errorCode: Option[ErrorMessageID]): Diagnostic = {
48+
new Diagnostic(range.toRange, message, severity, /*source =*/"",
49+
errorCode.map(_.errorNumber.toString).orNull)
50+
}
51+
52+
val expectedParams = marker.toPublishDiagnosticsParams(expected.toList.map(toDiagnostic))
53+
// Find the latest published diagnostics for the current source file
54+
val actualParams = testServer.client.diagnostics.get.reverse.find(_.getUri == marker.uri)
55+
.getOrElse(throw new Exception(s"No published diagnostics for ${marker.uri}"))
56+
assertEquals(expectedParams, actualParams)
57+
58+
this
59+
}
60+
3361
/**
3462
* Perform a hover over `range`, verifies that result matches `expected`.
3563
*

‎language-server/test/dotty/tools/languageserver/util/embedded/CodeMarker.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package dotty.tools.languageserver.util.embedded
33
import dotty.tools.languageserver.util.server.TestFile
44
import dotty.tools.languageserver.util.{CodeRange, PositionContext}
55

6+
import scala.collection.JavaConverters._
7+
68
import org.eclipse.lsp4j._
79

810
import PositionContext.PosCtx
@@ -16,6 +18,9 @@ class CodeMarker(val name: String) extends Embedded {
1618
/** The file containing this marker. */
1719
def file: PosCtx[TestFile] = posCtx.positionOf(this)._1
1820

21+
/** The uri of the file containing this marker. */
22+
def uri: PosCtx[String] = file.uri
23+
1924
/** The line containing this marker. */
2025
def line: PosCtx[Int] = posCtx.positionOf(this)._2
2126

@@ -34,6 +39,9 @@ class CodeMarker(val name: String) extends Embedded {
3439
def toCompletionParams: PosCtx[CompletionParams] =
3540
new CompletionParams(toTextDocumentIdentifier, toPosition)
3641

42+
def toPublishDiagnosticsParams(diagnostics: List[Diagnostic]): PosCtx[PublishDiagnosticsParams] =
43+
new PublishDiagnosticsParams(file.uri, diagnostics.asJava)
44+
3745
def toRenameParams(newName: String): PosCtx[RenameParams] =
3846
new RenameParams(toTextDocumentIdentifier, toPosition, newName)
3947

‎project/Build.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,7 @@ object Build {
894894
).
895895
settings(
896896
ideTestsCompilerVersion := (version in `dotty-compiler`).value,
897-
ideTestsCompilerArguments := (scalacOptions in `dotty-compiler`).value,
897+
ideTestsCompilerArguments := Seq(),
898898
ideTestsDependencyClasspath := {
899899
val dottyLib = (classDirectory in `dotty-library-bootstrapped` in Compile).value
900900
val scalaLib =

0 commit comments

Comments
 (0)