From 7d2eb000b078143e9fa6225d00ef52fc7c606fdf Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 10 Nov 2020 18:20:01 +0100 Subject: Switch to upstream protocol for resolving code action 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. --- crates/rust-analyzer/src/caps.rs | 6 ++-- crates/rust-analyzer/src/config.rs | 11 +++++-- .../test_data/rustc_unused_variable.txt | 2 +- .../test_data/rustc_unused_variable_as_hint.txt | 2 +- .../test_data/rustc_unused_variable_as_info.txt | 2 +- .../diagnostics/test_data/snap_multi_line_fix.txt | 2 +- crates/rust-analyzer/src/diagnostics/to_proto.rs | 2 +- crates/rust-analyzer/src/handlers.rs | 23 +++++++++----- crates/rust-analyzer/src/lsp_ext.rs | 35 +++++++++++----------- crates/rust-analyzer/src/main_loop.rs | 2 +- crates/rust-analyzer/src/to_proto.rs | 26 +++++++++++----- docs/dev/lsp-extensions.md | 26 +--------------- editors/code/src/client.ts | 21 +++++++------ editors/code/src/commands.ts | 11 +++---- editors/code/src/lsp_ext.ts | 6 ---- 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, ¶ms, &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> { - let _p = profile::span("handle_resolve_code_action"); + mut code_action: lsp_ext::CodeAction, +) -> Result { + 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, ¶ms.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::().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, } -pub enum ResolveCodeActionRequest {} - -impl Request for ResolveCodeActionRequest { - type Params = ResolveCodeActionParams; - type Result = Option; - 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, - #[serde(skip_serializing_if = "Option::is_none")] pub group: Option, #[serde(skip_serializing_if = "Option::is_none")] pub kind: Option, @@ -282,6 +271,16 @@ pub struct CodeAction { pub edit: Option, #[serde(skip_serializing_if = "Option::is_none")] pub is_preferred: Option, + + #[serde(skip_serializing_if = "Option::is_none")] + pub data: Option, +} + +#[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::(handlers::handle_runnables) .on::(handlers::handle_inlay_hints) .on::(handlers::handle_code_action) - .on::(handlers::handle_resolve_code_action) + .on::(handlers::handle_code_action_resolve) .on::(handlers::handle_hover) .on::(handlers::handle_open_docs) .on::(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 { 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 { 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 @@