From 460f0ef6695d58f21e431331ad60d9a76d40d064 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 12 Apr 2021 23:08:56 +0300 Subject: [PATCH] internal: unfork code paths for unresolved and resolved assist --- crates/ide_assists/src/lib.rs | 2 + crates/rust-analyzer/src/from_proto.rs | 30 ++--- crates/rust-analyzer/src/handlers.rs | 106 +++++++----------- crates/rust-analyzer/src/to_proto.rs | 39 +++---- .../rust-analyzer/tests/rust-analyzer/main.rs | 2 - 5 files changed, 75 insertions(+), 104 deletions(-) diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 3e2c82dace0..3694f468f0b 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -28,7 +28,9 @@ macro_rules! eprintln { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum AssistKind { + // FIXME: does the None variant make sense? Probably not. None, + QuickFix, Generate, Refactor, diff --git a/crates/rust-analyzer/src/from_proto.rs b/crates/rust-analyzer/src/from_proto.rs index 5b02b2598d3..712d5a9c277 100644 --- a/crates/rust-analyzer/src/from_proto.rs +++ b/crates/rust-analyzer/src/from_proto.rs @@ -42,27 +42,27 @@ pub(crate) fn text_range(line_index: &LineIndex, range: lsp_types::Range) -> Tex TextRange::new(start, end) } -pub(crate) fn file_id(world: &GlobalStateSnapshot, url: &lsp_types::Url) -> Result { - world.url_to_file_id(url) +pub(crate) fn file_id(snap: &GlobalStateSnapshot, url: &lsp_types::Url) -> Result { + snap.url_to_file_id(url) } pub(crate) fn file_position( - world: &GlobalStateSnapshot, + snap: &GlobalStateSnapshot, tdpp: lsp_types::TextDocumentPositionParams, ) -> Result { - let file_id = file_id(world, &tdpp.text_document.uri)?; - let line_index = world.file_line_index(file_id)?; + let file_id = file_id(snap, &tdpp.text_document.uri)?; + let line_index = snap.file_line_index(file_id)?; let offset = offset(&line_index, tdpp.position); Ok(FilePosition { file_id, offset }) } pub(crate) fn file_range( - world: &GlobalStateSnapshot, + snap: &GlobalStateSnapshot, text_document_identifier: lsp_types::TextDocumentIdentifier, range: lsp_types::Range, ) -> Result { - let file_id = file_id(world, &text_document_identifier.uri)?; - let line_index = world.file_line_index(file_id)?; + let file_id = file_id(snap, &text_document_identifier.uri)?; + let line_index = snap.file_line_index(file_id)?; let range = text_range(&line_index, range); Ok(FileRange { file_id, range }) } @@ -82,7 +82,7 @@ pub(crate) fn assist_kind(kind: lsp_types::CodeActionKind) -> Option } pub(crate) fn annotation( - world: &GlobalStateSnapshot, + snap: &GlobalStateSnapshot, code_lens: lsp_types::CodeLens, ) -> Result { let data = code_lens.data.unwrap(); @@ -91,25 +91,25 @@ pub(crate) fn annotation( match resolve { lsp_ext::CodeLensResolveData::Impls(params) => { let file_id = - world.url_to_file_id(¶ms.text_document_position_params.text_document.uri)?; - let line_index = world.file_line_index(file_id)?; + snap.url_to_file_id(¶ms.text_document_position_params.text_document.uri)?; + let line_index = snap.file_line_index(file_id)?; Ok(Annotation { range: text_range(&line_index, code_lens.range), kind: AnnotationKind::HasImpls { - position: file_position(world, params.text_document_position_params)?, + position: file_position(snap, params.text_document_position_params)?, data: None, }, }) } lsp_ext::CodeLensResolveData::References(params) => { - let file_id = world.url_to_file_id(¶ms.text_document.uri)?; - let line_index = world.file_line_index(file_id)?; + let file_id = snap.url_to_file_id(¶ms.text_document.uri)?; + let line_index = snap.file_line_index(file_id)?; Ok(Annotation { range: text_range(&line_index, code_lens.range), kind: AnnotationKind::HasReferences { - position: file_position(world, params)?, + position: file_position(snap, params)?, data: None, }, }) diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 107685c631e..85c70373a8c 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -8,8 +8,8 @@ }; use ide::{ - AnnotationConfig, FileId, FilePosition, FileRange, HoverAction, HoverGotoTypeData, Query, - RangeInfo, Runnable, RunnableKind, SearchScope, SourceChange, TextEdit, + AnnotationConfig, AssistKind, FileId, FilePosition, FileRange, HoverAction, HoverGotoTypeData, + Query, RangeInfo, Runnable, RunnableKind, SearchScope, SourceChange, TextEdit, }; use ide_db::SymbolKind; use itertools::Itertools; @@ -17,7 +17,7 @@ use lsp_types::{ CallHierarchyIncomingCall, CallHierarchyIncomingCallsParams, CallHierarchyItem, CallHierarchyOutgoingCall, CallHierarchyOutgoingCallsParams, CallHierarchyPrepareParams, - CodeActionKind, CodeLens, CompletionItem, Diagnostic, DiagnosticTag, DocumentFormattingParams, + CodeLens, CompletionItem, Diagnostic, DiagnosticTag, DocumentFormattingParams, DocumentHighlight, FoldingRange, FoldingRangeParams, HoverContents, Location, NumberOrString, Position, PrepareRenameResponse, Range, RenameParams, SemanticTokensDeltaParams, SemanticTokensFullDeltaResult, SemanticTokensParams, SemanticTokensRangeParams, @@ -36,7 +36,7 @@ diff::diff, from_proto, global_state::{GlobalState, GlobalStateSnapshot}, - line_index::{LineEndings, LineIndex}, + line_index::LineEndings, lsp_ext::{self, InlayHint, InlayHintsParams}, lsp_utils::all_edits_are_disjoint, to_proto, LspError, Result, @@ -982,88 +982,68 @@ pub(crate) fn handle_code_action( params: lsp_types::CodeActionParams, ) -> Result>> { let _p = profile::span("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 !snap.config.code_action_literals() { + // We intentionally don't support command-based actions, as those either + // require either custom client-code or server-initiated edits. Server + // initiated edits break causality, so we avoid those. return Ok(None); } - let file_id = from_proto::file_id(&snap, ¶ms.text_document.uri)?; - let line_index = snap.file_line_index(file_id)?; - let range = from_proto::text_range(&line_index, params.range); - let frange = FileRange { file_id, range }; + let line_index = + snap.file_line_index(from_proto::file_id(&snap, ¶ms.text_document.uri)?)?; + let frange = from_proto::file_range(&snap, params.text_document.clone(), params.range)?; let mut assists_config = snap.config.assist(); assists_config.allowed = params - .clone() .context .only + .clone() .map(|it| it.into_iter().filter_map(from_proto::assist_kind).collect()); let mut res: Vec = Vec::new(); - let include_quick_fixes = match ¶ms.context.only { - Some(v) => v.iter().any(|it| { - it == &lsp_types::CodeActionKind::EMPTY || it == &lsp_types::CodeActionKind::QUICKFIX - }), + let include_quick_fixes = match &assists_config.allowed { + Some(v) => v.iter().any(|it| it == &AssistKind::None || it == &AssistKind::QuickFix), None => true, }; + let code_action_resolve_cap = snap.config.code_action_resolve(); + + let mut assists = Vec::new(); + + // Fixes from native diagnostics. if include_quick_fixes { - add_quick_fixes(&snap, frange, &line_index, &mut res)?; + let diagnostics = snap.analysis.diagnostics(&snap.config.diagnostics(), frange.file_id)?; + assists.extend( + diagnostics + .into_iter() + .filter_map(|d| d.fix) + .filter(|fix| fix.target.intersect(frange.range).is_some()), + ) } - if snap.config.code_action_resolve() { - for (index, assist) in - snap.analysis.assists(&assists_config, false, frange)?.into_iter().enumerate() - { - res.push(to_proto::unresolved_code_action(&snap, params.clone(), assist, index)?); - } - } else { - for assist in snap.analysis.assists(&assists_config, true, frange)?.into_iter() { - res.push(to_proto::resolved_code_action(&snap, assist)?); + // Assists proper. + assists.extend(snap.analysis.assists(&assists_config, !code_action_resolve_cap, frange)?); + for (index, assist) in assists.into_iter().enumerate() { + let resolve_data = + if code_action_resolve_cap { Some((index, params.clone())) } else { None }; + let code_action = to_proto::code_action(&snap, assist, resolve_data)?; + res.push(code_action) + } + + // Fixes from `cargo check`. + for fix in snap.check_fixes.get(&frange.file_id).into_iter().flatten() { + // FIXME: this mapping is awkward and shouldn't exist. Refactor + // `snap.check_fixes` to not convert to LSP prematurely. + let fix_range = from_proto::text_range(&line_index, fix.range); + if fix_range.intersect(frange.range).is_some() { + res.push(fix.action.clone()); } } Ok(Some(res)) } -fn add_quick_fixes( - snap: &GlobalStateSnapshot, - frange: FileRange, - line_index: &LineIndex, - acc: &mut Vec, -) -> Result<()> { - let diagnostics = snap.analysis.diagnostics(&snap.config.diagnostics(), frange.file_id)?; - - for fix in diagnostics - .into_iter() - .filter_map(|d| d.fix) - .filter(|fix| fix.target.intersect(frange.range).is_some()) - { - if let Some(source_change) = fix.source_change { - let edit = to_proto::snippet_workspace_edit(&snap, source_change)?; - let action = lsp_ext::CodeAction { - title: fix.label.to_string(), - group: None, - kind: Some(CodeActionKind::QUICKFIX), - edit: Some(edit), - is_preferred: Some(false), - data: None, - }; - acc.push(action); - } - } - - for fix in snap.check_fixes.get(&frange.file_id).into_iter().flatten() { - let fix_range = from_proto::text_range(&line_index, fix.range); - if fix_range.intersect(frange.range).is_some() { - acc.push(fix.action.clone()); - } - } - Ok(()) -} - pub(crate) fn handle_code_action_resolve( snap: GlobalStateSnapshot, mut code_action: lsp_ext::CodeAction, @@ -1091,7 +1071,7 @@ pub(crate) fn handle_code_action_resolve( let index = index.parse::().unwrap(); let assist = &assists[index]; assert!(assist.id.0 == id); - let edit = to_proto::resolved_code_action(&snap, assist.clone())?.edit; + let edit = to_proto::code_action(&snap, assist.clone(), None)?.edit; code_action.edit = edit; Ok(code_action) } diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 9fac562ff71..53852bfdc81 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs @@ -843,40 +843,31 @@ pub(crate) fn code_action_kind(kind: AssistKind) -> lsp_types::CodeActionKind { } } -pub(crate) fn unresolved_code_action( +pub(crate) fn code_action( snap: &GlobalStateSnapshot, - code_action_params: lsp_types::CodeActionParams, assist: Assist, - index: usize, + resolve_data: Option<(usize, lsp_types::CodeActionParams)>, ) -> Result { - assert!(assist.source_change.is_none()); - let res = lsp_ext::CodeAction { + let mut res = lsp_ext::CodeAction { title: assist.label.to_string(), group: assist.group.filter(|_| snap.config.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) -} - -pub(crate) fn resolved_code_action( - snap: &GlobalStateSnapshot, - assist: Assist, -) -> Result { - let change = assist.source_change.unwrap(); - let res = lsp_ext::CodeAction { - edit: Some(snippet_workspace_edit(snap, change)?), - title: assist.label.to_string(), - group: assist.group.filter(|_| snap.config.code_action_group()).map(|gr| gr.0), - kind: Some(code_action_kind(assist.id.1)), - is_preferred: None, data: None, }; + match (assist.source_change, resolve_data) { + (Some(it), _) => res.edit = Some(snippet_workspace_edit(snap, it)?), + (None, Some((index, code_action_params))) => { + res.data = Some(lsp_ext::CodeActionData { + id: format!("{}:{}", assist.id.0, index.to_string()), + code_action_params, + }); + } + (None, None) => { + stdx::never!("assist should always be resolved if client can't do lazy resolving") + } + }; Ok(res) } diff --git a/crates/rust-analyzer/tests/rust-analyzer/main.rs b/crates/rust-analyzer/tests/rust-analyzer/main.rs index 1e4c04bbf6d..52a2674d576 100644 --- a/crates/rust-analyzer/tests/rust-analyzer/main.rs +++ b/crates/rust-analyzer/tests/rust-analyzer/main.rs @@ -340,7 +340,6 @@ fn main() {} } ] }, - "isPreferred": false, "kind": "quickfix", "title": "Create module" }]), @@ -411,7 +410,6 @@ fn main() {{}} } ] }, - "isPreferred": false, "kind": "quickfix", "title": "Create module" }]),