From 57cd936c5262c3b43626618be42d7a72f71c3539 Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Tue, 2 Jun 2020 22:21:48 +0200 Subject: Preliminary implementation of lazy CodeAssits --- crates/ra_ide/src/lib.rs | 39 +++++----- crates/rust-analyzer/src/config.rs | 7 +- ...tics__to_proto__tests__snap_multi_line_fix.snap | 1 + ...o_proto__tests__snap_rustc_unused_variable.snap | 1 + crates/rust-analyzer/src/diagnostics/to_proto.rs | 1 + crates/rust-analyzer/src/lsp_ext.rs | 19 +++++ crates/rust-analyzer/src/main_loop.rs | 1 + crates/rust-analyzer/src/main_loop/handlers.rs | 89 +++++++++++++++++----- crates/rust-analyzer/src/to_proto.rs | 39 +++++++++- 9 files changed, 150 insertions(+), 47 deletions(-) (limited to 'crates') diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 12d5716e8..34c2d75fe 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -77,7 +77,7 @@ pub use crate::{ }; pub use hir::Documentation; -pub use ra_assists::{AssistConfig, AssistId}; +pub use ra_assists::{Assist, AssistConfig, AssistId, ResolvedAssist}; pub use ra_db::{ Canceled, CrateGraph, CrateId, Edition, FileId, FilePosition, FileRange, SourceRootId, }; @@ -142,14 +142,6 @@ pub struct AnalysisHost { db: RootDatabase, } -#[derive(Debug)] -pub struct Assist { - pub id: AssistId, - pub label: String, - pub group_label: Option, - pub source_change: SourceChange, -} - impl AnalysisHost { pub fn new(lru_capacity: Option) -> AnalysisHost { AnalysisHost { db: RootDatabase::new(lru_capacity) } @@ -470,20 +462,23 @@ impl Analysis { self.with_db(|db| completion::completions(db, config, position).map(Into::into)) } - /// Computes assists (aka code actions aka intentions) for the given + /// Computes resolved assists with source changes for the given position. + pub fn resolved_assists( + &self, + config: &AssistConfig, + frange: FileRange, + ) -> Cancelable> { + self.with_db(|db| ra_assists::Assist::resolved(db, config, frange)) + } + + /// Computes unresolved assists (aka code actions aka intentions) for the given /// position. - pub fn assists(&self, config: &AssistConfig, frange: FileRange) -> Cancelable> { - self.with_db(|db| { - ra_assists::Assist::resolved(db, config, frange) - .into_iter() - .map(|assist| Assist { - id: assist.assist.id, - label: assist.assist.label, - group_label: assist.assist.group.map(|it| it.0), - source_change: assist.source_change, - }) - .collect() - }) + pub fn unresolved_assists( + &self, + config: &AssistConfig, + frange: FileRange, + ) -> Cancelable> { + self.with_db(|db| Assist::unresolved(db, config, frange)) } /// Computes the set of diagnostics for the given file. diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index c0f7c2c0c..9e8e7ab82 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -103,6 +103,7 @@ pub struct ClientCapsConfig { pub code_action_literals: bool, pub work_done_progress: bool, pub code_action_group: bool, + pub resolve_code_action: bool, } impl Default for Config { @@ -299,7 +300,11 @@ impl Config { let code_action_group = experimental.get("codeActionGroup").and_then(|it| it.as_bool()) == Some(true); - self.client_caps.code_action_group = code_action_group + self.client_caps.code_action_group = code_action_group; + + let resolve_code_action = + experimental.get("resolveCodeAction").and_then(|it| it.as_bool()) == Some(true); + self.client_caps.resolve_code_action = resolve_code_action; } } } diff --git a/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_multi_line_fix.snap b/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_multi_line_fix.snap index c40cfdcdc..272057b47 100644 --- a/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_multi_line_fix.snap +++ b/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_multi_line_fix.snap @@ -65,6 +65,7 @@ expression: diag fixes: [ CodeAction { title: "return the expression directly", + id: None, group: None, kind: Some( "quickfix", diff --git a/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_rustc_unused_variable.snap b/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_rustc_unused_variable.snap index 6dd3fcb2e..9a7972ff5 100644 --- a/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_rustc_unused_variable.snap +++ b/crates/rust-analyzer/src/diagnostics/snapshots/rust_analyzer__diagnostics__to_proto__tests__snap_rustc_unused_variable.snap @@ -50,6 +50,7 @@ expression: diag fixes: [ CodeAction { title: "consider prefixing with an underscore", + id: None, group: None, kind: Some( "quickfix", diff --git a/crates/rust-analyzer/src/diagnostics/to_proto.rs b/crates/rust-analyzer/src/diagnostics/to_proto.rs index a500d670a..257910e09 100644 --- a/crates/rust-analyzer/src/diagnostics/to_proto.rs +++ b/crates/rust-analyzer/src/diagnostics/to_proto.rs @@ -145,6 +145,7 @@ fn map_rust_child_diagnostic( } else { MappedRustChildDiagnostic::SuggestedFix(lsp_ext::CodeAction { title: rd.message.clone(), + id: None, group: None, kind: Some("quickfix".to_string()), edit: Some(lsp_ext::SnippetWorkspaceEdit { diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs index 173c23b9e..05b76e7c8 100644 --- a/crates/rust-analyzer/src/lsp_ext.rs +++ b/crates/rust-analyzer/src/lsp_ext.rs @@ -98,6 +98,23 @@ 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 label: String, +} + pub enum OnEnter {} impl Request for OnEnter { @@ -197,6 +214,8 @@ impl Request for CodeActionRequest { 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, diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index f1287d52c..ad9dd4c59 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -517,6 +517,7 @@ fn on_request( .on::(handlers::handle_runnables)? .on::(handlers::handle_inlay_hints)? .on::(handlers::handle_code_action)? + .on::(handlers::handle_resolve_code_action)? .on::(handlers::handle_on_type_formatting)? .on::(handlers::handle_document_symbol)? .on::(handlers::handle_workspace_symbol)? diff --git a/crates/rust-analyzer/src/main_loop/handlers.rs b/crates/rust-analyzer/src/main_loop/handlers.rs index bc7c7f1ef..b342f4bb7 100644 --- a/crates/rust-analyzer/src/main_loop/handlers.rs +++ b/crates/rust-analyzer/src/main_loop/handlers.rs @@ -693,40 +693,35 @@ pub fn handle_formatting( }])) } -pub fn handle_code_action( - world: WorldSnapshot, - params: lsp_types::CodeActionParams, -) -> Result>> { - let _p = profile("handle_code_action"); - // We intentionally don't support command-based actions, as those either - // requires custom client-code anyway, or requires server-initiated edits. - // Server initiated edits break causality, so we avoid those as well. - if !world.config.client_caps.code_action_literals { - return Ok(None); - } - +fn handle_fixes( + world: &WorldSnapshot, + params: &lsp_types::CodeActionParams, + res: &mut Vec, +) -> Result<()> { let file_id = from_proto::file_id(&world, ¶ms.text_document.uri)?; let line_index = world.analysis().file_line_index(file_id)?; let range = from_proto::text_range(&line_index, params.range); - let frange = FileRange { file_id, range }; let diagnostics = world.analysis().diagnostics(file_id)?; - let mut res: Vec = Vec::new(); let fixes_from_diagnostics = diagnostics .into_iter() .filter_map(|d| Some((d.range, d.fix?))) .filter(|(diag_range, _fix)| diag_range.intersect(range).is_some()) .map(|(_range, fix)| fix); - for fix in fixes_from_diagnostics { let title = fix.label; let edit = to_proto::snippet_workspace_edit(&world, fix.source_change)?; - let action = - lsp_ext::CodeAction { title, group: None, kind: None, edit: Some(edit), command: None }; + let action = lsp_ext::CodeAction { + title, + id: None, + group: None, + kind: None, + edit: Some(edit), + command: None, + }; res.push(action); } - for fix in world.check_fixes.get(&file_id).into_iter().flatten() { let fix_range = from_proto::text_range(&line_index, fix.range); if fix_range.intersect(range).is_none() { @@ -734,13 +729,67 @@ pub fn handle_code_action( } res.push(fix.action.clone()); } + Ok(()) +} + +pub fn handle_code_action( + world: WorldSnapshot, + params: lsp_types::CodeActionParams, +) -> Result>> { + let _p = profile("handle_code_action"); + // We intentionally don't support command-based actions, as those either + // requires custom client-code anyway, or requires server-initiated edits. + // Server initiated edits break causality, so we avoid those as well. + if !world.config.client_caps.code_action_literals { + return Ok(None); + } + + let file_id = from_proto::file_id(&world, ¶ms.text_document.uri)?; + let line_index = world.analysis().file_line_index(file_id)?; + let range = from_proto::text_range(&line_index, params.range); + let frange = FileRange { file_id, range }; + let mut res: Vec = Vec::new(); + + handle_fixes(&world, ¶ms, &mut res)?; - for assist in world.analysis().assists(&world.config.assist, frange)?.into_iter() { - res.push(to_proto::code_action(&world, assist)?.into()); + if world.config.client_caps.resolve_code_action { + for assist in world.analysis().unresolved_assists(&world.config.assist, frange)?.into_iter() + { + res.push(to_proto::unresolved_code_action(&world, assist)?.into()); + } + } else { + for assist in world.analysis().resolved_assists(&world.config.assist, frange)?.into_iter() { + res.push(to_proto::resolved_code_action(&world, assist)?.into()); + } } + Ok(Some(res)) } +pub fn handle_resolve_code_action( + world: WorldSnapshot, + params: lsp_ext::ResolveCodeActionParams, +) -> Result> { + if !world.config.client_caps.resolve_code_action { + return Ok(None); + } + + let _p = profile("handle_resolve_code_action"); + let file_id = from_proto::file_id(&world, ¶ms.code_action_params.text_document.uri)?; + let line_index = world.analysis().file_line_index(file_id)?; + let range = from_proto::text_range(&line_index, params.code_action_params.range); + let frange = FileRange { file_id, range }; + let mut res: Vec = Vec::new(); + + for assist in world.analysis().resolved_assists(&world.config.assist, frange)?.into_iter() { + res.push(to_proto::resolved_code_action(&world, assist)?.into()); + } + Ok(res + .into_iter() + .find(|action| action.id.clone().unwrap() == params.id && action.title == params.label) + .and_then(|action| action.edit)) +} + pub fn handle_code_lens( world: WorldSnapshot, params: lsp_types::CodeLensParams, diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 2fbbb4e63..db78c4b5c 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -3,8 +3,8 @@ use ra_db::{FileId, FileRange}; use ra_ide::{ Assist, CompletionItem, CompletionItemKind, Documentation, FileSystemEdit, Fold, FoldKind, FunctionSignature, Highlight, HighlightModifier, HighlightTag, HighlightedRange, Indel, - InlayHint, InlayKind, InsertTextFormat, LineIndex, NavigationTarget, ReferenceAccess, Severity, - SourceChange, SourceFileEdit, TextEdit, + InlayHint, InlayKind, InsertTextFormat, LineIndex, NavigationTarget, ReferenceAccess, + ResolvedAssist, Severity, SourceChange, SourceFileEdit, TextEdit, }; use ra_syntax::{SyntaxKind, TextRange, TextSize}; use ra_vfs::LineEndings; @@ -617,10 +617,41 @@ fn main() { } } -pub(crate) fn code_action(world: &WorldSnapshot, assist: Assist) -> Result { +pub(crate) fn unresolved_code_action( + world: &WorldSnapshot, + assist: Assist, +) -> Result { let res = lsp_ext::CodeAction { title: assist.label, - group: if world.config.client_caps.code_action_group { assist.group_label } else { None }, + id: Some(assist.id.0.to_owned()), + group: assist.group.and_then(|it| { + if world.config.client_caps.code_action_group { + None + } else { + Some(it.0) + } + }), + kind: Some(String::new()), + edit: None, + command: None, + }; + Ok(res) +} + +pub(crate) fn resolved_code_action( + world: &WorldSnapshot, + assist: ResolvedAssist, +) -> Result { + let res = lsp_ext::CodeAction { + title: assist.assist.label, + id: Some(assist.assist.id.0.to_owned()), + group: assist.assist.group.and_then(|it| { + if world.config.client_caps.code_action_group { + None + } else { + Some(it.0) + } + }), kind: Some(String::new()), edit: Some(snippet_workspace_edit(world, assist.source_change)?), command: None, -- cgit v1.2.3 From bacd0428fa0fd744eb0aac6d5d7abd18c6c707b7 Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Wed, 3 Jun 2020 18:39:01 +0200 Subject: Fix review comments --- crates/rust-analyzer/src/lsp_ext.rs | 1 - crates/rust-analyzer/src/main_loop/handlers.rs | 27 ++++++++++---------- crates/rust-analyzer/src/to_proto.rs | 34 +++++++++----------------- 3 files changed, 24 insertions(+), 38 deletions(-) (limited to 'crates') diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs index 4b436c301..3b957534d 100644 --- a/crates/rust-analyzer/src/lsp_ext.rs +++ b/crates/rust-analyzer/src/lsp_ext.rs @@ -111,7 +111,6 @@ impl Request for ResolveCodeActionRequest { pub struct ResolveCodeActionParams { pub code_action_params: lsp_types::CodeActionParams, pub id: String, - pub label: String, } pub enum OnEnter {} diff --git a/crates/rust-analyzer/src/main_loop/handlers.rs b/crates/rust-analyzer/src/main_loop/handlers.rs index 3c4064441..fab82ff7e 100644 --- a/crates/rust-analyzer/src/main_loop/handlers.rs +++ b/crates/rust-analyzer/src/main_loop/handlers.rs @@ -756,9 +756,13 @@ pub fn handle_code_action( handle_fixes(&world, ¶ms, &mut res)?; if world.config.client_caps.resolve_code_action { - for assist in world.analysis().unresolved_assists(&world.config.assist, frange)?.into_iter() + for (index, assist) in world + .analysis() + .unresolved_assists(&world.config.assist, frange)? + .into_iter() + .enumerate() { - res.push(to_proto::unresolved_code_action(&world, assist)?); + res.push(to_proto::unresolved_code_action(&world, assist, index)?); } } else { for assist in world.analysis().resolved_assists(&world.config.assist, frange)?.into_iter() { @@ -773,24 +777,19 @@ pub fn handle_resolve_code_action( world: WorldSnapshot, params: lsp_ext::ResolveCodeActionParams, ) -> Result> { - if !world.config.client_caps.resolve_code_action { - return Ok(None); - } - let _p = profile("handle_resolve_code_action"); let file_id = from_proto::file_id(&world, ¶ms.code_action_params.text_document.uri)?; let line_index = world.analysis().file_line_index(file_id)?; let range = from_proto::text_range(&line_index, params.code_action_params.range); let frange = FileRange { file_id, range }; - let mut res: Vec = Vec::new(); - for assist in world.analysis().resolved_assists(&world.config.assist, frange)?.into_iter() { - res.push(to_proto::resolved_code_action(&world, assist)?); - } - Ok(res - .into_iter() - .find(|action| action.id.clone().unwrap() == params.id && action.title == params.label) - .and_then(|action| action.edit)) + let assists = world.analysis().resolved_assists(&world.config.assist, frange)?; + let id_components = params.id.split(":").collect::>(); + let index = id_components.last().unwrap().parse::().unwrap(); + let id_string = id_components.first().unwrap(); + let assist = &assists[index]; + assert!(assist.assist.id.0 == *id_string); + Ok(to_proto::resolved_code_action(&world, assist.clone())?.edit) } pub fn handle_code_lens( diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 3672b1a26..fb33bdd5f 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -622,17 +622,12 @@ fn main() { pub(crate) fn unresolved_code_action( world: &WorldSnapshot, assist: Assist, + index: usize, ) -> Result { let res = lsp_ext::CodeAction { title: assist.label, - id: Some(assist.id.0.to_owned()), - group: assist.group.and_then(|it| { - if world.config.client_caps.code_action_group { - None - } else { - Some(it.0) - } - }), + id: Some(format!("{}:{}", assist.id.0.to_owned(), index.to_string())), + group: assist.group.filter(|_| world.config.client_caps.code_action_group).map(|gr| gr.0), kind: Some(String::new()), edit: None, command: None, @@ -644,21 +639,14 @@ pub(crate) fn resolved_code_action( world: &WorldSnapshot, assist: ResolvedAssist, ) -> Result { - let res = lsp_ext::CodeAction { - title: assist.assist.label, - id: Some(assist.assist.id.0.to_owned()), - group: assist.assist.group.and_then(|it| { - if world.config.client_caps.code_action_group { - None - } else { - Some(it.0) - } - }), - kind: Some(String::new()), - edit: Some(snippet_workspace_edit(world, assist.source_change)?), - command: None, - }; - Ok(res) + let change = assist.source_change; + unresolved_code_action(world, assist.assist, 0).and_then(|it| { + Ok(lsp_ext::CodeAction { + id: None, + edit: Some(snippet_workspace_edit(world, change)?), + ..it + }) + }) } pub(crate) fn runnable( -- cgit v1.2.3 From 6cd2e04bd2a703c335566224e8b6bf773b83c0c6 Mon Sep 17 00:00:00 2001 From: Mikhail Rakhmanov Date: Wed, 3 Jun 2020 19:33:57 +0200 Subject: Fix more comments --- crates/rust-analyzer/src/main_loop/handlers.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'crates') diff --git a/crates/rust-analyzer/src/main_loop/handlers.rs b/crates/rust-analyzer/src/main_loop/handlers.rs index a3361d6dc..6acf80c58 100644 --- a/crates/rust-analyzer/src/main_loop/handlers.rs +++ b/crates/rust-analyzer/src/main_loop/handlers.rs @@ -25,7 +25,7 @@ use ra_project_model::TargetKind; use ra_syntax::{AstNode, SyntaxKind, TextRange, TextSize}; use serde::{Deserialize, Serialize}; use serde_json::to_value; -use stdx::format_to; +use stdx::{format_to, split1}; use crate::{ cargo_target_spec::CargoTargetSpec, @@ -786,11 +786,10 @@ pub fn handle_resolve_code_action( let frange = FileRange { file_id, range }; let assists = snap.analysis().resolved_assists(&snap.config.assist, frange)?; - let id_components = params.id.split(":").collect::>(); - let index = id_components.last().unwrap().parse::().unwrap(); - let id_string = id_components.first().unwrap(); + let (id_string, index) = split1(¶ms.id, ':').unwrap(); + let index = index.parse::().unwrap(); let assist = &assists[index]; - assert!(assist.assist.id.0 == *id_string); + assert!(assist.assist.id.0 == id_string); Ok(to_proto::resolved_code_action(&snap, assist.clone())?.edit) } -- cgit v1.2.3