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

Scala cli actionable diagnostic #4297

Merged
merged 1 commit into from
Oct 19, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,55 @@
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 createActionableDiagnostic(
diagnostic: l.Diagnostic,
textEdit: l.TextEdit,
): l.CodeAction = {
val diagMessage = diagnostic.getMessage
val uri = params.getTextDocument().getUri()

CodeActionBuilder.build(
title =
s"Apply suggestion: ${diagMessage.linesIterator.headOption.getOrElse(diagMessage)}",
kind = l.CodeActionKind.QuickFix,
changes = List(uri.toAbsolutePath -> Seq(textEdit)),
diagnostics = List(diagnostic),
)
}

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()) =>
createActionableDiagnostic(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,29 @@ abstract class BaseCodeActionLspSuite(
changeFile: String => String = identity,
expectError: Boolean = false,
filterAction: CodeAction => Boolean = _ => true,
scalaCliLayout: Boolean = false,
)(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 Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package tests.codeactions

import scala.meta.internal.mtags.CoursierComplete

import coursier.version.Version

class ActionableDiagnosticsSuite
extends BaseCodeActionLspSuite("actionableDiagnostic") {

val oldOsLibVersion: Version = Version("0.7.8")
val newestOsLib: String = CoursierComplete
.complete("com.lihaoyi::os-lib:")
.headOption
.getOrElse("0.8.1")

check(
"actionable-diagnostic-update",
s"""|//> <<>>using lib "com.lihaoyi::os-lib:${oldOsLibVersion.repr}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the actual diagnostic tied to this look like?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Trace - 02:46:40 PM] Received notification 'build/publishDiagnostics'
Params: {
  "textDocument": {
    "uri": "file:///private/tmp/scala-demo/Hello.scala"
  },
  "buildTarget": {
    "uri": "file:/private/tmp/scala-demo/.scala-build/?id\u003dproject_bd2c96d2de"
  },
  "diagnostics": [
    {
      "range": {
        "start": {
          "line": 0,
          "character": 15
        },
        "end": {
          "line": 0,
          "character": 40
        }
      },
      "severity": 4,
      "source": "scala-cli",
      "message": "com.lihaoyi::os-lib:0.7.8 is outdated, update to 0.8.1\n     com.lihaoyi::os-lib:0.7.8 -\u003e com.lihaoyi::os-lib:0.8.1",
      "data": {
        "range": {
          "start": {
            "line": 0,
            "character": 15
          },
          "end": {
            "line": 0,
            "character": 40
          }
        },
        "newText": "com.lihaoyi::os-lib:0.8.1"
      }
    }
  ],
  "reset": false
}

|
|object Hello extends App {
| println("Hello")
|}
|""".stripMargin,
s"Apply suggestion: com.lihaoyi::os-lib:0.7.8 is outdated, update to $newestOsLib",
s"""|//> using lib "com.lihaoyi::os-lib:$newestOsLib"
|
|object Hello extends App {
| println("Hello")
|}
|""".stripMargin,
scalaCliOptions = List("--actions", "-S", scalaVersion),
expectNoDiagnostics = false,
scalaCliLayout = true,
)

}