Skip to content

Commit 8f9fddf

Browse files
tgodziktgodzik
authored
improvement: Allow passing -explain to the presentation compiler (#24740)
Needed for a code action show full explanation
1 parent 7eba9a7 commit 8f9fddf

File tree

4 files changed

+190
-49
lines changed

4 files changed

+190
-49
lines changed

‎presentation-compiler/src/main/dotty/tools/pc/DiagnosticProvider.scala‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import scala.jdk.CollectionConverters.*
1515
import scala.meta.pc.VirtualFileParams
1616

1717
class DiagnosticProvider(driver: InteractiveDriver, params: VirtualFileParams):
18-
1918
def diagnostics(): List[lsp4j.Diagnostic] =
2019
if params.shouldReturnDiagnostics then
2120
val diags = driver.run(params.uri().nn, params.text().nn)
@@ -25,11 +24,16 @@ class DiagnosticProvider(driver: InteractiveDriver, params: VirtualFileParams):
2524

2625
private def toLsp(diag: Diagnostic)(using Context): Option[lsp4j.Diagnostic] =
2726
Option.when(diag.pos.exists):
27+
val explanation =
28+
if Diagnostic.shouldExplain(diag)
29+
then
30+
"\n\n# Explanation (enabled by `-explain`)\n\n" + diag.msg.explanation
31+
else ""
2832
val lspDiag = lsp4j.Diagnostic(
2933
diag.pos.toLsp,
30-
diag.msg.message,
34+
diag.msg.message + explanation,
3135
toDiagnosticSeverity(diag.level),
32-
"presentation compiler",
36+
"presentation compiler"
3337
)
3438
lspDiag.setCode(diag.msg.errorId.errorNumber)
3539

@@ -45,7 +49,8 @@ class DiagnosticProvider(driver: InteractiveDriver, params: VirtualFileParams):
4549
val lspAction = lsp4j.CodeAction(action.title)
4650
lspAction.setKind(lsp4j.CodeActionKind.QuickFix)
4751
lspAction.setIsPreferred(true)
48-
val edits = action.patches.groupBy(_.srcPos.source.path)
52+
val edits = action.patches
53+
.groupBy(_.srcPos.source.path)
4954
.map((path, actions) => path -> (actions.map(toLspTextEdit).asJava))
5055
.asJava
5156

@@ -54,8 +59,12 @@ class DiagnosticProvider(driver: InteractiveDriver, params: VirtualFileParams):
5459
lspAction
5560

5661
private def toLspTextEdit(actionPatch: ActionPatch): lsp4j.TextEdit =
57-
val startPos = lsp4j.Position(actionPatch.srcPos.startLine, actionPatch.srcPos.startColumn)
58-
val endPos = lsp4j.Position(actionPatch.srcPos.endLine, actionPatch.srcPos.endColumn)
62+
val startPos = lsp4j.Position(
63+
actionPatch.srcPos.startLine,
64+
actionPatch.srcPos.startColumn
65+
)
66+
val endPos =
67+
lsp4j.Position(actionPatch.srcPos.endLine, actionPatch.srcPos.endColumn)
5968
val range = lsp4j.Range(startPos, endPos)
6069
lsp4j.TextEdit(range, actionPatch.replacement)
6170

‎presentation-compiler/test/dotty/tools/pc/base/BaseDiagnosticsSuite.scala‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package dotty.tools.pc.base
2+
3+
import dotty.tools.pc.RawScalaPresentationCompiler
4+
import dotty.tools.pc.base.TestResources
5+
import dotty.tools.pc.utils.PcAssertions
6+
import dotty.tools.pc.utils.TestExtensions.getOffset
7+
import org.eclipse.lsp4j.Diagnostic
8+
import org.eclipse.lsp4j.DiagnosticSeverity
9+
10+
import java.net.URI
11+
import scala.meta.internal.jdk.CollectionConverters.*
12+
import scala.meta.internal.metals.EmptyCancelToken
13+
import scala.meta.pc.CancelToken
14+
import scala.meta.pc.VirtualFileParams
15+
16+
class BaseDiagnosticsSuite extends PcAssertions:
17+
case class TestDiagnostic(
18+
startIndex: Int,
19+
endIndex: Int,
20+
msg: String,
21+
severity: DiagnosticSeverity
22+
)
23+
24+
def options: List[String] = Nil
25+
26+
val pc = RawScalaPresentationCompiler().newInstance(
27+
"",
28+
TestResources.classpath.asJava,
29+
options.asJava
30+
)
31+
32+
case class TestVirtualFileParams(uri: URI, text: String)
33+
extends VirtualFileParams {
34+
override def shouldReturnDiagnostics: Boolean = true
35+
override def token: CancelToken = EmptyCancelToken
36+
}
37+
38+
def check(
39+
text: String,
40+
expected: List[TestDiagnostic],
41+
additionalChecks: List[Diagnostic] => Unit = identity
42+
): Unit =
43+
val diagnostics = pc
44+
.didChange(
45+
TestVirtualFileParams(URI.create("file:/Diagnostic.scala"), text)
46+
)
47+
.asScala
48+
49+
val actual = diagnostics.map(d =>
50+
TestDiagnostic(
51+
d.getRange().getStart().getOffset(text),
52+
d.getRange().getEnd().getOffset(text),
53+
d.getMessage(),
54+
d.getSeverity()
55+
)
56+
)
57+
assertEquals(
58+
expected,
59+
actual,
60+
s"Expected [${expected.mkString(", ")}] but got [${actual.mkString(", ")}]"
61+
)
62+
additionalChecks(diagnostics.toList)

‎presentation-compiler/test/dotty/tools/pc/tests/DiagnosticProviderSuite.scala‎

Lines changed: 56 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,44 @@
11
package dotty.tools.pc.tests
22

3-
import java.net.URI
4-
import scala.meta.internal.jdk.CollectionConverters.*
5-
import scala.meta.internal.metals.EmptyCancelToken
6-
import scala.meta.pc.VirtualFileParams
7-
import scala.meta.pc.CancelToken
8-
9-
import org.junit.Test
10-
import org.eclipse.lsp4j.DiagnosticSeverity
11-
import dotty.tools.pc.utils.TestExtensions.getOffset
12-
import dotty.tools.pc.base.TestResources
13-
import java.nio.file.Path
14-
import dotty.tools.pc.RawScalaPresentationCompiler
15-
import dotty.tools.pc.utils.PcAssertions
16-
import org.eclipse.lsp4j.Diagnostic
3+
import dotty.tools.pc.base.BaseDiagnosticsSuite
174
import org.eclipse.lsp4j.CodeAction
5+
import org.eclipse.lsp4j.Diagnostic
6+
import org.eclipse.lsp4j.DiagnosticSeverity
7+
import org.junit.Test
188

19-
class DiagnosticProviderSuite extends PcAssertions {
20-
case class TestDiagnostic(startIndex: Int, endIndex: Int, msg: String, severity: DiagnosticSeverity)
21-
22-
val pc = RawScalaPresentationCompiler().newInstance("", TestResources.classpath.asJava, Nil.asJava)
23-
24-
case class TestVirtualFileParams(uri: URI, text: String) extends VirtualFileParams {
25-
override def shouldReturnDiagnostics: Boolean = true
26-
override def token: CancelToken = EmptyCancelToken
27-
}
28-
29-
def check(
30-
text: String,
31-
expected: List[TestDiagnostic],
32-
additionalChecks: List[Diagnostic] => Unit = identity
33-
): Unit =
34-
val diagnostics = pc
35-
.didChange(TestVirtualFileParams(URI.create("file:/Diagnostic.scala"), text))
36-
.asScala
9+
import java.net.URI
10+
import scala.meta.internal.jdk.CollectionConverters.*
3711

38-
val actual = diagnostics.map(d => TestDiagnostic(d.getRange().getStart().getOffset(text), d.getRange().getEnd().getOffset(text), d.getMessage(), d.getSeverity()))
39-
assertEquals(expected, actual, s"Expected [${expected.mkString(", ")}] but got [${actual.mkString(", ")}]")
40-
additionalChecks(diagnostics.toList)
12+
class DiagnosticProviderSuite extends BaseDiagnosticsSuite {
4113

4214
@Test def error =
4315
check(
4416
"""|object M:
4517
| Int.maaxValue
4618
|""".stripMargin,
47-
List(TestDiagnostic(12,25, "value maaxValue is not a member of object Int - did you mean Int.MaxValue?", DiagnosticSeverity.Error))
19+
List(
20+
TestDiagnostic(
21+
12,
22+
25,
23+
"value maaxValue is not a member of object Int - did you mean Int.MaxValue?",
24+
DiagnosticSeverity.Error
25+
)
26+
)
4827
)
4928

5029
@Test def warning =
5130
check(
5231
"""|object M:
5332
| 1 + 1
5433
|""".stripMargin,
55-
List(TestDiagnostic(12, 17, "A pure expression does nothing in statement position", DiagnosticSeverity.Warning))
34+
List(
35+
TestDiagnostic(
36+
12,
37+
17,
38+
"A pure expression does nothing in statement position",
39+
DiagnosticSeverity.Warning
40+
)
41+
)
5642
)
5743

5844
@Test def mixed =
@@ -62,8 +48,18 @@ class DiagnosticProviderSuite extends PcAssertions {
6248
| 1 + 1
6349
|""".stripMargin,
6450
List(
65-
TestDiagnostic(12,25, "value maaxValue is not a member of object Int - did you mean Int.MaxValue?", DiagnosticSeverity.Error),
66-
TestDiagnostic(28, 33, "A pure expression does nothing in statement position", DiagnosticSeverity.Warning)
51+
TestDiagnostic(
52+
12,
53+
25,
54+
"value maaxValue is not a member of object Int - did you mean Int.MaxValue?",
55+
DiagnosticSeverity.Error
56+
),
57+
TestDiagnostic(
58+
28,
59+
33,
60+
"A pure expression does nothing in statement position",
61+
DiagnosticSeverity.Warning
62+
)
6763
)
6864
)
6965

@@ -73,12 +69,29 @@ class DiagnosticProviderSuite extends PcAssertions {
7369
| private private class Test
7470
|""".stripMargin,
7571
List(
76-
TestDiagnostic(20, 27, "Repeated modifier private", DiagnosticSeverity.Error),
72+
TestDiagnostic(
73+
20,
74+
27,
75+
"Repeated modifier private",
76+
DiagnosticSeverity.Error
77+
)
7778
),
7879
diags =>
79-
val action = diags.head.getData().asInstanceOf[java.util.List[CodeAction]].asScala.head
80-
assertWithDiff("Remove repeated modifier: \"private\"", action.getTitle(), false)
81-
assertEquals(1, action.getEdit().getChanges().size(), "There should be one change")
80+
val action = diags.head
81+
.getData()
82+
.asInstanceOf[java.util.List[CodeAction]]
83+
.asScala
84+
.head
85+
assertWithDiff(
86+
"Remove repeated modifier: \"private\"",
87+
action.getTitle(),
88+
false
89+
)
90+
assertEquals(
91+
1,
92+
action.getEdit().getChanges().size(),
93+
"There should be one change"
94+
)
8295
)
8396

8497
}

‎presentation-compiler/test/dotty/tools/pc/tests/ExplainDiagnosticProviderSuite.scala‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package dotty.tools.pc.tests
2+
3+
import dotty.tools.pc.base.BaseDiagnosticsSuite
4+
import org.eclipse.lsp4j.CodeAction
5+
import org.eclipse.lsp4j.Diagnostic
6+
import org.eclipse.lsp4j.DiagnosticSeverity
7+
import org.junit.Test
8+
9+
import java.net.URI
10+
import scala.meta.internal.jdk.CollectionConverters.*
11+
12+
class ExplainDiagnosticProviderSuite extends BaseDiagnosticsSuite {
13+
14+
override def options: List[String] = List("-explain")
15+
16+
@Test def error1 =
17+
check(
18+
"""|object C:
19+
| def m(x: Int) = 1
20+
| object T extends K:
21+
| val x = m(1) // error
22+
|class K:
23+
| def m(i: Int) = 2
24+
|""".stripMargin,
25+
List(
26+
TestDiagnostic(
27+
64,
28+
65,
29+
"""|Reference to m is ambiguous.
30+
|It is both defined in object C
31+
|and inherited subsequently in object T
32+
|
33+
|# Explanation (enabled by `-explain`)
34+
|
35+
|The identifier m is ambiguous because a name binding of lower precedence
36+
|in an inner scope cannot shadow a binding with higher precedence in
37+
|an outer scope.
38+
|
39+
|The precedence of the different kinds of name bindings, from highest to lowest, is:
40+
| - Definitions in an enclosing scope
41+
| - Inherited definitions and top-level definitions in packages
42+
| - Names introduced by import of a specific name
43+
| - Names introduced by wildcard import
44+
| - Definitions from packages in other files
45+
|Note:
46+
| - As a rule, definitions take precedence over imports.
47+
| - Definitions in an enclosing scope take precedence over inherited definitions,
48+
| which can result in ambiguities in nested classes.
49+
| - When importing, you can avoid naming conflicts by renaming:
50+
| import scala.{m => mTick}
51+
|""".stripMargin,
52+
DiagnosticSeverity.Error
53+
)
54+
)
55+
)
56+
57+
}

0 commit comments

Comments
 (0)