From 9688740dff102e2a8930a97c241c901c823b25d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Rochala?= <48657087+rochala@users.noreply.github.com> Date: Tue, 26 Mar 2024 16:15:29 +0100 Subject: [PATCH] Adjust actionable diagnostics for scripts (#2815) * 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 --- .../scala/scala/build/bsp/BspClient.scala | 20 ++- .../cli/integration/BspTestDefinitions.scala | 168 ++++++++---------- .../scala/cli/integration/BspTests213.scala | 76 +++++++- 3 files changed, 169 insertions(+), 95 deletions(-) diff --git a/modules/build/src/main/scala/scala/build/bsp/BspClient.scala b/modules/build/src/main/scala/scala/build/bsp/BspClient.scala index 1302a774bd..7eb7020d58 100644 --- 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 index 70913ddc37..2817cb9951 100644 --- 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 index 2a1dafca20..3de44c3e8e 100644 --- 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) + } + } + } + } + +}