improvement: Use codeAction/resolve for code actions

I changed one code action for now, but probably it would be good to change others also.
scalameta · Nov 27, 2024 · ee1e0f8 · ee1e0f8
1 parent aed121d
commit ee1e0f8
Show file tree

Hide file tree

Showing 13 changed files with 151 additions and 92 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala
@@ -1400,4 +1400,13 @@ object MetalsEnrichments
       .map(_.reverse.flatten)
   }
 
+  val getOptDisplayableMessage: PartialFunction[Throwable, String] = {
+    case e: m.pc.DisplayableException => e.getMessage()
+    case e: Exception if (e.getCause() match {
+          case _: m.pc.DisplayableException => true
+          case _ => false
+        }) =>
+      e.getCause().getMessage()
+  }
+
 }
diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
@@ -1209,6 +1209,22 @@ abstract class MetalsLspService(
       codeActionProvider.codeActions(params, token).map(_.asJava)
     }
 
+  override def codeActionResolve(
+      params: CodeAction
+  ): CompletableFuture[CodeAction] = {
+    CancelTokens.future { token =>
+      codeActionProvider
+        .resolveCodeAction(params, token)
+        .recover(
+          getOptDisplayableMessage.andThen { msg =>
+            languageClient
+              .showMessage(MessageType.Info, msg)
+            params
+          }
+        )
+    }
+  }
+
   override def codeLens(
       params: CodeLensParams
   ): CompletableFuture[util.List[CodeLens]] =

diff --git a/metals/src/main/scala/scala/meta/internal/metals/ServerCommands.scala b/metals/src/main/scala/scala/meta/internal/metals/ServerCommands.scala
@@ -6,7 +6,6 @@ import javax.annotation.Nullable
 import scala.meta.internal.metals.newScalaFile.NewFileTypes
 
 import ch.epfl.scala.{bsp4j => b}
-import org.eclipse.lsp4j
 import org.eclipse.lsp4j.Location
 import org.eclipse.lsp4j.TextDocumentIdentifier
 import org.eclipse.lsp4j.TextDocumentPositionParams
@@ -630,23 +629,6 @@ object ServerCommands {
        |""".stripMargin,
   )
 
-  final case class ExtractMethodParams(
-      param: TextDocumentIdentifier,
-      range: lsp4j.Range,
-      extractPosition: lsp4j.Position,
-  )
-  val ExtractMethod = new ParametrizedCommand[ExtractMethodParams](
-    "extract-method",
-    "Extract method from range",
-    """|Whenever a user chooses code action to extract method, this command is later ran to
-       |calculate parameters for the newly created method and create its definition.
-       |""".stripMargin,
-    """|LSP [`TextDocumentIdentifier`], (https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocumentIdentifier),
-       |LSP [`Range`], range of the code you'd like to extract as method,
-       |LSP [`Position`], position where the definition of extracted method will be created.
-       |""".stripMargin,
-  )
-
   final case class ConvertToNamedArgsRequest(
       position: TextDocumentPositionParams,
       argIndices: ju.List[Integer],
@@ -788,7 +770,6 @@ object ServerCommands {
       PresentationCompilerRestart,
       ResetChoicePopup,
       ResetNotifications,
-      ExtractMethod,
       RestartBuildServer,
       RunDoctor,
       RunScalafix,

diff --git a/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/WorkspaceLspService.scala
@@ -44,7 +44,6 @@ import scala.meta.internal.tvp.TreeViewProvider
 import scala.meta.internal.tvp.TreeViewVisibilityDidChangeParams
 import scala.meta.io.AbsolutePath
 import scala.meta.metals.lsp.ScalaLspService
-import scala.meta.pc.DisplayableException
 
 import ch.epfl.scala.bsp4j.DebugSessionParams
 import com.google.gson.Gson
@@ -536,6 +535,15 @@ class WorkspaceLspService(
   ): CompletableFuture[ju.List[CodeAction]] =
     getServiceFor(params.getTextDocument.getUri).codeAction(params)
 
+  override def codeActionResolve(
+      codeAction: CodeAction
+  ): CompletableFuture[CodeAction] =
+    currentFolder
+      .map(
+        _.codeActionResolve(codeAction)
+      )
+      .getOrElse(Future.successful(codeAction).asJava)
+
   override def codeLens(
       params: CodeLensParams
   ): CompletableFuture[ju.List[CodeLens]] =
@@ -1111,14 +1119,6 @@ class WorkspaceLspService(
           if currentOrHeadOrFallback.allActionCommandsIds(
             actionCommand.getCommand()
           ) =>
-        val getOptDisplayableMessage: PartialFunction[Throwable, String] = {
-          case e: DisplayableException => e.getMessage()
-          case e: Exception if (e.getCause() match {
-                case _: DisplayableException => true
-                case _ => false
-              }) =>
-            e.getCause().getMessage()
-        }
         CancelTokens.future { token =>
           currentFolder
             .map(
@@ -1195,19 +1195,20 @@ class WorkspaceLspService(
         capabilities.setWorkspaceSymbolProvider(true)
         capabilities.setDocumentSymbolProvider(true)
         capabilities.setDocumentFormattingProvider(true)
+        val codeActionOptions = new lsp4j.CodeActionOptions()
+
         if (initializeParams.supportsCodeActionLiterals) {
-          capabilities.setCodeActionProvider(
-            new lsp4j.CodeActionOptions(
-              List(
-                lsp4j.CodeActionKind.QuickFix,
-                lsp4j.CodeActionKind.Refactor,
-                lsp4j.CodeActionKind.SourceOrganizeImports,
-              ).asJava
-            )
+          codeActionOptions.setCodeActionKinds(
+            List(
+              lsp4j.CodeActionKind.QuickFix,
+              lsp4j.CodeActionKind.Refactor,
+              lsp4j.CodeActionKind.SourceOrganizeImports,
+            ).asJava
           )
-        } else {
-          capabilities.setCodeActionProvider(true)
         }
+        codeActionOptions.setResolveProvider(true)
+
+        capabilities.setCodeActionProvider(codeActionOptions)
         val inlayHintsCapabilities = new lsp4j.InlayHintRegistrationOptions()
         inlayHintsCapabilities.setResolveProvider(true)
         capabilities.setInlayHintProvider(inlayHintsCapabilities)

diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeAction.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeAction.scala
@@ -33,6 +33,10 @@ trait CodeAction {
       ec: ExecutionContext
   ): Future[Unit] = Future.unit
 
+  def resolveCodeAction(codeAction: l.CodeAction, token: CancelToken)(implicit
+      ec: ExecutionContext
+  ): Option[Future[l.CodeAction]] = None
+
   def contribute(
       params: l.CodeActionParams,
       token: CancelToken,

diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionBuilder.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionBuilder.scala
@@ -4,6 +4,7 @@ import scala.meta.internal.metals.MetalsEnrichments._
 import scala.meta.internal.metals.logging
 import scala.meta.io.AbsolutePath
 
+import com.google.gson.JsonObject
 import org.eclipse.{lsp4j => l}
 
 object CodeActionBuilder {
@@ -15,6 +16,7 @@ object CodeActionBuilder {
       changes: Seq[(AbsolutePath, Seq[l.TextEdit])] = Nil,
       documentChanges: List[DocumentChange] = Nil,
       command: Option[l.Command] = None,
+      data: Option[JsonObject] = None,
       diagnostics: List[l.Diagnostic] = Nil,
       disabledReason: Option[String] = None,
   ): l.CodeAction = {
@@ -45,6 +47,7 @@ object CodeActionBuilder {
       codeAction.setEdit(workspaceEdits)
     }
     command.foreach(codeAction.setCommand)
+    data.foreach(codeAction.setData)
     disabledReason.foreach(reason =>
       codeAction.setDisabled(new l.CodeActionDisabled(reason))
     )

diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala
@@ -2,9 +2,8 @@ package scala.meta.internal.metals.codeactions
 
 import scala.concurrent.ExecutionContext
 import scala.concurrent.Future
-import scala.jdk.CollectionConverters._
 
-import scala.meta.internal.metals.MetalsEnrichments.XtensionString
+import scala.meta.internal.metals.MetalsEnrichments._
 import scala.meta.internal.metals._
 import scala.meta.internal.metals.clients.language.MetalsLanguageClient
 import scala.meta.internal.metals.codeactions.CodeAction
@@ -40,7 +39,7 @@ final class CodeActionProvider(
     new RewriteBracesParensCodeAction(trees),
     new ExtractValueCodeAction(trees, buffers),
     new CreateCompanionObjectCodeAction(trees, buffers),
-    new ExtractMethodCodeAction(trees, compilers, languageClient),
+    new ExtractMethodCodeAction(trees, compilers),
     new InlineValueCodeAction(trees, compilers, languageClient),
     new ConvertToNamedArguments(trees, compilers, languageClient),
     new FlatMapToForComprehensionCodeAction(trees, buffers),
@@ -88,6 +87,25 @@ final class CodeActionProvider(
     Future.sequence(running).map(_ => ())
   }
 
+  /**
+   * Resolved command inside a code action lazily.
+   */
+  def resolveCodeAction(
+      resolvedAction: l.CodeAction,
+      token: CancelToken,
+  ): Future[l.CodeAction] = {
+    val resolved = for {
+      action <- allActions
+    } yield action.resolveCodeAction(resolvedAction, token)
+
+    resolved.collectFirst { case Some(resolved) =>
+      resolved
+    } match {
+      case None => Future.successful(resolvedAction)
+      case Some(resolvingAction) => resolvingAction
+    }
+  }
+
   val allActionCommandsIds: Set[String] =
     allActions.flatMap(_.command).map(_.id).toSet
 

diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/ExtractMethodCodeAction.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/ExtractMethodCodeAction.scala
@@ -9,9 +9,9 @@ import scala.meta.Template
 import scala.meta.Term
 import scala.meta.Tree
 import scala.meta.internal.metals.Compilers
+import scala.meta.internal.metals.JsonParser
+import scala.meta.internal.metals.JsonParser.XtensionSerializableToJson
 import scala.meta.internal.metals.MetalsEnrichments._
-import scala.meta.internal.metals.ServerCommands
-import scala.meta.internal.metals.clients.language.MetalsLanguageClient
 import scala.meta.internal.metals.logging
 import scala.meta.internal.parsing.Trees
 import scala.meta.pc.CancelToken
@@ -22,38 +22,45 @@ import org.eclipse.{lsp4j => l}
 class ExtractMethodCodeAction(
     trees: Trees,
     compilers: Compilers,
-    languageClient: MetalsLanguageClient,
 ) extends CodeAction {
+  ExtractMethodCodeAction
 
-  override type CommandData = ServerCommands.ExtractMethodParams
+  private val parser = new JsonParser.Of[ExtractMethodParams]
 
-  override def command: Option[ActionCommand] = Some(
-    ServerCommands.ExtractMethod
+  private case class ExtractMethodParams(
+      param: l.TextDocumentIdentifier,
+      range: l.Range,
+      extractPosition: l.Position,
   )
+
   override def kind: String = l.CodeActionKind.RefactorExtract
 
-  override def handleCommand(
-      data: ServerCommands.ExtractMethodParams,
-      token: CancelToken,
-  )(implicit ec: ExecutionContext): Future[Unit] = {
-    val doc = data.param
-    val uri = doc.getUri()
-    for {
-      edits <- compilers.extractMethod(
-        doc,
-        data.range,
-        data.extractPosition,
-        token,
-      )
-      _ = logging.logErrorWhen(
-        edits.isEmpty(),
-        s"Could not extract method from range \n${data.range}\nin file ${uri.toAbsolutePath}",
-      )
-      workspaceEdit = new l.WorkspaceEdit(Map(uri -> edits).asJava)
-      _ <- languageClient
-        .applyEdit(new l.ApplyWorkspaceEditParams(workspaceEdit))
-        .asScala
-    } yield ()
+  override def resolveCodeAction(codeAction: l.CodeAction, token: CancelToken)(
+      implicit ec: ExecutionContext
+  ): Option[Future[l.CodeAction]] = {
+    val data = codeAction.getData()
+    data match {
+      case parser.Jsonized(data) =>
+        val doc = data.param
+        val uri = doc.getUri()
+        val modifiedCodeAction = for {
+          edits <- compilers.extractMethod(
+            doc,
+            data.range,
+            data.extractPosition,
+            token,
+          )
+          _ = logging.logErrorWhen(
+            edits.isEmpty(),
+            s"Could not extract method from range \n${data.range}\nin file ${uri.toAbsolutePath}",
+          )
+          workspaceEdit = new l.WorkspaceEdit(Map(uri -> edits).asJava)
+          _ = codeAction.setEdit(workspaceEdit)
+        } yield codeAction
+        Some(modifiedCodeAction)
+      case _ =>
+        None
+    }
   }
 
   override def contribute(params: CodeActionParams, token: CancelToken)(implicit
@@ -102,17 +109,15 @@ class ExtractMethodCodeAction(
             head.pos.toLsp.getStart(),
             expr.pos.toLsp.getEnd(),
           )
-          val command = ServerCommands.ExtractMethod.toLsp(
-            ServerCommands.ExtractMethodParams(
-              params.getTextDocument(),
-              exprRange,
-              defnPos.pos.toLsp.getStart(),
-            )
+          val data = ExtractMethodParams(
+            params.getTextDocument(),
+            exprRange,
+            defnPos.pos.toLsp.getStart(),
           )
           CodeActionBuilder.build(
             title = ExtractMethodCodeAction.title(scopeName),
             kind = this.kind,
-            command = Some(command),
+            data = Some(data.toJsonObject),
           )
         }
       }

diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/ExtractRenameMember.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/ExtractRenameMember.scala
@@ -30,7 +30,6 @@ import scala.meta.pc.CancelToken
 import scala.meta.tokens.Token
 import scala.meta.transversers.SimpleTraverser
 
-import org.eclipse.lsp4j.ApplyWorkspaceEditParams
 import org.eclipse.lsp4j.Location
 import org.eclipse.lsp4j.WorkspaceEdit
 import org.eclipse.{lsp4j => l}
@@ -440,7 +439,7 @@ class ExtractRenameMember(
 
   private def calculate(
       params: l.TextDocumentPositionParams
-  ): Future[(ApplyWorkspaceEditParams, Option[Location])] = Future {
+  ): Future[(l.ApplyWorkspaceEditParams, Option[Location])] = Future {
     val uri = params.getTextDocument().getUri()
 
     def isCompanion(
@@ -516,7 +515,7 @@ class ExtractRenameMember(
       newFileMemberRange.setEnd(pos)
       val workspaceEdit = new WorkspaceEdit(Map(uri -> edits.asJava).asJava)
       (
-        new ApplyWorkspaceEditParams(workspaceEdit),
+        new l.ApplyWorkspaceEditParams(workspaceEdit),
         Option(new Location(newFileUri, newFileMemberRange)),
       )
     }

diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/InsertInferredType.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/InsertInferredType.scala
@@ -14,6 +14,7 @@ import scala.meta.internal.metals.ServerCommands
 import scala.meta.internal.metals.clients.language.MetalsLanguageClient
 import scala.meta.internal.metals.codeactions.CodeAction
 import scala.meta.internal.metals.codeactions.CodeActionBuilder
+import scala.meta.internal.metals.logging
 import scala.meta.internal.parsing.Trees
 import scala.meta.pc.CancelToken
 
@@ -43,7 +44,10 @@ class InsertInferredType(
         textDocumentParams,
         token,
       )
-      if (!edits.isEmpty())
+      _ = logging.logErrorWhen(
+        edits.isEmpty(),
+        s"No inferred type found for ${textDocumentParams}",
+      )
       workspaceEdit = new l.WorkspaceEdit(Map(uri -> edits).asJava)
       _ <- languageClient
         .applyEdit(new l.ApplyWorkspaceEditParams(workspaceEdit))

diff --git a/metals/src/main/scala/scala/meta/metals/lsp/DelegatingScalaService.scala b/metals/src/main/scala/scala/meta/metals/lsp/DelegatingScalaService.scala
@@ -145,6 +145,10 @@ class DelegatingScalaService(
       params: CodeActionParams
   ): CompletableFuture[util.List[CodeAction]] = underlying.codeAction(params)
 
+  override def codeActionResolve(
+      params: CodeAction
+  ): CompletableFuture[CodeAction] = underlying.codeActionResolve(params)
+
   override def codeLens(
       params: CodeLensParams
   ): CompletableFuture[util.List[CodeLens]] = underlying.codeLens(params)