Update bsp scala diagnostics, don't send bad actionable diagnostics

metals/src/main/scala/scala/meta/internal/metals/Diagnostics.scala

@@ -283,7 +283,7 @@ final class Diagnostics(
                 .isLeft() && d.getCode().getLeft() != "-1"
             )
               ld.setCode(d.getCode())
-            ld.setData(adjustDiagnosticData(d, edit))
+            adjustedDiagnosticData(d, edit).map(newData => ld.setData(newData))
             ld
           }
         if (result.isEmpty) {
@@ -313,22 +313,28 @@ final class Diagnostics(
     toPublish
   }
 
-  private def adjustDiagnosticData(
+  private def adjustedDiagnosticData(
       diagnostic: l.Diagnostic,
       edit: TokenEditDistance,
-  ): Object =
+  ): Option[Object] =
     diagnostic match {
       case ScalacDiagnostic.ScalaDiagnostic(Left(textEdit)) =>
-        val range = textEdit.getRange
-        val revisedRange = edit
+        edit
           .toRevised(
-            range = range,
-            adjustWithinToken = shouldAdjustWithinToken(diagnostic),
+            textEdit,
+            shouldAdjustWithinToken(diagnostic),
+            fallbackToNearest = false,
           )
-        textEdit.setRange(revisedRange.getOrElse(range))
-        textEdit.toJsonObject
-      // TODO also update bsp scala diagnostics, e.g. ScalaDiagnostic(Right(textEdits))
-      case diagnostic => diagnostic.getData
+          .map(_.toJsonObject)
+      case ScalacDiagnostic.ScalaDiagnostic(Right(scalaDiagnostic)) =>
+        edit
+          .toRevised(
+            scalaDiagnostic,
+            shouldAdjustWithinToken(diagnostic),
+            fallbackToNearest = false,
+          )
+          .map(_.toJsonObject)
+      case _ => Some(diagnostic.getData())
     }
 
   private def shouldAdjustWithinToken(diagnostic: l.Diagnostic): Boolean =

metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala

@@ -748,6 +748,11 @@ object MetalsEnrichments
       new l.Position(pos.getLine, pos.getCharacter)
   }
 
+  implicit class XtensionLspPositionBsp(pos: l.Position) {
+    def toBsp: b.Position =
+      new b.Position(pos.getLine, pos.getCharacter)
+  }
+
   implicit class XtensionPositionRange(range: s.Range) {
     def inString(text: String): Option[String] = {
       var i = 0
@@ -826,6 +831,31 @@ object MetalsEnrichments
     }
   }
 
+  implicit class XtensionLspRangeBsp(range: l.Range) {
+    def toBsp: b.Range =
+      new b.Range(range.getStart.toBsp, range.getEnd.toBsp)
+  }
+
+  implicit class XtensionScalaAction(scalaAction: b.ScalaAction) {
+    def asLspTextEdits: Seq[l.TextEdit] =
+      scalaAction
+        .getEdit()
+        .getChanges()
+        .asScala
+        .toSeq
+        .map(edit => new l.TextEdit(edit.getRange().toLsp, edit.getNewText()))
+
+    def setEditFromLspTextEdits(lspTextEdits: Seq[l.TextEdit]): Unit = {
+      val scalaWorkspaceEdit =
+        new b.ScalaWorkspaceEdit(
+          lspTextEdits.map { edit =>
+            new b.ScalaTextEdit(edit.getRange().toBsp, edit.getNewText())
+          }.asJava
+        )
+      scalaAction.setEdit(scalaWorkspaceEdit)
+    }
+  }
+
   implicit class XtensionHttpExchange(exchange: HttpServerExchange) {
     def getQuery(key: String): Option[String] =
       Option(exchange.getQueryParameters.get(key)).flatMap(_.asScala.headOption)

metals/src/main/scala/scala/meta/internal/metals/codeactions/ActionableDiagnostic.scala

@@ -36,14 +36,7 @@ class ActionableDiagnostic() extends CodeAction {
           )
         case Right(scalaAction) =>
           val uri = params.getTextDocument().getUri()
-          val edits = scalaAction
-            .getEdit()
-            .getChanges()
-            .asScala
-            .toSeq
-            .map(edit =>
-              new l.TextEdit(edit.getRange().toLsp, edit.getNewText())
-            )
+          val edits = scalaAction.asLspTextEdits
           CodeActionBuilder.build(
             title = scalaAction.getTitle(),
             kind = l.CodeActionKind.QuickFix,

metals/src/main/scala/scala/meta/internal/parsing/TokenEditDistance.scala

@@ -15,6 +15,7 @@ import scala.meta.internal.parsing.TokenOps.syntax._
 import scala.meta.internal.{semanticdb => s}
 import scala.meta.tokenizers.Tokenized
 
+import ch.epfl.scala.{bsp4j => b}
 import difflib._
 import org.eclipse.{lsp4j => l}
 
@@ -29,7 +30,8 @@ sealed trait TokenEditDistance {
    * This method behaves differently from the other `toRevised` in a few ways:
    * - it should only return `None` in the case when the sources don't tokenize.
    *   When the original token is removed in the revised document, we find instead the
-   *   nearest token in the original document instead.
+   *   nearest token in the original document instead.  This behaviour can be toggled
+   *   off by providing value of false for `fallbackToNearest`.
    *
    * If `adjustWithinToken` is true and the original and revised range are both contained within
    * a single (matching) token, then adjust the returned range to match the start/end offsets of
@@ -38,6 +40,7 @@ sealed trait TokenEditDistance {
   def toRevised(
       range: l.Range,
       adjustWithinToken: Boolean = false,
+      fallbackToNearest: Boolean = true,
   ): Option[l.Range]
   def toRevised(originalOffset: Int): Either[EmptyResult, Position]
   def toRevised(
@@ -99,6 +102,73 @@ sealed trait TokenEditDistance {
     }
   }
 
+  final def toRevised(
+      textEdit: l.TextEdit,
+      adjustWithinToken: Boolean,
+      fallbackToNearest: Boolean,
+  ): Option[l.TextEdit] = {
+    val revisedRange = toRevised(
+      range = textEdit.getRange(),
+      adjustWithinToken = adjustWithinToken,
+      fallbackToNearest = fallbackToNearest,
+    )
+    revisedRange.map(r => new l.TextEdit(r, textEdit.getNewText()))
+  }
+
+  final def toRevised(
+      scalaAction: b.ScalaAction,
+      adjustWithinToken: Boolean,
+      fallbackToNearest: Boolean,
+  ): Option[b.ScalaAction] = {
+    val existingEdits = scalaAction.asLspTextEdits
+    existingEdits
+      .map { textEdit =>
+        toRevised(
+          textEdit = textEdit,
+          adjustWithinToken = adjustWithinToken,
+          fallbackToNearest = fallbackToNearest,
+        )
+      }
+      .foldLeft[Option[Seq[l.TextEdit]]](Some(Seq.empty)) {
+        case (Some(seq), Some(e)) => Some(seq :+ e)
+        case _ => None
+      }
+      .map { newEdits =>
+        val newAction = new b.ScalaAction(scalaAction.getTitle())
+        newAction.setEditFromLspTextEdits(newEdits)
+        newAction
+      }
+  }
+
+  /**
+   * Converts a bsp diagnostic in the original document to the revised document.
+   * `None` will be returned if conversion of any of the diagnostic's underlying
+   * text edits cannot be revised.
+   */
+  final def toRevised(
+      scalaDiagnostic: b.ScalaDiagnostic,
+      adjustWithinToken: Boolean,
+      fallbackToNearest: Boolean,
+  ): Option[b.ScalaDiagnostic] = {
+    val actions = scalaDiagnostic.getActions()
+    val newActions = actions.asScala
+      .map { action =>
+        toRevised(
+          action,
+          adjustWithinToken = adjustWithinToken,
+          fallbackToNearest = fallbackToNearest,
+        )
+      }
+      .foldLeft[Option[Seq[b.ScalaAction]]](Some(Seq.empty)) {
+        case (Some(seq), Some(e)) => Some(seq :+ e)
+        case _ => None
+      }
+    newActions.map { scalaActions =>
+      val newDiagnostic = new b.ScalaDiagnostic()
+      newDiagnostic.setActions(scalaActions.asJava)
+      newDiagnostic
+    }
+  }
 }
 
 object TokenEditDistance {
@@ -107,6 +177,7 @@ object TokenEditDistance {
     def toRevised(
         range: l.Range,
         adjustWithinToken: Boolean = false,
+        fallbackToNearest: Boolean = true,
     ): Option[l.Range] = Some(range)
     def toRevised(originalOffset: Int): Either[EmptyResult, Position] =
       EmptyResult.unchanged
@@ -132,6 +203,7 @@ object TokenEditDistance {
     def toRevised(
         range: l.Range,
         adjustWithinToken: Boolean = false,
+        fallbackToNearest: Boolean = true,
     ): Option[l.Range] = None
     def toRevised(originalOffset: Int): Either[EmptyResult, Position] =
       EmptyResult.noMatch
@@ -164,6 +236,7 @@ object TokenEditDistance {
     def toRevised(
         range: l.Range,
         adjustWithinToken: Boolean = false,
+        fallbackToNearest: Boolean = true,
     ): Option[l.Range] = {
       range.toMeta(originalInput).flatMap { pos =>
         val matchingTokens = matching.lift
@@ -230,23 +303,27 @@ object TokenEditDistance {
 
         (startMatch, endMatch) match {
           case (Some(start), Some(end)) =>
-            val revised =
-              if (startFallback && endFallback) {
-                val offset = end.revised.start
-                Position.Range(revisedInput, offset - 1, offset)
-              } else if (start.revised == end.revised) {
-                // new range spans one token
-                if (adjustWithinToken)
-                  computeAdjustmentWithinToken(range, start)
-                else start.revised.pos
-              } else {
-                val endOffset = end.revised match {
-                  case t if t.isLF => t.start
-                  case t => t.end
-                }
-                Position.Range(revisedInput, start.revised.start, endOffset)
+            if ((startFallback || endFallback) && !fallbackToNearest)
+              None
+            else if (startFallback && endFallback) {
+              val offset = end.revised.start
+              Some(Position.Range(revisedInput, offset - 1, offset).toLsp)
+            } else if (start.revised == end.revised) {
+              // new range spans one token
+              if (adjustWithinToken)
+                Some(computeAdjustmentWithinToken(range, start).toLsp)
+              else Some(start.revised.pos.toLsp)
+            } else {
+              val endOffset = end.revised match {
+                case t if t.isLF => t.start
+                case t => t.end
               }
-            Some(revised.toLsp)
+              Some(
+                Position
+                  .Range(revisedInput, start.revised.start, endOffset)
+                  .toLsp
+              )
+            }
           case (start, end) =>
             logger.warning(
               s"stale range: ${start.map(_.show)} ${end.map(_.show)}"

tests/slow/src/main/scala/tests/scalacli/BaseScalaCLIActionSuite.scala

@@ -26,14 +26,12 @@ class BaseScalaCLIActionSuite(name: String)
       changeFile: String => String = identity,
       expectError: Boolean = false,
       filterAction: CodeAction => Boolean = _ => true,
-  ): Unit = {
-    val fileContent = input.replace("<<", "").replace(">>", "")
-
+  ): Unit =
     checkScalaCLI(
       name = name,
       input = input,
       expectedActions = "",
-      expectedCode = fileContent,
+      expectedCode = changeFile(input).replace("<<", "").replace(">>", ""),
       expectNoDiagnostics = expectNoDiagnostics,
       kind = kind,
       scalaCliOptions = scalaCliOptions,
@@ -48,7 +46,6 @@ class BaseScalaCLIActionSuite(name: String)
       expectError = expectError,
       filterAction = filterAction,
     )
-  }
 
   def checkScalaCLI(
       name: TestOptions,

tests/slow/src/test/scala/tests/scalacli/ScalaCliActionsSuite.scala

@@ -48,7 +48,9 @@ class ScalaCliActionsSuite
         |  println("Hello")
         |}
         |""".stripMargin,
-    s"""Apply suggestion: "os-lib is outdated, update to $newestOsLib"""",
+    s"""|"os-lib is outdated, update to ${newestOsLib}"
+        |     os-lib 0.7.8 -> com.lihaoyi::os-lib:${newestOsLib}
+        |""".stripMargin,
     s"""|// commentary
         |//> using lib "com.lihaoyi::os-lib:$newestOsLib"
         |
@@ -66,6 +68,24 @@ class ScalaCliActionsSuite
     expectNoDiagnostics = false,
   )
 
+  checkNoActionScalaCLI(
+    "actionable-diagnostic-didchange-stale-action-not-returned",
+    s"""|//> using lib "<<>>com.lihaoyi::os-lib:${oldOsLibVersion.repr}"
+        |
+        |object Hello extends App {
+        |  println("Hello")
+        |}
+        |""".stripMargin,
+    changeFile = f =>
+      f.replace(
+        s"""//> using lib "<<>>com.lihaoyi::os-lib:${oldOsLibVersion.repr}""",
+        s"""|//> using lib "<<>>com.lihaoyi::os-
+            |//lib:${oldOsLibVersion.repr}""".stripMargin,
+      ).stripMargin,
+    scalaCliOptions = List("--actions", "-S", scalaVersion),
+    expectNoDiagnostics = false,
+  )
+
   checkNoActionScalaCLI(
     "actionable-diagnostic-out-of-range",
     s"""|//> <<>>using lib "com.lihaoyi::os-lib:${oldOsLibVersion.repr}"