Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust actionable diagnostics for scripts #2815

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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)
}
}
}
}

}