Skip to content

Commit

Permalink
Adjust actionable diagnostics for scripts (#2815)
Browse files Browse the repository at this point in the history
* Adjust actionable diagnostics for scripts

* Add test for 2.13, and make them run accordingly

* Change test format, remove old test

* Revert changes made by metalsOrganizeImports
  • Loading branch information
rochala committed Mar 26, 2024
1 parent bbb9a71 commit 9688740
Show file tree
Hide file tree
Showing 3 changed files with 169 additions and 95 deletions.
20 changes: 19 additions & 1 deletion modules/build/src/main/scala/scala/build/bsp/BspClient.scala
Expand Up @@ -2,7 +2,7 @@ package scala.build.bsp

import ch.epfl.scala.bsp4j.{ScalaAction, ScalaDiagnostic, ScalaTextEdit, ScalaWorkspaceEdit}
import ch.epfl.scala.bsp4j as b
import com.google.gson.Gson
import com.google.gson.{Gson, JsonElement}

import java.lang.Boolean as JBoolean
import java.net.URI
Expand Down Expand Up @@ -48,6 +48,24 @@ class BspClient(
diag0.getRange.getStart.setLine(startLine)
diag0.getRange.getEnd.setLine(endLine)

val scalaDiagnostic = new Gson().fromJson[b.ScalaDiagnostic](
diag0.getData().asInstanceOf[JsonElement],
classOf[b.ScalaDiagnostic]
)

scalaDiagnostic.getActions().asScala.foreach { action =>
for {
change <- action.getEdit().getChanges().asScala
startLine <- updateLine(change.getRange.getStart.getLine)
endLine <- updateLine(change.getRange.getEnd.getLine)
} yield {
change.getRange().getStart.setLine(startLine)
change.getRange().getEnd.setLine(endLine)
}
}

diag0.setData(scalaDiagnostic)

if (
diag0.getMessage.contains(
"cannot be a main method since it cannot be accessed statically"
Expand Down
Expand Up @@ -1589,7 +1589,6 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
val visibleDiagnostics =
localClient.diagnostics().takeWhile(!_.getReset).flatMap(_.getDiagnostics.asScala)

expect(visibleDiagnostics.nonEmpty)
expect(visibleDiagnostics.length == 1)

val updateActionableDiagnostic = visibleDiagnostics.head
Expand Down Expand Up @@ -1625,6 +1624,77 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
}
}
}

if (actualScalaVersion.startsWith("3."))
List(".sc", ".scala").foreach { filetype =>
test(s"bsp should report actionable diagnostic from bloop for $filetype files (Scala 3)") {
val fileName = s"Hello$filetype"
val inputs = TestInputs(
os.rel / fileName ->
s"""
|object Hello {
| sealed trait TestTrait
| case class TestA() extends TestTrait
| case class TestB() extends TestTrait
| val traitInstance: TestTrait = ???
| traitInstance match {
| case TestA() =>
| }
|}
|""".stripMargin
)
withBsp(inputs, Seq(".")) {
(_, localClient, remoteServer) =>
async {
// prepare build
val buildTargetsResp = await(remoteServer.workspaceBuildTargets().asScala)
// build code
val targets = buildTargetsResp.getTargets.asScala.map(_.getId()).asJava
await(remoteServer.buildTargetCompile(new b.CompileParams(targets)).asScala)

val visibleDiagnostics =
localClient.diagnostics().map(_.getDiagnostics().asScala).find(
!_.isEmpty
).getOrElse(
Nil
)

expect(visibleDiagnostics.size == 1)

val updateActionableDiagnostic = visibleDiagnostics.head

checkDiagnostic(
diagnostic = updateActionableDiagnostic,
expectedMessage = "match may not be exhaustive.",
expectedSeverity = b.DiagnosticSeverity.WARNING,
expectedStartLine = 6,
expectedStartCharacter = 2,
expectedEndLine = 6,
expectedEndCharacter = 15,
expectedSource = Some("bloop"),
strictlyCheckMessage = false
)

val scalaDiagnostic = new Gson().fromJson[b.ScalaDiagnostic](
updateActionableDiagnostic.getData().asInstanceOf[JsonElement],
classOf[b.ScalaDiagnostic]
)

val actions = scalaDiagnostic.getActions().asScala.toList
assert(actions.size == 1)
val changes = actions.head.getEdit().getChanges().asScala.toList
assert(changes.size == 1)
val textEdit = changes.head

expect(textEdit.getNewText().contains("\n case TestB() => ???"))
expect(textEdit.getRange().getStart.getLine == 7)
expect(textEdit.getRange().getStart.getCharacter == 19)
expect(textEdit.getRange().getEnd.getLine == 7)
expect(textEdit.getRange().getEnd.getCharacter == 19)
}
}
}
}
test("bsp should support jvmRunEnvironment request") {
val inputs = TestInputs(
os.rel / "Hello.scala" ->
Expand Down Expand Up @@ -1818,95 +1888,6 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
)
}

if (actualScalaVersion.startsWith("3."))
test("actionable diagnostics from compiler") {
val inputs = TestInputs(
os.rel / "test.sc" ->
"""//> using scala 3.3.2-RC1-bin-20230723-5afe621-NIGHTLY
|def foo(): Int = 23
|
|def test: Int = foo
|// ^^^ error: missing parentheses
|""".stripMargin
)

withBsp(inputs, Seq(".")) { (root, localClient, remoteServer) =>
async {
val buildTargetsResp = await(remoteServer.workspaceBuildTargets().asScala)
val target = {
val targets = buildTargetsResp.getTargets.asScala.map(_.getId).toSeq
expect(targets.length == 2)
extractMainTargets(targets)
}

val targetUri = TestUtil.normalizeUri(target.getUri)
checkTargetUri(root, targetUri)

val targets = List(target).asJava

val compileResp = await {
remoteServer
.buildTargetCompile(new b.CompileParams(targets))
.asScala
}
expect(compileResp.getStatusCode == b.StatusCode.ERROR)

val diagnosticsParams = {
val diagnostics = localClient.diagnostics()
val params = diagnostics(2)
expect(params.getBuildTarget.getUri == targetUri)
expect(
TestUtil.normalizeUri(params.getTextDocument.getUri) ==
TestUtil.normalizeUri((root / "test.sc").toNIO.toUri.toASCIIString)
)
params
}

val diagnostics = diagnosticsParams.getDiagnostics.asScala
expect(diagnostics.size == 1)

val theDiagnostic = diagnostics.head

checkDiagnostic(
diagnostic = theDiagnostic,
expectedMessage =
"method foo in class test$_ must be called with () argument",
expectedSeverity = b.DiagnosticSeverity.ERROR,
expectedStartLine = 3,
expectedStartCharacter = 16,
expectedEndLine = 3,
expectedEndCharacter = 19
)

// Shouldn't dataKind be set to "scala"? expect(theDiagnostic.getDataKind == "scala")

val gson = new com.google.gson.Gson()

val scalaDiagnostic: b.ScalaDiagnostic = gson.fromJson(
theDiagnostic.getData.toString,
classOf[b.ScalaDiagnostic]
)

val actions = scalaDiagnostic.getActions.asScala
expect(actions.size == 1)

val action = actions.head
expect(action.getTitle == "Insert ()")

val edit = action.getEdit
expect(edit.getChanges.asScala.size == 1)
val change = edit.getChanges.asScala.head

val expectedRange = new b.Range(
new b.Position(9, 19),
new b.Position(9, 19)
)
expect(change.getRange == expectedRange)
expect(change.getNewText == "()")
}
}
}

if (!actualScalaVersion.startsWith("2.12"))
test("actionable diagnostics on deprecated using directives") {
val inputs = TestInputs(
Expand Down Expand Up @@ -2074,7 +2055,7 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
params
}

private def checkDiagnostic(
protected def checkDiagnostic(
diagnostic: b.Diagnostic,
expectedMessage: String,
expectedSeverity: b.DiagnosticSeverity,
Expand All @@ -2090,10 +2071,11 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
expect(diagnostic.getRange.getStart.getCharacter == expectedStartCharacter)
expect(diagnostic.getRange.getEnd.getLine == expectedEndLine)
expect(diagnostic.getRange.getEnd.getCharacter == expectedEndCharacter)
val message = TestUtil.removeAnsiColors(diagnostic.getMessage)
if (strictlyCheckMessage)
assertNoDiff(diagnostic.getMessage, expectedMessage)
assertNoDiff(message, expectedMessage)
else
expect(diagnostic.getMessage.contains(expectedMessage))
expect(message.contains(expectedMessage))
for (es <- expectedSource)
expect(diagnostic.getSource == es)
}
Expand Down
@@ -1,3 +1,77 @@
package scala.cli.integration

class BspTests213 extends BspTestDefinitions with Test213
import ch.epfl.scala.bsp4j as b
import com.eed3si9n.expecty.Expecty.expect
import com.google.gson.{Gson, JsonElement}

import scala.async.Async.{async, await}
import scala.concurrent.ExecutionContext.Implicits.global
import scala.jdk.CollectionConverters.*

class BspTests213 extends BspTestDefinitions with Test213 {

List(".sc", ".scala").foreach { filetype =>
test(s"bsp should report actionable diagnostic from bloop for $filetype files (Scala 2.13)") {
val fileName = s"Hello$filetype"
val inputs = TestInputs(
os.rel / fileName ->
s"""
|object Hello {
| def foo: Any = {
| x: Int => x * 2
| }
|}
|""".stripMargin
)
withBsp(inputs, Seq(".", "-O", "-Xsource:3")) {
(_, localClient, remoteServer) =>
async {
// prepare build
val buildTargetsResp = await(remoteServer.workspaceBuildTargets().asScala)
// build code
val targets = buildTargetsResp.getTargets.asScala.map(_.getId()).asJava
await(remoteServer.buildTargetCompile(new b.CompileParams(targets)).asScala)

val visibleDiagnostics =
localClient.diagnostics().map(_.getDiagnostics().asScala).find(!_.isEmpty).getOrElse(
Nil
)

expect(visibleDiagnostics.size == 1)

val updateActionableDiagnostic = visibleDiagnostics.head

checkDiagnostic(
diagnostic = updateActionableDiagnostic,
expectedMessage = "parentheses are required around the parameter of a lambda",
expectedSeverity = b.DiagnosticSeverity.ERROR,
expectedStartLine = 3,
expectedStartCharacter = 5,
expectedEndLine = 3,
expectedEndCharacter = 5,
expectedSource = Some("bloop"),
strictlyCheckMessage = false
)

val scalaDiagnostic = new Gson().fromJson[b.ScalaDiagnostic](
updateActionableDiagnostic.getData().asInstanceOf[JsonElement],
classOf[b.ScalaDiagnostic]
)

val actions = scalaDiagnostic.getActions().asScala.toList
assert(actions.size == 1)
val changes = actions.head.getEdit().getChanges().asScala.toList
assert(changes.size == 1)
val textEdit = changes.head

expect(textEdit.getNewText().contains("(x: Int)"))
expect(textEdit.getRange().getStart.getLine == 3)
expect(textEdit.getRange().getStart.getCharacter == 4)
expect(textEdit.getRange().getEnd.getLine == 3)
expect(textEdit.getRange().getEnd.getCharacter == 10)
}
}
}
}

}

0 comments on commit 9688740

Please sign in to comment.