From 6e87828756d970e9c25635aa9f71f0a90cc8ff65 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 6 Jan 2021 20:23:53 +0300 Subject: YAGNI active_resolve_capabilities This leaks a lot of LSP details into ide layer, which we want to avoid: https://github.com/rust-analyzer/rust-analyzer/tree/c9cec381bcfd97e5f3536e31a9c546ab5c0665e6/docs/dev#lsp-independence Additionally, all what this infra does is providing a toggle for auto-import completion, but we already have one! --- .../completion/src/completions/unqualified_path.rs | 8 +--- crates/completion/src/config.rs | 22 ----------- crates/completion/src/lib.rs | 2 +- crates/ide/src/lib.rs | 4 +- crates/rust-analyzer/src/caps.rs | 46 +++++++++------------- crates/rust-analyzer/src/config.rs | 7 ++-- crates/rust-analyzer/src/handlers.rs | 21 +++------- 7 files changed, 32 insertions(+), 78 deletions(-) (limited to 'crates') diff --git a/crates/completion/src/completions/unqualified_path.rs b/crates/completion/src/completions/unqualified_path.rs index 2f41a3f96..896f167ff 100644 --- a/crates/completion/src/completions/unqualified_path.rs +++ b/crates/completion/src/completions/unqualified_path.rs @@ -46,7 +46,7 @@ pub(crate) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC acc.add_resolution(ctx, name.to_string(), &res) }); - if ctx.config.enable_autoimport_completions && ctx.config.resolve_additional_edits_lazily() { + if ctx.config.enable_autoimport_completions { fuzzy_completion(acc, ctx); } } @@ -206,11 +206,7 @@ mod tests { } fn fuzzy_completion_config() -> CompletionConfig { - let mut completion_config = CompletionConfig::default(); - completion_config - .active_resolve_capabilities - .insert(crate::CompletionResolveCapability::AdditionalTextEdits); - completion_config + CompletionConfig::default() } #[test] diff --git a/crates/completion/src/config.rs b/crates/completion/src/config.rs index 30577dc11..9f82b0346 100644 --- a/crates/completion/src/config.rs +++ b/crates/completion/src/config.rs @@ -5,7 +5,6 @@ //! completions if we are allowed to. use ide_db::helpers::insert_use::MergeBehavior; -use rustc_hash::FxHashSet; #[derive(Clone, Debug, PartialEq, Eq)] pub struct CompletionConfig { @@ -15,32 +14,12 @@ pub struct CompletionConfig { pub add_call_argument_snippets: bool, pub snippet_cap: Option, pub merge: Option, - /// A set of capabilities, enabled on the client and supported on the server. - pub active_resolve_capabilities: FxHashSet, -} - -/// A resolve capability, supported on the server. -/// If the client registers any completion resolve capabilities, -/// the server is able to render completion items' corresponding fields later, -/// not during an initial completion item request. -/// See https://github.com/rust-analyzer/rust-analyzer/issues/6366 for more details. -#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)] -pub enum CompletionResolveCapability { - Documentation, - Detail, - AdditionalTextEdits, } impl CompletionConfig { pub fn allow_snippets(&mut self, yes: bool) { self.snippet_cap = if yes { Some(SnippetCap { _private: () }) } else { None } } - - /// Whether the completions' additional edits are calculated when sending an initional completions list - /// or later, in a separate resolve request. - pub fn resolve_additional_edits_lazily(&self) -> bool { - self.active_resolve_capabilities.contains(&CompletionResolveCapability::AdditionalTextEdits) - } } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -57,7 +36,6 @@ impl Default for CompletionConfig { add_call_argument_snippets: true, snippet_cap: Some(SnippetCap { _private: () }), merge: Some(MergeBehavior::Full), - active_resolve_capabilities: FxHashSet::default(), } } } diff --git a/crates/completion/src/lib.rs b/crates/completion/src/lib.rs index c57d05bbe..366aced71 100644 --- a/crates/completion/src/lib.rs +++ b/crates/completion/src/lib.rs @@ -20,7 +20,7 @@ use text_edit::TextEdit; use crate::{completions::Completions, context::CompletionContext, item::CompletionKind}; pub use crate::{ - config::{CompletionConfig, CompletionResolveCapability}, + config::CompletionConfig, item::{CompletionItem, CompletionItemKind, CompletionScore, ImportEdit, InsertTextFormat}, }; diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index a450794f3..72c8bfd09 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -82,8 +82,8 @@ pub use crate::{ }; pub use assists::{Assist, AssistConfig, AssistId, AssistKind}; pub use completion::{ - CompletionConfig, CompletionItem, CompletionItemKind, CompletionResolveCapability, - CompletionScore, ImportEdit, InsertTextFormat, + CompletionConfig, CompletionItem, CompletionItemKind, CompletionScore, ImportEdit, + InsertTextFormat, }; pub use hir::{Documentation, Semantics}; pub use ide_db::base_db::{ diff --git a/crates/rust-analyzer/src/caps.rs b/crates/rust-analyzer/src/caps.rs index 80e46bf7f..3db0d55c5 100644 --- a/crates/rust-analyzer/src/caps.rs +++ b/crates/rust-analyzer/src/caps.rs @@ -1,7 +1,6 @@ //! Advertizes the capabilities of the LSP Server. use std::env; -use ide::CompletionResolveCapability; use lsp_types::{ CallHierarchyServerCapability, ClientCapabilities, CodeActionKind, CodeActionOptions, CodeActionProviderCapability, CodeLensOptions, CompletionOptions, @@ -14,7 +13,6 @@ use lsp_types::{ WorkDoneProgressOptions, WorkspaceFileOperationsServerCapabilities, WorkspaceServerCapabilities, }; -use rustc_hash::FxHashSet; use serde_json::json; use crate::semantic_tokens; @@ -118,37 +116,31 @@ pub fn server_capabilities(client_caps: &ClientCapabilities) -> ServerCapabiliti } fn completions_resolve_provider(client_caps: &ClientCapabilities) -> Option { - if enabled_completions_resolve_capabilities(client_caps)?.is_empty() { + if completion_item_edit_resolve(client_caps) { + Some(true) + } else { log::info!("No `additionalTextEdits` completion resolve capability was found in the client capabilities, autoimport completion is disabled"); None - } else { - Some(true) } } /// Parses client capabilities and returns all completion resolve capabilities rust-analyzer supports. -pub(crate) fn enabled_completions_resolve_capabilities( - caps: &ClientCapabilities, -) -> Option> { - Some( - caps.text_document - .as_ref()? - .completion - .as_ref()? - .completion_item - .as_ref()? - .resolve_support - .as_ref()? - .properties - .iter() - .filter_map(|cap_string| match cap_string.as_str() { - "additionalTextEdits" => Some(CompletionResolveCapability::AdditionalTextEdits), - "detail" => Some(CompletionResolveCapability::Detail), - "documentation" => Some(CompletionResolveCapability::Documentation), - _unsupported => None, - }) - .collect(), - ) +pub(crate) fn completion_item_edit_resolve(caps: &ClientCapabilities) -> bool { + (|| { + Some( + caps.text_document + .as_ref()? + .completion + .as_ref()? + .completion_item + .as_ref()? + .resolve_support + .as_ref()? + .properties + .iter() + .any(|cap_string| cap_string.as_str() == "additionalTextEdits"), + ) + })() == Some(true) } fn code_action_capabilities(client_caps: &ClientCapabilities) -> CodeActionProviderCapability { diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 24e7936fc..ce9526315 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -20,7 +20,7 @@ use rustc_hash::FxHashSet; use serde::{de::DeserializeOwned, Deserialize}; use vfs::AbsPathBuf; -use crate::{caps::enabled_completions_resolve_capabilities, diagnostics::DiagnosticsMapConfig}; +use crate::{caps::completion_item_edit_resolve, diagnostics::DiagnosticsMapConfig}; config_data! { struct ConfigData { @@ -536,12 +536,11 @@ impl Config { pub fn completion(&self) -> CompletionConfig { let mut res = CompletionConfig::default(); res.enable_postfix_completions = self.data.completion_postfix_enable; - res.enable_autoimport_completions = self.data.completion_autoimport_enable; + res.enable_autoimport_completions = + self.data.completion_autoimport_enable && completion_item_edit_resolve(&self.caps); res.add_call_parenthesis = self.data.completion_addCallParenthesis; res.add_call_argument_snippets = self.data.completion_addCallArgumentSnippets; res.merge = self.merge_behavior(); - res.active_resolve_capabilities = - enabled_completions_resolve_capabilities(&self.caps).unwrap_or_default(); res.allow_snippets(try_or!( self.caps diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 33661325a..a7bf0ec6a 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -9,9 +9,8 @@ use std::{ }; use ide::{ - CompletionResolveCapability, FileId, FilePosition, FileRange, HoverAction, HoverGotoTypeData, - LineIndex, NavigationTarget, Query, RangeInfo, Runnable, RunnableKind, SearchScope, - SourceChange, SymbolKind, TextEdit, + FileId, FilePosition, FileRange, HoverAction, HoverGotoTypeData, LineIndex, NavigationTarget, + Query, RangeInfo, Runnable, RunnableKind, SearchScope, SourceChange, SymbolKind, TextEdit, }; use itertools::Itertools; use lsp_server::ErrorCode; @@ -634,10 +633,9 @@ pub(crate) fn handle_completion( let mut new_completion_items = to_proto::completion_item(&line_index, line_endings, item.clone()); - if completion_config.resolve_additional_edits_lazily() { + if completion_config.enable_autoimport_completions { for new_item in &mut new_completion_items { - let _ = fill_resolve_data(&mut new_item.data, &item, &text_document_position) - .take(); + fill_resolve_data(&mut new_item.data, &item, &text_document_position); } } @@ -663,15 +661,6 @@ pub(crate) fn handle_completion_resolve( .into()); } - // FIXME resolve the other capabilities also? - let completion_config = &snap.config.completion(); - if !completion_config - .active_resolve_capabilities - .contains(&CompletionResolveCapability::AdditionalTextEdits) - { - return Ok(original_completion); - } - let resolve_data = match original_completion .data .take() @@ -690,7 +679,7 @@ pub(crate) fn handle_completion_resolve( let additional_edits = snap .analysis .resolve_completion_edits( - &completion_config, + &snap.config.completion(), FilePosition { file_id, offset }, &resolve_data.full_import_path, resolve_data.imported_name, -- cgit v1.2.3