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.

Sign up for GitHub

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

Jump to bottom

Adjust actionable diagnostics for scripts #2815

Merged
merged 4 commits into from Mar 26, 2024
Merged

Adjust actionable diagnostics for scripts #2815

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b98e72
Adjust actionable diagnostics for scripts
rochala Mar 25, 2024
6141d6d
Add test for 2.13, and make them run accordingly
rochala Mar 26, 2024
3b5a5f4
Change test format, remove old test
rochala Mar 26, 2024
bb0cde7
Revert changes made by metalsOrganizeImports
rochala Mar 26, 2024
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
View file
Open in desktop
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
158 changes: 155 additions & 3 deletions modules/integration/src/test/scala/scala/cli/integration/BspTestDefinitions.scala
View file
Open in desktop
Expand Up @@ -21,6 +21,10 @@ import scala.concurrent.{Await, Future, Promise}
import scala.jdk.CollectionConverters.*
import scala.util.control.NonFatal
import scala.util.{Failure, Properties, Success, Try}
import scala.cli.integration.TestUtil.IgnoreScalaVersion
import scala.cli.integration.TestUtil.IgnoreScala3
import scala.cli.integration.TestUtil.IgnoreScala212
import scala.cli.integration.TestUtil.IgnoreScala2

abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArgs {
_: TestScalaVersion =>
Expand Down Expand Up @@ -61,6 +65,18 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
pool.shutdown()
}

override def munitTests(): Seq[Test] = {
val default = super.munitTests()
val scalaVersion = scalaVersionOpt.getOrElse(Constants.scala3Next)

default.filterNot {
_.tags.exists {
case IgnoreScalaVersion(isIgnored) => isIgnored(scalaVersion)
case _ => false
}
}
}

private def extractMainTargets(targets: Seq[BuildTargetIdentifier]): BuildTargetIdentifier =
targets.collectFirst {
case t if !t.getUri.contains("-test") => t
Expand Down Expand Up @@ -1589,7 +1605,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 +1640,142 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
}
}
}

List(".sc", ".scala").foreach { filetype =>
test(s"bsp should report actionable diagnostic from bloop for $filetype files (Scala 2.13)".tag(
IgnoreScala3 and IgnoreScala212
)) {
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)
}
}
}
}

List(".sc", ".scala").foreach { filetype =>
test(s"bsp should report actionable diagnostic from bloop for $filetype files (Scala 3)".tag(
IgnoreScala2
)) {
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 @@ -2090,10 +2241,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
12 changes: 12 additions & 0 deletions modules/integration/src/test/scala/scala/cli/integration/TestUtil.scala
View file
Open in desktop
Expand Up @@ -12,6 +12,8 @@ import scala.concurrent.duration.{Duration, DurationInt, FiniteDuration}
import scala.concurrent.{Await, ExecutionContext, Future}
import scala.util.Properties

import munit.Tag

object TestUtil {

val cliKind: String = sys.props("test.scala-cli.kind")
Expand All @@ -25,6 +27,16 @@ object TestUtil {
val cli: Seq[String] = cliCommand(cliPath)
val ltsEqualsNext: Boolean = Constants.scala3Lts equals Constants.scala3Next

case class IgnoreScalaVersion(ignored: String => Boolean) extends Tag("NoScalaVersion") {
def and(other: IgnoreScalaVersion): IgnoreScalaVersion =
IgnoreScalaVersion(v => ignored(v) || other.ignored(v))
}

object IgnoreScala2 extends IgnoreScalaVersion(_.startsWith("2."))
object IgnoreScala3 extends IgnoreScalaVersion(_.startsWith("3."))
object IgnoreScala212 extends IgnoreScalaVersion(_.startsWith("2.12"))
object IgnoreScala213 extends IgnoreScalaVersion(_.startsWith("2.13"))

def cliCommand(cliPath: String): Seq[String] =
if (isNativeCli)
Seq(cliPath)
Expand Down