Skip to content

Commit

Permalink
Add support for actionable diagnotics from scala-cli
Browse files Browse the repository at this point in the history
  • Loading branch information
lwronski committed Oct 18, 2022
1 parent fef3498 commit ba4118d
Show file tree
Hide file tree
Showing 8 changed files with 174 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -261,12 +261,14 @@ final class Diagnostics(
snapshot: Input,
): Option[Diagnostic] = {
val result = edit.toRevised(d.getRange).map { range =>
new l.Diagnostic(
val ld = new l.Diagnostic(
range,
d.getMessage,
d.getSeverity,
d.getSource,
)
ld.setData(d.getData)
ld
}
if (result.isEmpty) {
d.getRange.toMeta(snapshot).foreach { pos =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,9 @@ object MetalsEnrichments
val severity = d.getSeverity.toString.toLowerCase()
s"$severity:$hint $uri:${d.getRange.getStart.getLine} ${d.getMessage}"
}
def asTextEdit: Option[l.TextEdit] = {
decodeJson(d.getData, classOf[l.TextEdit])
}
}

implicit class XtensionSeverityBsp(sev: b.DiagnosticSeverity) {
Expand Down Expand Up @@ -752,15 +755,18 @@ object MetalsEnrichments
}

implicit class XtensionDiagnosticBsp(diag: b.Diagnostic) {
def toLsp: l.Diagnostic =
new l.Diagnostic(
def toLsp: l.Diagnostic = {
val ld = new l.Diagnostic(
diag.getRange.toLsp,
fansi.Str(diag.getMessage, ErrorMode.Strip).plainText,
diag.getSeverity.toLsp,
if (diag.getSource == null) "scalac" else diag.getSource,
// We omit diag.getCode since Bloop's BSP implementation uses 'code' with different semantics
// than LSP. See https://github.com/scalacenter/bloop/issues/1134 for details
)
ld.setData(diag.getData)
ld
}
}

implicit class XtensionHttpExchange(exchange: HttpServerExchange) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
package scala.meta.internal.metals

import scala.meta.internal.metals.MetalsEnrichments._

import org.eclipse.{lsp4j => l}

object ScalacDiagnostic {

object ScalaAction {
def unapply(d: l.Diagnostic): Option[l.TextEdit] = d.asTextEdit
}

object NotAMember {
private val regex = """(?s)value (.+) is not a member of.*""".r
def unapply(d: l.Diagnostic): Option[String] =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package scala.meta.internal.metals.codeactions

import scala.concurrent.ExecutionContext
import scala.concurrent.Future

import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.metals._
import scala.meta.pc.CancelToken

import org.eclipse.{lsp4j => l}

class ActionableDiagnostic() extends CodeAction {
override def kind: String = l.CodeActionKind.QuickFix

override def contribute(
params: l.CodeActionParams,
token: CancelToken,
)(implicit ec: ExecutionContext): Future[Seq[l.CodeAction]] = {

def path =
params.getTextDocument.getUri.toAbsolutePath.toURI.toString

def createDiagnosticAction(
diagnostic: l.Diagnostic,
textEdit: l.TextEdit,
): l.CodeAction = {
val codeAction = new l.CodeAction()
val diagMessage = diagnostic.getMessage
codeAction.setKind(l.CodeActionKind.QuickFix)
codeAction.setTitle(
s"Apply suggestion: ${diagMessage.split(System.lineSeparator()).headOption.getOrElse(diagMessage)}"
)
codeAction.setDiagnostics(List(diagnostic).asJava)
codeAction.setEdit(
new l.WorkspaceEdit(
Map(path -> List(textEdit).asJava).asJava
)
)
codeAction
}

val codeActions = params
.getContext()
.getDiagnostics()
.asScala
.groupBy {
case ScalacDiagnostic.ScalaAction(textEdit) =>
Some(textEdit)
case _ => None
}
.collect {
case (Some(textEdit), diags)
if params.getRange().overlapsWith(diags.head.getRange()) =>
createDiagnosticAction(diags.head, textEdit)
}
.toList
.sorted

Future.successful(codeActions)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ final class CodeActionProvider(
new ImplementAbstractMembers(compilers),
new ImportMissingSymbol(compilers, buildTargets),
new CreateNewSymbol(),
new ActionableDiagnostic(),
new StringActions(buffers),
extractMemberAction,
new SourceOrganizeImports(
Expand Down
55 changes: 30 additions & 25 deletions tests/unit/src/main/scala/tests/BaseScalaCliSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -101,35 +101,12 @@ abstract class BaseScalaCliSuite(scalaVersion: String)
|}
|
|""".stripMargin

private def escape(s: String): String =
s.replace("\\", "\\\\")
private def bspLayout =
s"""/.bsp/scala-cli.json
|{
| "name": "scala-cli",
| "argv": [
| "${escape(ScalaCli.javaCommand)}",
| "-cp",
| "${escape(ScalaCli.scalaCliClassPath().mkString(File.pathSeparator))}",
| "${ScalaCli.scalaCliMainClass}",
| "bsp",
| "."
| ],
| "version": "${BuildInfo.scalaCliVersion}",
| "bspVersion": "2.0.0",
| "languages": [
| "scala",
| "java"
| ]
|}
|${BaseScalaCliSuite.scalaCliBspJsonContent()}
|
|/.scala-build/ide-inputs.json
|{
| "args": [
| "."
| ]
|}
|${BaseScalaCliSuite.scalaCliIdeInputJson(".")}
|
|""".stripMargin

Expand Down Expand Up @@ -354,3 +331,31 @@ abstract class BaseScalaCliSuite(scalaVersion: String)
} yield ()
}
}

object BaseScalaCliSuite {
def scalaCliBspJsonContent(args: List[String] = Nil): String = {
val argv = List(
ScalaCli.javaCommand,
"-cp",
ScalaCli.scalaCliClassPath().mkString(File.pathSeparator),
ScalaCli.scalaCliMainClass,
"bsp",
".",
) ++ args
val bsjJson = ujson.Obj(
"name" -> "scala-cli",
"argv" -> argv,
"version" -> BuildInfo.scalaCliVersion,
"bspVersion" -> "2.0.0",
"languages" -> List("scala", "java"),
)
ujson.write(bsjJson)
}

def scalaCliIdeInputJson(args: String*): String = {
val ideInputJson = ujson.Obj(
"args" -> args
)
ujson.write(ideInputJson)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import munit.Location
import munit.TestOptions
import org.eclipse.lsp4j.CodeAction
import tests.BaseLspSuite
import tests.BaseScalaCliSuite
import tests.FileLayout

abstract class BaseCodeActionLspSuite(
Expand Down Expand Up @@ -48,6 +49,7 @@ abstract class BaseCodeActionLspSuite(
kind: List[String] = Nil,
scalafixConf: String = "",
scalacOptions: List[String] = Nil,
scalaCliOptions: List[String] = Nil,
configuration: => Option[String] = None,
scalaVersion: String = scalaVersion,
renamePath: Option[String] = None,
Expand All @@ -56,19 +58,30 @@ abstract class BaseCodeActionLspSuite(
changeFile: String => String = identity,
expectError: Boolean = false,
filterAction: CodeAction => Boolean = _ => true,
scalaCliLayout: Boolean = false,
assertion: Option[Unit => Unit] = None,
)(implicit loc: Location): Unit = {
val scalacOptionsJson =
if (scalacOptions.nonEmpty)
s""""scalacOptions": ["${scalacOptions.mkString("\",\"")}"],"""
else ""
val path = s"a/src/main/scala/a/$fileName"

val layout =
s"""/metals.json
|{"a":{$scalacOptionsJson "scalaVersion" : "$scalaVersion"}}
|$scalafixConf
|/$path
|$input""".stripMargin
val layout = {
if (scalaCliLayout)
s"""/.bsp/scala-cli.json
|${BaseScalaCliSuite.scalaCliBspJsonContent(scalaCliOptions)}
|/.scala-build/ide-inputs.json
|${BaseScalaCliSuite.scalaCliIdeInputJson(".")}
|/$path
|$input""".stripMargin
else
s"""/metals.json
|{"a":{$scalacOptionsJson "scalaVersion" : "$scalaVersion"}}
|$scalafixConf
|/$path
|$input""".stripMargin
}

checkEdit(
name,
Expand All @@ -84,6 +97,7 @@ abstract class BaseCodeActionLspSuite(
changeFile,
expectError,
filterAction,
assertion,
)
}

Expand All @@ -101,6 +115,7 @@ abstract class BaseCodeActionLspSuite(
changeFile: String => String = identity,
expectError: Boolean = false,
filterAction: CodeAction => Boolean = _ => true,
assertion: Option[Unit => Unit] = None,
)(implicit loc: Location): Unit = {
val files = FileLayout.mapFromString(layout)
val (path, input) = files
Expand Down Expand Up @@ -147,7 +162,11 @@ abstract class BaseCodeActionLspSuite(
else
server.bufferContents(newPath)
}
_ = assertNoDiff(server.bufferContents(newPath), actualExpectedCode)
_ = assertion match {
case Some(assert) => assert()
case None =>
assertNoDiff(server.bufferContents(newPath), actualExpectedCode)
}
_ = if (expectNoDiagnostics) assertNoDiagnostics() else ()
_ = extraOperations
} yield ()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package tests.codeactions

import coursier.version.Version

class ActionableDiagnosticsSuite
extends BaseCodeActionLspSuite("actionableDiagnostic") {

val oldOsLibVersion: Version = Version("0.7.8")

check(
"actionable-diagnostic-update",
s"""|//> <<>>using lib "com.lihaoyi::os-lib:${oldOsLibVersion.repr}"
|
|object Hello extends App {
| println("Hello")
|}
|""".stripMargin,
"Apply suggestion: com.lihaoyi::os-lib:0.7.8 is outdated",
expectedCode = "",
scalaCliOptions = List("--actions", "-S", scalaVersion),
expectNoDiagnostics = false,
scalaCliLayout = true,
assertion = Some { _ =>
val sourceContent = server.bufferContents("a/src/main/scala/a/A.scala")
val newOsLibVersion = raw"\d+\.\d+\.\d+".r
.findFirstIn(sourceContent)
.map(Version(_))
assert(
newOsLibVersion.nonEmpty,
"os-lib dependency should have been updated",
)
assert(
oldOsLibVersion < newOsLibVersion.get,
s"os-lib dependency should have been updated to newer version than ${oldOsLibVersion.repr}",
)
},
)

}

0 comments on commit ba4118d

Please sign in to comment.