From f989032591370a62b280bf66fe5ecc09e8d94ebb Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Fri, 12 Dec 2025 16:20:36 +0100 Subject: [PATCH] improvement: Allow passing -explain to the presentation compiler --- .../dotty/tools/pc/DiagnosticProvider.scala | 7 ++- .../tools/pc/base/BaseDiagnosticsSuite.scala | 39 +++++++++++++ .../pc/tests/DiagnosticProviderSuite.scala | 44 +++----------- .../ExplainDiagnosticProviderSuite.scala | 57 +++++++++++++++++++ 4 files changed, 109 insertions(+), 38 deletions(-) create mode 100644 presentation-compiler/test/dotty/tools/pc/base/BaseDiagnosticsSuite.scala create mode 100644 presentation-compiler/test/dotty/tools/pc/tests/ExplainDiagnosticProviderSuite.scala diff --git a/presentation-compiler/src/main/dotty/tools/pc/DiagnosticProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/DiagnosticProvider.scala index 878dcf72d89b..1d84f872a122 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,9 +24,13 @@ class DiagnosticProvider(driver: InteractiveDriver, params: VirtualFileParams): private def toLsp(diag: Diagnostic)(using Context): Option[lsp4j.Diagnostic] = Option.when(diag.pos.exists): + val message = + if Diagnostic.shouldExplain(diag) then + diag.msg.message + "\n\n# Explanation (enabled by `-explain`)\n\n" + diag.msg.explanation + else diag.msg.message val lspDiag = lsp4j.Diagnostic( diag.pos.toLsp, - diag.msg.message, + message, toDiagnosticSeverity(diag.level), "presentation compiler", ) 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..74ecffe6800c --- /dev/null +++ b/presentation-compiler/test/dotty/tools/pc/base/BaseDiagnosticsSuite.scala @@ -0,0 +1,39 @@ +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..8e060a82f17f 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/DiagnosticProviderSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/DiagnosticProviderSuite.scala @@ -1,43 +1,15 @@ 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( @@ -45,7 +17,7 @@ class DiagnosticProviderSuite extends PcAssertions { | Int.maaxValue |""".stripMargin, List(TestDiagnostic(12,25, "value maaxValue is not a member of object Int - did you mean Int.MaxValue?", DiagnosticSeverity.Error)) - ) + ) @Test def warning = check( 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..fcc52526591a --- /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 + + ) + ) + ) + +}