Adjust actionable diagnostics for scripts (#2815)

rochala · web-flow · commit 9688740dff10 · 2024-03-26T15:15:29.000Z
* 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
diff --git a/modules/build/src/main/scala/scala/build/bsp/BspClient.scala b/modules/build/src/main/scala/scala/build/bsp/BspClient.scala
@@ -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
@@ -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"
diff --git a/modules/integration/src/test/scala/scala/cli/integration/BspTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/BspTestDefinitions.scala
@@ -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
@@ -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" ->
@@ -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(
@@ -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,
@@ -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)
   }
diff --git a/modules/integration/src/test/scala/scala/cli/integration/BspTests213.scala b/modules/integration/src/test/scala/scala/cli/integration/BspTests213.scala
@@ -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)
+          }
+      }
+    }
+  }
+
+}
