From f3cf401afd3a1fffea575a1e37dcade715cefffa Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 20 Nov 2018 19:51:21 +0100 Subject: [PATCH 1/6] Remove extra newlines in type mismatch message --- .../dotty/tools/dotc/reporting/diagnostic/messages.scala | 5 ++--- compiler/test-resources/repl/errmsgs | 7 ------- compiler/test-resources/repl/importFromObj | 1 - compiler/test-resources/type-printer/type-mismatch | 1 - 4 files changed, 2 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index fd21974a7ccc..6ac21e44e9c0 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -291,11 +291,10 @@ object messages { val kind: String = "Type Mismatch" val msg: String = { val (where, printCtx) = Formatting.disambiguateTypes(found, expected) + val whereSuffix = if (where.isEmpty) where else s"\n\n$where" val (fnd, exp) = Formatting.typeDiff(found, expected)(printCtx) s"""|found: $fnd - |required: $exp - | - |$where""".stripMargin + whyNoMatch + implicitFailure + |required: $exp""".stripMargin + whereSuffix + whyNoMatch + implicitFailure } val explanation: String = "" diff --git a/compiler/test-resources/repl/errmsgs b/compiler/test-resources/repl/errmsgs index aef05a07c103..4c122eb19a40 100644 --- a/compiler/test-resources/repl/errmsgs +++ b/compiler/test-resources/repl/errmsgs @@ -5,36 +5,30 @@ scala> val x: List[String] = List(1) | ^ | found: Int(1) | required: String - | scala> val y: List[List[String]] = List(List(1)) 1 | val y: List[List[String]] = List(List(1)) | ^ | found: Int(1) | required: String - | scala> val z: (List[String], List[Int]) = (List(1), List("a")) 1 | val z: (List[String], List[Int]) = (List(1), List("a")) | ^ | found: Int(1) | required: String - | 1 | val z: (List[String], List[Int]) = (List(1), List("a")) | ^^^ | found: String("a") | required: Int - | scala> val a: Inv[String] = new Inv(new Inv(1)) 1 | val a: Inv[String] = new Inv(new Inv(1)) | ^^^^^^^^^^ | found: Inv[Int] | required: String - | scala> val b: Inv[String] = new Inv(1) 1 | val b: Inv[String] = new Inv(1) | ^ | found: Int(1) | required: String - | 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() } }; } 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() } }; } | ^ @@ -59,7 +53,6 @@ scala> val x: List[Int] = "foo" :: List(1) | ^^^^^ | found: String("foo") | required: Int - | scala> { def f: Int = g; val x: Int = 1; def g: Int = 5; } 1 | { def f: Int = g; val x: Int = 1; def g: Int = 5; } | ^ diff --git a/compiler/test-resources/repl/importFromObj b/compiler/test-resources/repl/importFromObj index f70b24d8d1c4..b3655c3f094a 100644 --- a/compiler/test-resources/repl/importFromObj +++ b/compiler/test-resources/repl/importFromObj @@ -9,7 +9,6 @@ scala> buf += xs | ^^ | found: List[Int](o.xs) | required: Int - | scala> buf ++= xs val res0: scala.collection.mutable.ListBuffer[Int] = ListBuffer(1, 2, 3) scala> import util.foo diff --git a/compiler/test-resources/type-printer/type-mismatch b/compiler/test-resources/type-printer/type-mismatch index 4452d880a324..27c0ad9384e5 100644 --- a/compiler/test-resources/type-printer/type-mismatch +++ b/compiler/test-resources/type-printer/type-mismatch @@ -7,4 +7,3 @@ scala> val x: Foo[String] = res0 | ^^^^ | found: Foo[Int](res0) | required: Foo[String] - | From 7469d590501fd0bea9e624362e809c94de228ffb Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 21 Nov 2018 15:54:49 +0100 Subject: [PATCH 2/6] Don't display usecases in IDE/REPL It's not really useful and can be confusing. --- compiler/src/dotty/tools/dotc/util/ParsedComment.scala | 1 - .../test/dotty/tools/languageserver/HoverTest.scala | 5 ----- 2 files changed, 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/ParsedComment.scala b/compiler/src/dotty/tools/dotc/util/ParsedComment.scala index 98cff1a325d1..9d1b7e374b72 100644 --- a/compiler/src/dotty/tools/dotc/util/ParsedComment.scala +++ b/compiler/src/dotty/tools/dotc/util/ParsedComment.scala @@ -117,7 +117,6 @@ object ParsedComment { "@throws" -> TagFormatter("Throws", toDescriptionList), "@see" -> TagFormatter("See Also", toMarkdownList), "@example" -> TagFormatter("Examples", toCodeFences("scala")), - "@usecase" -> TagFormatter("Usecases", toCodeFences("scala")), "@note" -> TagFormatter("Note", toMarkdownList), "@author" -> TagFormatter("Authors", toMarkdownList), "@since" -> TagFormatter("Since", toMarkdownList), diff --git a/language-server/test/dotty/tools/languageserver/HoverTest.scala b/language-server/test/dotty/tools/languageserver/HoverTest.scala index acb003a9a872..2de9d5faafaa 100644 --- a/language-server/test/dotty/tools/languageserver/HoverTest.scala +++ b/language-server/test/dotty/tools/languageserver/HoverTest.scala @@ -153,11 +153,6 @@ class HoverTest { | myFoo.bar[Int, String](0, "hello, world") | ``` | - |**Usecases** - | - ```scala - | def bar(fizz: Int, buzz: String): Any - | ``` - | |**Note** | - A note | From a3e8e95781d5e79eb0c24cb9f9f5a5b955e173c3 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 20 Nov 2018 22:07:15 +0100 Subject: [PATCH 3/6] IDE test: add missing stripMargin for code interpolator This means that many tests were failing to compile previously, but we never noticed. FIXME: Somehow, this broke the documentation test in HoverTest (the @usecase part of the doc is missing from the output now) --- .../test/dotty/tools/languageserver/SymbolTest.scala | 4 ++-- .../test/dotty/tools/languageserver/util/Code.scala | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/language-server/test/dotty/tools/languageserver/SymbolTest.scala b/language-server/test/dotty/tools/languageserver/SymbolTest.scala index e9e6318b8e38..5bb876cd633c 100644 --- a/language-server/test/dotty/tools/languageserver/SymbolTest.scala +++ b/language-server/test/dotty/tools/languageserver/SymbolTest.scala @@ -21,11 +21,11 @@ class SymbolTest { code"class $Bar", code"""package foo |class $fooFoo { - | class Bar + | class ${m7}Bar$m8 |} """ ) .symbol("Foo", Foo.range.symInfo("Foo", SymbolKind.Class), fooFoo.range.symInfo("Foo", SymbolKind.Class, "foo")) - .symbol("Bar", Bar.range.symInfo("Bar", SymbolKind.Class)) + .symbol("Bar", Bar.range.symInfo("Bar", SymbolKind.Class), (m7 to m8).symInfo("Bar", SymbolKind.Class, "Foo")) } @Test def symbolShowModule: Unit = { diff --git a/language-server/test/dotty/tools/languageserver/util/Code.scala b/language-server/test/dotty/tools/languageserver/util/Code.scala index c5a742d21fa5..9d9bc5f7c291 100644 --- a/language-server/test/dotty/tools/languageserver/util/Code.scala +++ b/language-server/test/dotty/tools/languageserver/util/Code.scala @@ -106,7 +106,7 @@ object Code { } if (pi.hasNext) - stringBuilder.append(pi.next()) + stringBuilder.append(pi.next().stripMargin) (stringBuilder.result(), positions.result()) } From 15d9af3051e3675963b4b37facfd235ff89f8e4c Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 20 Nov 2018 19:39:36 +0100 Subject: [PATCH 4/6] IDE: add test infrastructure for diagnostics --- .../languageserver/DiagnosticsTest.scala | 18 +++++++++++ .../tools/languageserver/util/CodeRange.scala | 2 +- .../languageserver/util/CodeTester.scala | 30 ++++++++++++++++++- .../util/embedded/CodeMarker.scala | 8 +++++ 4 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala diff --git a/language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala b/language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala new file mode 100644 index 000000000000..7b96162b30fc --- /dev/null +++ b/language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala @@ -0,0 +1,18 @@ +package dotty.tools.languageserver + +import org.junit.Test + +import dotty.tools.dotc.reporting.diagnostic.ErrorMessageID._ +import dotty.tools.languageserver.util.Code._ +import org.eclipse.lsp4j.DiagnosticSeverity._ + +class DiagnosticsTest { + @Test def diagnosticWrongType: Unit = + code"""object Test { + | val x: Int = $m1"foo"$m2 + |}""".withSource + .diagnostics(m1, + (m1 to m2, """found: String("foo") + |required: Int""".stripMargin, Error, Some(TypeMismatchID)) + ) +} diff --git a/language-server/test/dotty/tools/languageserver/util/CodeRange.scala b/language-server/test/dotty/tools/languageserver/util/CodeRange.scala index 60a965955f21..1cdcd63dba8b 100644 --- a/language-server/test/dotty/tools/languageserver/util/CodeRange.scala +++ b/language-server/test/dotty/tools/languageserver/util/CodeRange.scala @@ -19,7 +19,7 @@ case class CodeRange(start: CodeMarker, end: CodeMarker) { if (!checked) { assert(start.file == end.file, s"$start and $end where not in the same file") assert(start.line <= end.line, s"Expected $end to be after $start") - assert(start.line != end.line || start.character < end.character, s"Expected $end to be after $start") + assert(start.line != end.line || start.character <= end.character, s"Expected $end to be at or after $start") checked = true } } diff --git a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala index c808ff6b8d05..28036655b51e 100644 --- a/language-server/test/dotty/tools/languageserver/util/CodeTester.scala +++ b/language-server/test/dotty/tools/languageserver/util/CodeTester.scala @@ -5,9 +5,12 @@ import dotty.tools.languageserver.util.actions._ import dotty.tools.languageserver.util.embedded.CodeMarker import dotty.tools.languageserver.util.server.{TestFile, TestServer} +import dotty.tools.dotc.reporting.diagnostic.ErrorMessageID import dotty.tools.dotc.util.Signatures.Signature -import org.eclipse.lsp4j.{CompletionItemKind, DocumentHighlightKind} +import org.eclipse.lsp4j.{ CompletionItemKind, DocumentHighlightKind, Diagnostic, DiagnosticSeverity } + +import org.junit.Assert.assertEquals /** * Simulates an LSP client for test in a project defined by `sources`. @@ -30,6 +33,31 @@ class CodeTester(projects: List[Project]) { private val positions: PositionContext = getPositions(files) + /** Check that the last diagnostics that have been published so far by the server + * for a given file match `expected`. + * + * @param marker The marker defining the source file from which to query. + * @param expected The expected diagnostics to be found + */ + def diagnostics(marker: CodeMarker, + expected: (CodeRange, String, DiagnosticSeverity, Option[ErrorMessageID])*): this.type = { + implicit val posCtx = positions + + def toDiagnostic(range: CodeRange, message: String, severity: DiagnosticSeverity, + errorCode: Option[ErrorMessageID]): Diagnostic = { + new Diagnostic(range.toRange, message, severity, /*source =*/"", + errorCode.map(_.errorNumber.toString).orNull) + } + + val expectedParams = marker.toPublishDiagnosticsParams(expected.toList.map(toDiagnostic)) + // Find the latest published diagnostics for the current source file + val actualParams = testServer.client.diagnostics.get.reverse.find(_.getUri == marker.uri) + .getOrElse(throw new Exception(s"No published diagnostics for ${marker.uri}")) + assertEquals(expectedParams, actualParams) + + this + } + /** * Perform a hover over `range`, verifies that result matches `expected`. * diff --git a/language-server/test/dotty/tools/languageserver/util/embedded/CodeMarker.scala b/language-server/test/dotty/tools/languageserver/util/embedded/CodeMarker.scala index 8c198ec12900..392c40060556 100644 --- a/language-server/test/dotty/tools/languageserver/util/embedded/CodeMarker.scala +++ b/language-server/test/dotty/tools/languageserver/util/embedded/CodeMarker.scala @@ -3,6 +3,8 @@ package dotty.tools.languageserver.util.embedded import dotty.tools.languageserver.util.server.TestFile import dotty.tools.languageserver.util.{CodeRange, PositionContext} +import scala.collection.JavaConverters._ + import org.eclipse.lsp4j._ import PositionContext.PosCtx @@ -16,6 +18,9 @@ class CodeMarker(val name: String) extends Embedded { /** The file containing this marker. */ def file: PosCtx[TestFile] = posCtx.positionOf(this)._1 + /** The uri of the file containing this marker. */ + def uri: PosCtx[String] = file.uri + /** The line containing this marker. */ def line: PosCtx[Int] = posCtx.positionOf(this)._2 @@ -34,6 +39,9 @@ class CodeMarker(val name: String) extends Embedded { def toCompletionParams: PosCtx[CompletionParams] = new CompletionParams(toTextDocumentIdentifier, toPosition) + def toPublishDiagnosticsParams(diagnostics: List[Diagnostic]): PosCtx[PublishDiagnosticsParams] = + new PublishDiagnosticsParams(file.uri, diagnostics.asJava) + def toRenameParams(newName: String): PosCtx[RenameParams] = new RenameParams(toTextDocumentIdentifier, toPosition, newName) From 38c6d462da32462aea18df0be7b3d6be75c40da6 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 20 Nov 2018 18:27:24 +0100 Subject: [PATCH 5/6] Report the correct errors in the IDE In some situations (like when interpolating type variables) the logic changes depending on whether we have unreported errors or not. Before this commit, we assumed that any error in a StoreReporter was unreported, but this is wrong: the IDE, the REPL and other tools use a StoreReporter as their root reporter. This commit fixes this by making sure all the code that looks for unreported errors does so by calling `hasUnreportedErrors` and making that method return false when there is no outer reporter. --- compiler/src/dotty/tools/dotc/reporting/Reporter.scala | 6 ++++-- .../src/dotty/tools/dotc/reporting/StoreReporter.scala | 4 ++-- compiler/src/dotty/tools/dotc/typer/Inferencing.scala | 5 +---- compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala | 2 +- .../dotty/tools/languageserver/DiagnosticsTest.scala | 10 ++++++++++ 5 files changed, 18 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala index aa6aebe17146..ad4a073e46fb 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala @@ -238,8 +238,10 @@ abstract class Reporter extends interfaces.ReporterResult { def isHidden(m: MessageContainer)(implicit ctx: Context): Boolean = ctx.mode.is(Mode.Printing) - /** Does this reporter contain not yet reported errors or warnings? */ - def hasPendingErrors: Boolean = false + /** Does this reporter contain errors that have yet to be reported by its outer reporter ? + * Note: this is always false when there is no outer reporter. + */ + def hasUnreportedErrors: Boolean = false /** If this reporter buffers messages, remove and return all buffered messages. */ def removeBufferedMessages(implicit ctx: Context): List[MessageContainer] = Nil diff --git a/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala b/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala index e642e294fcf4..7707dfd9c5e1 100644 --- a/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala @@ -28,8 +28,8 @@ class StoreReporter(outer: Reporter) extends Reporter { infos += m } - override def hasPendingErrors: Boolean = - infos != null && infos.exists(_.isInstanceOf[Error]) + override def hasUnreportedErrors: Boolean = + outer != null && infos != null && infos.exists(_.isInstanceOf[Error]) override def removeBufferedMessages(implicit ctx: Context): List[MessageContainer] = if (infos != null) try infos.toList finally infos = null diff --git a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala index eae27f16df0c..befc06112068 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala @@ -432,10 +432,7 @@ trait Inferencing { this: Typer => // found : Int(1) // required: String // val y: List[List[String]] = List(List(1)) - val hasUnreportedErrors = state.reporter match { - case r: StoreReporter if r.hasErrors => true - case _ => false - } + val hasUnreportedErrors = state.reporter.hasUnreportedErrors def constraint = state.constraint for (tvar <- qualifying) if (!tvar.isInstantiated && state.constraint.contains(tvar)) { diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index 91960ed68850..8e97fce05348 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -264,7 +264,7 @@ object ProtoTypes { targ = arg.withType(WildcardType) else { targ = typerFn(arg) - if (!ctx.reporter.hasPendingErrors) { + if (!ctx.reporter.hasUnreportedErrors) { // FIXME: This can swallow warnings by updating the typerstate from a nested // context that gets discarded later. But we do have to update the // typerstate if there are no errors. If we also omitted the next two lines diff --git a/language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala b/language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala index 7b96162b30fc..93f933e9f937 100644 --- a/language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala +++ b/language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala @@ -15,4 +15,14 @@ class DiagnosticsTest { (m1 to m2, """found: String("foo") |required: Int""".stripMargin, Error, Some(TypeMismatchID)) ) + + @Test def diagnosticMissingLambdaBody: Unit = + code"""object Test { + | Nil.map(x => x).filter(x$m1 =>$m2) + |$m3}""".withSource + .diagnostics(m1, + (m2 to m3, "illegal start of simple expression", Error, Some(IllegalStartSimpleExprID)), + (m1 to m1, """found: Null + |required: Boolean""".stripMargin, Error, Some(TypeMismatchID)) + ) } From d180287105e88ac1a1596e6b0b57c0d1b5073221 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 21 Nov 2018 17:03:38 +0100 Subject: [PATCH 6/6] Add diagnostics test for "pure expression" warning Also do not reuse the compiler options used in the Dotty build for the tests, since they can influence their result (e.g. -Xfatal-warnings) --- .../tools/languageserver/DiagnosticsTest.scala | 13 +++++++++++++ project/Build.scala | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala b/language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala index 93f933e9f937..541042c598b8 100644 --- a/language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala +++ b/language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala @@ -25,4 +25,17 @@ class DiagnosticsTest { (m1 to m1, """found: Null |required: Boolean""".stripMargin, Error, Some(TypeMismatchID)) ) + + @Test def diagnosticPureExpression: Unit = + code"""object Test { + | ${m1}1$m2 + |}""".withSource + .diagnostics(m1, + (m1 to m2, + "a pure expression does nothing in statement position; you may be omitting necessary parentheses", + Warning, Some(PureExpressionInStatementPositionID))) + + @Test def diagnosticWorksheetPureExpression: Unit = + ws"""${m1}1""".withSource + .diagnostics(m1 /* no "pure expression" warning because this is a worksheet */) } diff --git a/project/Build.scala b/project/Build.scala index de79d4d8a79b..bc68566826bd 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -894,7 +894,7 @@ object Build { ). settings( ideTestsCompilerVersion := (version in `dotty-compiler`).value, - ideTestsCompilerArguments := (scalacOptions in `dotty-compiler`).value, + ideTestsCompilerArguments := Seq(), ideTestsDependencyClasspath := { val dottyLib = (classDirectory in `dotty-library-bootstrapped` in Compile).value val scalaLib =