Merge #6521

6521: Switch to upstream protocol for resolving code action r=matklad a=matklad

Note that we have to maintain custom implementation on the client
side: I don't see how to marry bulitin resolve support with groups and
snippets.


Co-authored-by: Aleksey Kladov <[email protected]>

15 files changed, 86 insertions, 91 deletions

diff --git a/crates/rust-analyzer/src/caps.rs b/crates/rust-analyzer/src/caps.rs
index ff1ae9575..9a8870053 100644
--- a/crates/rust-analyzer/src/caps.rs
+++ b/crates/rust-analyzer/src/caps.rs
@@ -16,8 +16,6 @@ use serde_json::json;
 use crate::semantic_tokens;
 
 pub fn server_capabilities(client_caps: &ClientCapabilities) -> ServerCapabilities {
-    let code_action_provider = code_action_capabilities(client_caps);
-
     ServerCapabilities {
         text_document_sync: Some(TextDocumentSyncCapability::Options(TextDocumentSyncOptions {
             open_close: Some(true),
@@ -49,7 +47,7 @@ pub fn server_capabilities(client_caps: &ClientCapabilities) -> ServerCapabiliti
         document_highlight_provider: Some(OneOf::Left(true)),
         document_symbol_provider: Some(OneOf::Left(true)),
         workspace_symbol_provider: Some(OneOf::Left(true)),
-        code_action_provider: Some(code_action_provider),
+        code_action_provider: Some(code_action_capabilities(client_caps)),
         code_lens_provider: Some(CodeLensOptions { resolve_provider: Some(true) }),
         document_formatting_provider: Some(OneOf::Left(true)),
         document_range_formatting_provider: None,
@@ -113,7 +111,7 @@ fn code_action_capabilities(client_caps: &ClientCapabilities) -> CodeActionProvi
                     CodeActionKind::REFACTOR_INLINE,
                     CodeActionKind::REFACTOR_REWRITE,
                 ]),
-                resolve_provider: None,
+                resolve_provider: Some(true),
                 work_done_progress_options: Default::default(),
             })
         })
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index 2ed6a0d82..9cc14fe82 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -144,7 +144,7 @@ pub struct ClientCapsConfig {
     pub code_action_literals: bool,
     pub work_done_progress: bool,
     pub code_action_group: bool,
-    pub resolve_code_action: bool,
+    pub code_action_resolve: bool,
     pub hover_actions: bool,
     pub status_notification: bool,
     pub signature_help_label_offsets: bool,
@@ -383,6 +383,14 @@ impl Config {
                     }
                 }
             }
+
+            if let Some(code_action) = &doc_caps.code_action {
+                if let Some(resolve_support) = &code_action.resolve_support {
+                    if resolve_support.properties.iter().any(|it| it == "edit") {
+                        self.client_caps.code_action_resolve = true;
+                    }
+                }
+            }
         }
 
         if let Some(window_caps) = caps.window.as_ref() {
@@ -400,7 +408,6 @@ impl Config {
             self.assist.allow_snippets(snippet_text_edit);
 
             self.client_caps.code_action_group = get_bool("codeActionGroup");
-            self.client_caps.resolve_code_action = get_bool("resolveCodeAction");
             self.client_caps.hover_actions = get_bool("hoverActions");
             self.client_caps.status_notification = get_bool("statusNotification");
         }
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt
index 8dc53391e..a8d85f008 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt
@@ -36,7 +36,6 @@
         fixes: [
             CodeAction {
                 title: "consider prefixing with an underscore",
-                id: None,
                 group: None,
                 kind: Some(
                     CodeActionKind(
@@ -70,6 +69,7 @@
                 is_preferred: Some(
                     true,
                 ),
+                data: None,
             },
         ],
     },
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt
index c8703194c..0b8937376 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt
@@ -36,7 +36,6 @@
         fixes: [
             CodeAction {
                 title: "consider prefixing with an underscore",
-                id: None,
                 group: None,
                 kind: Some(
                     CodeActionKind(
@@ -70,6 +69,7 @@
                 is_preferred: Some(
                     true,
                 ),
+                data: None,
             },
         ],
     },
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt
index dc93227ad..7fa9dee62 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt
@@ -36,7 +36,6 @@
         fixes: [
             CodeAction {
                 title: "consider prefixing with an underscore",
-                id: None,
                 group: None,
                 kind: Some(
                     CodeActionKind(
@@ -70,6 +69,7 @@
                 is_preferred: Some(
                     true,
                 ),
+                data: None,
             },
         ],
     },
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt b/crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt
index 81f752672..3c97b2084 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt
@@ -51,7 +51,6 @@
         fixes: [
             CodeAction {
                 title: "return the expression directly",
-                id: None,
                 group: None,
                 kind: Some(
                     CodeActionKind(
@@ -98,6 +97,7 @@
                 is_preferred: Some(
                     true,
                 ),
+                data: None,
             },
         ],
     },
diff --git a/crates/rust-analyzer/src/diagnostics/to_proto.rs b/crates/rust-analyzer/src/diagnostics/to_proto.rs
index b949577c1..15145415b 100644
--- a/crates/rust-analyzer/src/diagnostics/to_proto.rs
+++ b/crates/rust-analyzer/src/diagnostics/to_proto.rs
@@ -110,7 +110,6 @@ fn map_rust_child_diagnostic(
     } else {
         MappedRustChildDiagnostic::SuggestedFix(lsp_ext::CodeAction {
             title: rd.message.clone(),
-            id: None,
             group: None,
             kind: Some(lsp_types::CodeActionKind::QUICKFIX),
             edit: Some(lsp_ext::SnippetWorkspaceEdit {
@@ -119,6 +118,7 @@ fn map_rust_child_diagnostic(
                 document_changes: None,
             }),
             is_preferred: Some(true),
+            data: None,
         })
     }
 }
diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs
index 049c583a4..95659b0db 100644
--- a/crates/rust-analyzer/src/handlers.rs
+++ b/crates/rust-analyzer/src/handlers.rs
@@ -806,11 +806,11 @@ fn handle_fixes(
         let edit = to_proto::snippet_workspace_edit(&snap, fix.source_change)?;
         let action = lsp_ext::CodeAction {
             title: fix.label.to_string(),
-            id: None,
             group: None,
             kind: Some(CodeActionKind::QUICKFIX),
             edit: Some(edit),
             is_preferred: Some(false),
+            data: None,
         };
         res.push(action);
     }
@@ -852,11 +852,11 @@ pub(crate) fn handle_code_action(
 
     handle_fixes(&snap, &params, &mut res)?;
 
-    if snap.config.client_caps.resolve_code_action {
+    if snap.config.client_caps.code_action_resolve {
         for (index, assist) in
             snap.analysis.unresolved_assists(&snap.config.assist, frange)?.into_iter().enumerate()
         {
-            res.push(to_proto::unresolved_code_action(&snap, assist, index)?);
+            res.push(to_proto::unresolved_code_action(&snap, params.clone(), assist, index)?);
         }
     } else {
         for assist in snap.analysis.resolved_assists(&snap.config.assist, frange)?.into_iter() {
@@ -867,11 +867,16 @@ pub(crate) fn handle_code_action(
     Ok(Some(res))
 }
 
-pub(crate) fn handle_resolve_code_action(
+pub(crate) fn handle_code_action_resolve(
     mut snap: GlobalStateSnapshot,
-    params: lsp_ext::ResolveCodeActionParams,
-) -> Result<Option<lsp_ext::SnippetWorkspaceEdit>> {
-    let _p = profile::span("handle_resolve_code_action");
+    mut code_action: lsp_ext::CodeAction,
+) -> Result<lsp_ext::CodeAction> {
+    let _p = profile::span("handle_code_action_resolve");
+    let params = match code_action.data.take() {
+        Some(it) => it,
+        None => Err("can't resolve code action without data")?,
+    };
+
     let file_id = from_proto::file_id(&snap, &params.code_action_params.text_document.uri)?;
     let line_index = snap.analysis.file_line_index(file_id)?;
     let range = from_proto::text_range(&line_index, params.code_action_params.range);
@@ -888,7 +893,9 @@ pub(crate) fn handle_resolve_code_action(
     let index = index.parse::<usize>().unwrap();
     let assist = &assists[index];
     assert!(assist.assist.id.0 == id);
-    Ok(to_proto::resolved_code_action(&snap, assist.clone())?.edit)
+    let edit = to_proto::resolved_code_action(&snap, assist.clone())?.edit;
+    code_action.edit = edit;
+    Ok(code_action)
 }
 
 pub(crate) fn handle_code_lens(
diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs
index f31f8d900..a7c3028e4 100644
--- a/crates/rust-analyzer/src/lsp_ext.rs
+++ b/crates/rust-analyzer/src/lsp_ext.rs
@@ -113,22 +113,6 @@ pub struct JoinLinesParams {
     pub ranges: Vec<Range>,
 }
 
-pub enum ResolveCodeActionRequest {}
-
-impl Request for ResolveCodeActionRequest {
-    type Params = ResolveCodeActionParams;
-    type Result = Option<SnippetWorkspaceEdit>;
-    const METHOD: &'static str = "experimental/resolveCodeAction";
-}
-
-/// Params for the ResolveCodeActionRequest
-#[derive(Debug, Eq, PartialEq, Clone, Deserialize, Serialize)]
-#[serde(rename_all = "camelCase")]
-pub struct ResolveCodeActionParams {
-    pub code_action_params: lsp_types::CodeActionParams,
-    pub id: String,
-}
-
 pub enum OnEnter {}
 
 impl Request for OnEnter {
@@ -265,13 +249,18 @@ impl Request for CodeActionRequest {
     const METHOD: &'static str = "textDocument/codeAction";
 }
 
+pub enum CodeActionResolveRequest {}
+impl Request for CodeActionResolveRequest {
+    type Params = CodeAction;
+    type Result = CodeAction;
+    const METHOD: &'static str = "codeAction/resolve";
+}
+
 #[derive(Debug, PartialEq, Clone, Default, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct CodeAction {
     pub title: String,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub id: Option<String>,
-    #[serde(skip_serializing_if = "Option::is_none")]
     pub group: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub kind: Option<CodeActionKind>,
@@ -282,6 +271,16 @@ pub struct CodeAction {
     pub edit: Option<SnippetWorkspaceEdit>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub is_preferred: Option<bool>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub data: Option<CodeActionData>,
+}
+
+#[derive(Debug, Eq, PartialEq, Clone, Deserialize, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct CodeActionData {
+    pub code_action_params: lsp_types::CodeActionParams,
+    pub id: String,
 }
 
 #[derive(Debug, Eq, PartialEq, Clone, Default, Deserialize, Serialize)]
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index d504572a8..6e6cac42e 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -435,7 +435,7 @@ impl GlobalState {
             .on::<lsp_ext::Runnables>(handlers::handle_runnables)
             .on::<lsp_ext::InlayHints>(handlers::handle_inlay_hints)
             .on::<lsp_ext::CodeActionRequest>(handlers::handle_code_action)
-            .on::<lsp_ext::ResolveCodeActionRequest>(handlers::handle_resolve_code_action)
+            .on::<lsp_ext::CodeActionResolveRequest>(handlers::handle_code_action_resolve)
             .on::<lsp_ext::HoverRequest>(handlers::handle_hover)
             .on::<lsp_ext::ExternalDocs>(handlers::handle_open_docs)
             .on::<lsp_types::request::OnTypeFormatting>(handlers::handle_on_type_formatting)
diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs
index f8ecd8e83..4bdf4bf0f 100644
--- a/crates/rust-analyzer/src/to_proto.rs
+++ b/crates/rust-analyzer/src/to_proto.rs
@@ -735,16 +735,20 @@ pub(crate) fn code_action_kind(kind: AssistKind) -> lsp_types::CodeActionKind {
 
 pub(crate) fn unresolved_code_action(
     snap: &GlobalStateSnapshot,
+    code_action_params: lsp_types::CodeActionParams,
     assist: Assist,
     index: usize,
 ) -> Result<lsp_ext::CodeAction> {
     let res = lsp_ext::CodeAction {
         title: assist.label.to_string(),
-        id: Some(format!("{}:{}", assist.id.0, index.to_string())),
         group: assist.group.filter(|_| snap.config.client_caps.code_action_group).map(|gr| gr.0),
         kind: Some(code_action_kind(assist.id.1)),
         edit: None,
         is_preferred: None,
+        data: Some(lsp_ext::CodeActionData {
+            id: format!("{}:{}", assist.id.0, index.to_string()),
+            code_action_params,
+        }),
     };
     Ok(res)
 }
@@ -754,13 +758,19 @@ pub(crate) fn resolved_code_action(
     assist: ResolvedAssist,
 ) -> Result<lsp_ext::CodeAction> {
     let change = assist.source_change;
-    unresolved_code_action(snap, assist.assist, 0).and_then(|it| {
-        Ok(lsp_ext::CodeAction {
-            id: None,
-            edit: Some(snippet_workspace_edit(snap, change)?),
-            ..it
-        })
-    })
+    let res = lsp_ext::CodeAction {
+        edit: Some(snippet_workspace_edit(snap, change)?),
+        title: assist.assist.label.to_string(),
+        group: assist
+            .assist
+            .group
+            .filter(|_| snap.config.client_caps.code_action_group)
+            .map(|gr| gr.0),
+        kind: Some(code_action_kind(assist.assist.id.1)),
+        is_preferred: None,
+        data: None,
+    };
+    Ok(res)
 }
 
 pub(crate) fn runnable(
diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md
index 780f5cb91..77d4e0ec9 100644
--- a/docs/dev/lsp-extensions.md
+++ b/docs/dev/lsp-extensions.md
@@ -1,5 +1,5 @@
 <!---
-lsp_ext.rs hash: 286f8bbac885531a
+lsp_ext.rs hash: 4f86fb54e4b2870e
 
 If you need to change the above hash to make the test pass, please check if you
 need to adjust this doc as well and ping this issue:
@@ -109,30 +109,6 @@ Invoking code action at this position will yield two code actions for importing
 * Is a fixed two-level structure enough?
 * Should we devise a general way to encode custom interaction protocols for GUI refactorings?
 
-## Lazy assists with `ResolveCodeAction`
-
-**Issue:** https://github.com/microsoft/language-server-protocol/issues/787
-
-**Client Capability** `{ "resolveCodeAction": boolean }`
-
-If this capability is set, the assists will be computed lazily. Thus `CodeAction` returned from the server will only contain `id` but not `edit` or `command` fields. The only exclusion from the rule is the diagnostic edits.
-
-After the client got the id, it should then call `experimental/resolveCodeAction` command on the server and provide the following payload:
-
-```typescript
-interface ResolveCodeActionParams {
-    id: string;
-    codeActionParams: lc.CodeActionParams;
-}
-```
-
-As a result of the command call the client will get the respective workspace edit (`lc.WorkspaceEdit`).
-
-### Unresolved Questions
-
-* Apply smarter filtering for ids?
-* Upon `resolveCodeAction` command only call the assits which should be resolved and not all of them?
-
 ## Parent Module
 
 **Issue:** https://github.com/microsoft/language-server-protocol/issues/1002
diff --git a/editors/code/src/client.ts b/editors/code/src/client.ts
index d032b45b7..c9d032ead 100644
--- a/editors/code/src/client.ts
+++ b/editors/code/src/client.ts
@@ -4,6 +4,7 @@ import * as ra from '../src/lsp_ext';
 import * as Is from 'vscode-languageclient/lib/common/utils/is';
 import { DocumentSemanticsTokensSignature, DocumentSemanticsTokensEditsSignature, DocumentRangeSemanticTokensSignature } from 'vscode-languageclient/lib/common/semanticTokens';
 import { assert } from './util';
+import { WorkspaceEdit } from 'vscode';
 
 function renderCommand(cmd: ra.CommandLink) {
     return `[${cmd.title}](command:${cmd.command}?${encodeURIComponent(JSON.stringify(cmd.arguments))} '${cmd.tooltip!}')`;
@@ -75,8 +76,8 @@ export function createClient(serverPath: string, cwd: string): lc.LanguageClient
                         return Promise.resolve(null);
                     });
             },
-            // Using custom handling of CodeActions where each code action is resolved lazily
-            // That's why we are not waiting for any command or edits
+            // Using custom handling of CodeActions to support action groups and snippet edits.
+            // Note that this means we have to re-implement lazy edit resolving ourselves as well.
             async provideCodeActions(document: vscode.TextDocument, range: vscode.Range, context: vscode.CodeActionContext, token: vscode.CancellationToken, _next: lc.ProvideCodeActionsSignature) {
                 const params: lc.CodeActionParams = {
                     textDocument: client.code2ProtocolConverter.asTextDocumentIdentifier(document),
@@ -99,16 +100,15 @@ export function createClient(serverPath: string, cwd: string): lc.LanguageClient
                         const kind = client.protocol2CodeConverter.asCodeActionKind((item as any).kind);
                         const action = new vscode.CodeAction(item.title, kind);
                         const group = (item as any).group;
-                        const id = (item as any).id;
-                        const resolveParams: ra.ResolveCodeActionParams = {
-                            id: id,
-                            codeActionParams: params
-                        };
                         action.command = {
                             command: "rust-analyzer.resolveCodeAction",
                             title: item.title,
-                            arguments: [resolveParams],
+                            arguments: [item],
                         };
+
+                        // Set a dummy edit, so that VS Code doesn't try to resolve this.
+                        action.edit = new WorkspaceEdit();
+
                         if (group) {
                             let entry = groups.get(group);
                             if (!entry) {
@@ -134,6 +134,10 @@ export function createClient(serverPath: string, cwd: string): lc.LanguageClient
                                     return { label: item.title, arguments: item.command!!.arguments!![0] };
                                 })],
                             };
+
+                            // Set a dummy edit, so that VS Code doesn't try to resolve this.
+                            action.edit = new WorkspaceEdit();
+
                             result[index] = action;
                         }
                     }
@@ -164,7 +168,6 @@ class ExperimentalFeatures implements lc.StaticFeature {
         const caps: any = capabilities.experimental ?? {};
         caps.snippetTextEdit = true;
         caps.codeActionGroup = true;
-        caps.resolveCodeAction = true;
         caps.hoverActions = true;
         caps.statusNotification = true;
         capabilities.experimental = caps;
diff --git a/editors/code/src/commands.ts b/editors/code/src/commands.ts
index 22509e874..cf34622c3 100644
--- a/editors/code/src/commands.ts
+++ b/editors/code/src/commands.ts
@@ -395,7 +395,7 @@ export function showReferences(ctx: Ctx): Cmd {
 }
 
 export function applyActionGroup(_ctx: Ctx): Cmd {
-    return async (actions: { label: string; arguments: ra.ResolveCodeActionParams }[]) => {
+    return async (actions: { label: string; arguments: lc.CodeAction }[]) => {
         const selectedAction = await vscode.window.showQuickPick(actions);
         if (!selectedAction) return;
         vscode.commands.executeCommand(
@@ -442,12 +442,13 @@ export function openDocs(ctx: Ctx): Cmd {
 
 export function resolveCodeAction(ctx: Ctx): Cmd {
     const client = ctx.client;
-    return async (params: ra.ResolveCodeActionParams) => {
-        const item: lc.WorkspaceEdit = await client.sendRequest(ra.resolveCodeAction, params);
-        if (!item) {
+    return async (params: lc.CodeAction) => {
+        params.command = undefined;
+        const item = await client.sendRequest(lc.CodeActionResolveRequest.type, params);
+        if (!item.edit) {
             return;
         }
-        const edit = client.protocol2CodeConverter.asWorkspaceEdit(item);
+        const edit = client.protocol2CodeConverter.asWorkspaceEdit(item.edit);
         await applySnippetWorkspaceEdit(edit);
     };
 }
diff --git a/editors/code/src/lsp_ext.ts b/editors/code/src/lsp_ext.ts
index fc8e120b3..d320c3cd7 100644
--- a/editors/code/src/lsp_ext.ts
+++ b/editors/code/src/lsp_ext.ts
@@ -43,12 +43,6 @@ export const matchingBrace = new lc.RequestType<MatchingBraceParams, lc.Position
 
 export const parentModule = new lc.RequestType<lc.TextDocumentPositionParams, lc.LocationLink[], void>("experimental/parentModule");
 
-export interface ResolveCodeActionParams {
-    id: string;
-    codeActionParams: lc.CodeActionParams;
-}
-export const resolveCodeAction = new lc.RequestType<ResolveCodeActionParams, lc.WorkspaceEdit, unknown>('experimental/resolveCodeAction');
-
 export interface JoinLinesParams {
     textDocument: lc.TextDocumentIdentifier;
     ranges: lc.Range[];