diff --git a/presentation-compiler/src/main/dotty/tools/pc/DiagnosticProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/DiagnosticProvider.scala index 878dcf72d89b..a77b28ef6c08 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/DiagnosticProvider.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/DiagnosticProvider.scala @@ -15,7 +15,6 @@ import scala.jdk.CollectionConverters.* import scala.meta.pc.VirtualFileParams class DiagnosticProvider(driver: InteractiveDriver, params: VirtualFileParams): - def diagnostics(): List[lsp4j.Diagnostic] = if params.shouldReturnDiagnostics then val diags = driver.run(params.uri().nn, params.text().nn) @@ -25,11 +24,16 @@ class DiagnosticProvider(driver: InteractiveDriver, params: VirtualFileParams): private def toLsp(diag: Diagnostic)(using Context): Option[lsp4j.Diagnostic] = Option.when(diag.pos.exists): + val explanation = + if Diagnostic.shouldExplain(diag) + then + "\n\n# Explanation (enabled by `-explain`)\n\n" + diag.msg.explanation + else "" val lspDiag = lsp4j.Diagnostic( diag.pos.toLsp, - diag.msg.message, + diag.msg.message + explanation, toDiagnosticSeverity(diag.level), - "presentation compiler", + "presentation compiler" ) lspDiag.setCode(diag.msg.errorId.errorNumber) @@ -45,7 +49,8 @@ class DiagnosticProvider(driver: InteractiveDriver, params: VirtualFileParams): val lspAction = lsp4j.CodeAction(action.title) lspAction.setKind(lsp4j.CodeActionKind.QuickFix) lspAction.setIsPreferred(true) - val edits = action.patches.groupBy(_.srcPos.source.path) + val edits = action.patches + .groupBy(_.srcPos.source.path) .map((path, actions) => path -> (actions.map(toLspTextEdit).asJava)) .asJava @@ -54,8 +59,12 @@ class DiagnosticProvider(driver: InteractiveDriver, params: VirtualFileParams): lspAction private def toLspTextEdit(actionPatch: ActionPatch): lsp4j.TextEdit = - val startPos = lsp4j.Position(actionPatch.srcPos.startLine, actionPatch.srcPos.startColumn) - val endPos = lsp4j.Position(actionPatch.srcPos.endLine, actionPatch.srcPos.endColumn) + val startPos = lsp4j.Position( + actionPatch.srcPos.startLine, + actionPatch.srcPos.startColumn + ) + val endPos = + lsp4j.Position(actionPatch.srcPos.endLine, actionPatch.srcPos.endColumn) val range = lsp4j.Range(startPos, endPos) lsp4j.TextEdit(range, actionPatch.replacement) diff --git a/presentation-compiler/test/dotty/tools/pc/base/BaseDiagnosticsSuite.scala b/presentation-compiler/test/dotty/tools/pc/base/BaseDiagnosticsSuite.scala new file mode 100644 index 000000000000..d6ec62447f70 --- /dev/null +++ b/presentation-compiler/test/dotty/tools/pc/base/BaseDiagnosticsSuite.scala @@ -0,0 +1,62 @@ +package dotty.tools.pc.base + +import dotty.tools.pc.RawScalaPresentationCompiler +import dotty.tools.pc.base.TestResources +import dotty.tools.pc.utils.PcAssertions +import dotty.tools.pc.utils.TestExtensions.getOffset +import org.eclipse.lsp4j.Diagnostic +import org.eclipse.lsp4j.DiagnosticSeverity + +import java.net.URI +import scala.meta.internal.jdk.CollectionConverters.* +import scala.meta.internal.metals.EmptyCancelToken +import scala.meta.pc.CancelToken +import scala.meta.pc.VirtualFileParams + +class BaseDiagnosticsSuite extends PcAssertions: + case class TestDiagnostic( + startIndex: Int, + endIndex: Int, + msg: String, + severity: DiagnosticSeverity + ) + + def options: List[String] = Nil + + val pc = RawScalaPresentationCompiler().newInstance( + "", + TestResources.classpath.asJava, + options.asJava + ) + + case class TestVirtualFileParams(uri: URI, text: String) + extends VirtualFileParams { + override def shouldReturnDiagnostics: Boolean = true + override def token: CancelToken = EmptyCancelToken + } + + def check( + text: String, + expected: List[TestDiagnostic], + additionalChecks: List[Diagnostic] => Unit = identity + ): Unit = + val diagnostics = pc + .didChange( + TestVirtualFileParams(URI.create("file:/Diagnostic.scala"), text) + ) + .asScala + + val actual = diagnostics.map(d => + TestDiagnostic( + d.getRange().getStart().getOffset(text), + d.getRange().getEnd().getOffset(text), + d.getMessage(), + d.getSeverity() + ) + ) + assertEquals( + expected, + actual, + s"Expected [${expected.mkString(", ")}] but got [${actual.mkString(", ")}]" + ) + additionalChecks(diagnostics.toList) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/DiagnosticProviderSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/DiagnosticProviderSuite.scala index 96b525931f50..8a59a0ce335d 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/DiagnosticProviderSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/DiagnosticProviderSuite.scala @@ -1,50 +1,29 @@ package dotty.tools.pc.tests -import java.net.URI -import scala.meta.internal.jdk.CollectionConverters.* -import scala.meta.internal.metals.EmptyCancelToken -import scala.meta.pc.VirtualFileParams -import scala.meta.pc.CancelToken - -import org.junit.Test -import org.eclipse.lsp4j.DiagnosticSeverity -import dotty.tools.pc.utils.TestExtensions.getOffset -import dotty.tools.pc.base.TestResources -import java.nio.file.Path -import dotty.tools.pc.RawScalaPresentationCompiler -import dotty.tools.pc.utils.PcAssertions -import org.eclipse.lsp4j.Diagnostic +import dotty.tools.pc.base.BaseDiagnosticsSuite import org.eclipse.lsp4j.CodeAction +import org.eclipse.lsp4j.Diagnostic +import org.eclipse.lsp4j.DiagnosticSeverity +import org.junit.Test -class DiagnosticProviderSuite extends PcAssertions { - case class TestDiagnostic(startIndex: Int, endIndex: Int, msg: String, severity: DiagnosticSeverity) - - val pc = RawScalaPresentationCompiler().newInstance("", TestResources.classpath.asJava, Nil.asJava) - - case class TestVirtualFileParams(uri: URI, text: String) extends VirtualFileParams { - override def shouldReturnDiagnostics: Boolean = true - override def token: CancelToken = EmptyCancelToken - } - - def check( - text: String, - expected: List[TestDiagnostic], - additionalChecks: List[Diagnostic] => Unit = identity - ): Unit = - val diagnostics = pc - .didChange(TestVirtualFileParams(URI.create("file:/Diagnostic.scala"), text)) - .asScala +import java.net.URI +import scala.meta.internal.jdk.CollectionConverters.* - val actual = diagnostics.map(d => TestDiagnostic(d.getRange().getStart().getOffset(text), d.getRange().getEnd().getOffset(text), d.getMessage(), d.getSeverity())) - assertEquals(expected, actual, s"Expected [${expected.mkString(", ")}] but got [${actual.mkString(", ")}]") - additionalChecks(diagnostics.toList) +class DiagnosticProviderSuite extends BaseDiagnosticsSuite { @Test def error = check( """|object M: | Int.maaxValue |""".stripMargin, - List(TestDiagnostic(12,25, "value maaxValue is not a member of object Int - did you mean Int.MaxValue?", DiagnosticSeverity.Error)) + List( + TestDiagnostic( + 12, + 25, + "value maaxValue is not a member of object Int - did you mean Int.MaxValue?", + DiagnosticSeverity.Error + ) + ) ) @Test def warning = @@ -52,7 +31,14 @@ class DiagnosticProviderSuite extends PcAssertions { """|object M: | 1 + 1 |""".stripMargin, - List(TestDiagnostic(12, 17, "A pure expression does nothing in statement position", DiagnosticSeverity.Warning)) + List( + TestDiagnostic( + 12, + 17, + "A pure expression does nothing in statement position", + DiagnosticSeverity.Warning + ) + ) ) @Test def mixed = @@ -62,8 +48,18 @@ class DiagnosticProviderSuite extends PcAssertions { | 1 + 1 |""".stripMargin, List( - TestDiagnostic(12,25, "value maaxValue is not a member of object Int - did you mean Int.MaxValue?", DiagnosticSeverity.Error), - TestDiagnostic(28, 33, "A pure expression does nothing in statement position", DiagnosticSeverity.Warning) + TestDiagnostic( + 12, + 25, + "value maaxValue is not a member of object Int - did you mean Int.MaxValue?", + DiagnosticSeverity.Error + ), + TestDiagnostic( + 28, + 33, + "A pure expression does nothing in statement position", + DiagnosticSeverity.Warning + ) ) ) @@ -73,12 +69,29 @@ class DiagnosticProviderSuite extends PcAssertions { | private private class Test |""".stripMargin, List( - TestDiagnostic(20, 27, "Repeated modifier private", DiagnosticSeverity.Error), + TestDiagnostic( + 20, + 27, + "Repeated modifier private", + DiagnosticSeverity.Error + ) ), diags => - val action = diags.head.getData().asInstanceOf[java.util.List[CodeAction]].asScala.head - assertWithDiff("Remove repeated modifier: \"private\"", action.getTitle(), false) - assertEquals(1, action.getEdit().getChanges().size(), "There should be one change") + val action = diags.head + .getData() + .asInstanceOf[java.util.List[CodeAction]] + .asScala + .head + assertWithDiff( + "Remove repeated modifier: \"private\"", + action.getTitle(), + false + ) + assertEquals( + 1, + action.getEdit().getChanges().size(), + "There should be one change" + ) ) } diff --git a/presentation-compiler/test/dotty/tools/pc/tests/ExplainDiagnosticProviderSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/ExplainDiagnosticProviderSuite.scala new file mode 100644 index 000000000000..34e044a8e2d8 --- /dev/null +++ b/presentation-compiler/test/dotty/tools/pc/tests/ExplainDiagnosticProviderSuite.scala @@ -0,0 +1,57 @@ +package dotty.tools.pc.tests + +import dotty.tools.pc.base.BaseDiagnosticsSuite +import org.eclipse.lsp4j.CodeAction +import org.eclipse.lsp4j.Diagnostic +import org.eclipse.lsp4j.DiagnosticSeverity +import org.junit.Test + +import java.net.URI +import scala.meta.internal.jdk.CollectionConverters.* + +class ExplainDiagnosticProviderSuite extends BaseDiagnosticsSuite { + + override def options: List[String] = List("-explain") + + @Test def error1 = + check( + """|object C: + | def m(x: Int) = 1 + | object T extends K: + | val x = m(1) // error + |class K: + | def m(i: Int) = 2 + |""".stripMargin, + List( + TestDiagnostic( + 64, + 65, + """|Reference to m is ambiguous. + |It is both defined in object C + |and inherited subsequently in object T + | + |# Explanation (enabled by `-explain`) + | + |The identifier m is ambiguous because a name binding of lower precedence + |in an inner scope cannot shadow a binding with higher precedence in + |an outer scope. + | + |The precedence of the different kinds of name bindings, from highest to lowest, is: + | - Definitions in an enclosing scope + | - Inherited definitions and top-level definitions in packages + | - Names introduced by import of a specific name + | - Names introduced by wildcard import + | - Definitions from packages in other files + |Note: + | - As a rule, definitions take precedence over imports. + | - Definitions in an enclosing scope take precedence over inherited definitions, + | which can result in ambiguities in nested classes. + | - When importing, you can avoid naming conflicts by renaming: + | import scala.{m => mTick} + |""".stripMargin, + DiagnosticSeverity.Error + ) + ) + ) + +}