From 3183ff3a7b1fbcf3cb5379cf162a3d769a21be7a Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 8 Dec 2020 00:46:56 +0200 Subject: Disable the completion for no corresponding client resolve capabilities --- .../completion/src/completions/unqualified_path.rs | 134 +++++++++------------ crates/completion/src/config.rs | 4 +- crates/completion/src/item.rs | 35 +----- crates/completion/src/lib.rs | 2 +- crates/completion/src/render.rs | 29 +---- crates/completion/src/render/enum_variant.rs | 2 +- crates/completion/src/render/function.rs | 2 +- crates/completion/src/render/macro_.rs | 5 +- crates/completion/src/test_utils.rs | 11 +- 9 files changed, 79 insertions(+), 145 deletions(-) (limited to 'crates/completion') diff --git a/crates/completion/src/completions/unqualified_path.rs b/crates/completion/src/completions/unqualified_path.rs index 1482df8fb..2a315cb86 100644 --- a/crates/completion/src/completions/unqualified_path.rs +++ b/crates/completion/src/completions/unqualified_path.rs @@ -44,7 +44,7 @@ pub(crate) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC acc.add_resolution(ctx, name.to_string(), &res) }); - if ctx.config.enable_experimental_completions { + if !ctx.config.disable_fuzzy_autoimports && ctx.config.resolve_additional_edits_lazily() { fuzzy_completion(acc, ctx).unwrap_or_default() } } @@ -99,6 +99,7 @@ fn complete_enum_variants(acc: &mut Completions, ctx: &CompletionContext, ty: &T // // To avoid an excessive amount of the results returned, completion input is checked for inclusion in the identifiers only // (i.e. in `HashMap` in the `std::collections::HashMap` path), also not in the module indentifiers. +// It also avoids searching for any imports for inputs with their length less that 3 symbols. // // .Merge Behaviour // @@ -107,53 +108,53 @@ fn complete_enum_variants(acc: &mut Completions, ctx: &CompletionContext, ty: &T // // .LSP and performance implications // -// LSP 3.16 provides the way to defer the computation of some completion data, including the import edits for this feature. -// If the LSP client supports the `additionalTextEdits` (case sensitive) resolve client capability, rust-analyzer computes -// the completion edits only when a corresponding completion item is selected. +// The feature is enabled only if the LSP client supports LSP protocol version 3.16+ and reports the `additionalTextEdits` +// (case sensitive) resolve client capability in its client capabilities. +// This way the server is able to defer the costly computations, doing them for a selected completion item only. // For clients with no such support, all edits have to be calculated on the completion request, including the fuzzy search completion ones, -// which might be slow. +// which might be slow ergo the feature is automatically disabled. // // .Feature toggle // -// The feature can be turned off in the settings with the `rust-analyzer.completion.enableExperimental` flag. +// The feature can be forcefully turned off in the settings with the `rust-analyzer.completion.disableFuzzyAutoimports` flag. fn fuzzy_completion(acc: &mut Completions, ctx: &CompletionContext) -> Option<()> { let _p = profile::span("fuzzy_completion"); + let potential_import_name = ctx.token.to_string(); + + if potential_import_name.len() < 3 { + return None; + } + let current_module = ctx.scope.module()?; let anchor = ctx.name_ref_syntax.as_ref()?; let import_scope = ImportScope::find_insert_use_container(anchor.syntax(), &ctx.sema)?; - let potential_import_name = ctx.token.to_string(); - - let possible_imports = imports_locator::find_similar_imports( - &ctx.sema, - ctx.krate?, - &potential_import_name, - 50, - true, - ) - .filter_map(|import_candidate| { - Some(match import_candidate { - Either::Left(module_def) => { - (current_module.find_use_path(ctx.db, module_def)?, ScopeDef::ModuleDef(module_def)) - } - Either::Right(macro_def) => { - (current_module.find_use_path(ctx.db, macro_def)?, ScopeDef::MacroDef(macro_def)) - } - }) - }) - .filter(|(mod_path, _)| mod_path.len() > 1) - .take(20) - .filter_map(|(import_path, definition)| { - render_resolution_with_import( - RenderContext::new(ctx), - ImportEdit { - import_path: import_path.clone(), - import_scope: import_scope.clone(), - merge_behaviour: ctx.config.merge, - }, - &definition, - ) - }); + let possible_imports = + imports_locator::find_similar_imports(&ctx.sema, ctx.krate?, &potential_import_name, true) + .filter_map(|import_candidate| { + Some(match import_candidate { + Either::Left(module_def) => ( + current_module.find_use_path(ctx.db, module_def)?, + ScopeDef::ModuleDef(module_def), + ), + Either::Right(macro_def) => ( + current_module.find_use_path(ctx.db, macro_def)?, + ScopeDef::MacroDef(macro_def), + ), + }) + }) + .filter(|(mod_path, _)| mod_path.len() > 1) + .filter_map(|(import_path, definition)| { + render_resolution_with_import( + RenderContext::new(ctx), + ImportEdit { + import_path: import_path.clone(), + import_scope: import_scope.clone(), + merge_behaviour: ctx.config.merge, + }, + &definition, + ) + }); acc.add_all(possible_imports); Some(()) @@ -775,7 +776,13 @@ impl My<|> #[test] fn function_fuzzy_completion() { - check_edit( + let mut completion_config = CompletionConfig::default(); + completion_config + .active_resolve_capabilities + .insert(crate::CompletionResolveCapability::AdditionalTextEdits); + + check_edit_with_config( + completion_config, "stdin", r#" //- /lib.rs crate:dep @@ -800,7 +807,13 @@ fn main() { #[test] fn macro_fuzzy_completion() { - check_edit( + let mut completion_config = CompletionConfig::default(); + completion_config + .active_resolve_capabilities + .insert(crate::CompletionResolveCapability::AdditionalTextEdits); + + check_edit_with_config( + completion_config, "macro_with_curlies!", r#" //- /lib.rs crate:dep @@ -827,37 +840,6 @@ fn main() { #[test] fn struct_fuzzy_completion() { - check_edit( - "ThirdStruct", - r#" -//- /lib.rs crate:dep -pub struct FirstStruct; -pub mod some_module { - pub struct SecondStruct; - pub struct ThirdStruct; -} - -//- /main.rs crate:main deps:dep -use dep::{FirstStruct, some_module::SecondStruct}; - -fn main() { - this<|> -} -"#, - r#" -use dep::{FirstStruct, some_module::{SecondStruct, ThirdStruct}}; - -fn main() { - ThirdStruct -} -"#, - ); - } - - /// LSP protocol supports separate completion resolve requests to do the heavy computations there. - /// This test checks that for a certain resolve capatilities no such operations (autoimport) are done. - #[test] - fn no_fuzzy_completions_applied_for_certain_resolve_capability() { let mut completion_config = CompletionConfig::default(); completion_config .active_resolve_capabilities @@ -870,22 +852,22 @@ fn main() { //- /lib.rs crate:dep pub struct FirstStruct; pub mod some_module { -pub struct SecondStruct; -pub struct ThirdStruct; + pub struct SecondStruct; + pub struct ThirdStruct; } //- /main.rs crate:main deps:dep use dep::{FirstStruct, some_module::SecondStruct}; fn main() { -this<|> + this<|> } "#, r#" -use dep::{FirstStruct, some_module::SecondStruct}; +use dep::{FirstStruct, some_module::{SecondStruct, ThirdStruct}}; fn main() { -ThirdStruct + ThirdStruct } "#, ); diff --git a/crates/completion/src/config.rs b/crates/completion/src/config.rs index 487c1d0f1..8082ec9cb 100644 --- a/crates/completion/src/config.rs +++ b/crates/completion/src/config.rs @@ -10,7 +10,7 @@ use rustc_hash::FxHashSet; #[derive(Clone, Debug, PartialEq, Eq)] pub struct CompletionConfig { pub enable_postfix_completions: bool, - pub enable_experimental_completions: bool, + pub disable_fuzzy_autoimports: bool, pub add_call_parenthesis: bool, pub add_call_argument_snippets: bool, pub snippet_cap: Option, @@ -52,7 +52,7 @@ impl Default for CompletionConfig { fn default() -> Self { CompletionConfig { enable_postfix_completions: true, - enable_experimental_completions: true, + disable_fuzzy_autoimports: false, add_call_parenthesis: true, add_call_argument_snippets: true, snippet_cap: Some(SnippetCap { _private: () }), diff --git a/crates/completion/src/item.rs b/crates/completion/src/item.rs index 978ea76f9..bd94402d7 100644 --- a/crates/completion/src/item.rs +++ b/crates/completion/src/item.rs @@ -209,7 +209,6 @@ impl CompletionItem { score: None, ref_match: None, import_to_add: None, - resolve_import_lazily: false, } } @@ -301,7 +300,6 @@ pub(crate) struct Builder { source_range: TextRange, completion_kind: CompletionKind, import_to_add: Option, - resolve_import_lazily: bool, label: String, insert_text: Option, insert_text_format: InsertTextFormat, @@ -339,25 +337,13 @@ impl Builder { } } - let mut text_edit = match self.text_edit { + let text_edit = match self.text_edit { Some(it) => it, None => { TextEdit::replace(self.source_range, insert_text.unwrap_or_else(|| label.clone())) } }; - let import_to_add = if self.resolve_import_lazily { - self.import_to_add - } else { - match apply_import_eagerly(self.import_to_add.as_ref(), &mut text_edit) { - Ok(()) => self.import_to_add, - Err(()) => { - log::error!("Failed to apply eager import edit: original edit and import edit intersect"); - None - } - } - }; - CompletionItem { source_range: self.source_range, label, @@ -372,7 +358,7 @@ impl Builder { trigger_call_info: self.trigger_call_info.unwrap_or(false), score: self.score, ref_match: self.ref_match, - import_to_add, + import_to_add: self.import_to_add, } } pub(crate) fn lookup_by(mut self, lookup: impl Into) -> Builder { @@ -435,13 +421,8 @@ impl Builder { self.trigger_call_info = Some(true); self } - pub(crate) fn add_import( - mut self, - import_to_add: Option, - resolve_import_lazily: bool, - ) -> Builder { + pub(crate) fn add_import(mut self, import_to_add: Option) -> Builder { self.import_to_add = import_to_add; - self.resolve_import_lazily = resolve_import_lazily; self } pub(crate) fn set_ref_match( @@ -453,16 +434,6 @@ impl Builder { } } -fn apply_import_eagerly( - import_to_add: Option<&ImportEdit>, - original_edit: &mut TextEdit, -) -> Result<(), ()> { - match import_to_add.and_then(|import_edit| import_edit.to_text_edit()) { - Some(import_edit) => original_edit.union(import_edit).map_err(|_| ()), - None => Ok(()), - } -} - impl<'a> Into for Builder { fn into(self) -> CompletionItem { self.build() diff --git a/crates/completion/src/lib.rs b/crates/completion/src/lib.rs index 066d589af..8df9f00fe 100644 --- a/crates/completion/src/lib.rs +++ b/crates/completion/src/lib.rs @@ -73,7 +73,7 @@ pub use crate::{ // } // ``` // -// And experimental completions, enabled with the `rust-analyzer.completion.enableExperimental` setting. +// And experimental completions, enabled with the `rust-analyzer.completion.disableFuzzyAutoimports` setting. // This flag enables or disables: // // - Auto import: additional completion options with automatic `use` import and options from all project importable items, matched for the input diff --git a/crates/completion/src/render.rs b/crates/completion/src/render.rs index 9a43480e1..b940388df 100644 --- a/crates/completion/src/render.rs +++ b/crates/completion/src/render.rs @@ -190,10 +190,7 @@ impl<'a> Render<'a> { local_name, ) .kind(CompletionItemKind::UnresolvedReference) - .add_import( - import_to_add, - self.ctx.completion.config.resolve_additional_edits_lazily(), - ) + .add_import(import_to_add) .build(); return Some(item); } @@ -248,7 +245,7 @@ impl<'a> Render<'a> { let item = item .kind(kind) - .add_import(import_to_add, self.ctx.completion.config.resolve_additional_edits_lazily()) + .add_import(import_to_add) .set_documentation(docs) .set_ref_match(ref_match) .build(); @@ -449,28 +446,6 @@ fn main() { let _: m::Spam = S<|> } insert: "m", kind: Module, }, - CompletionItem { - label: "m::Spam", - source_range: 75..76, - text_edit: TextEdit { - indels: [ - Indel { - insert: "use m::Spam;", - delete: 0..0, - }, - Indel { - insert: "\n\n", - delete: 0..0, - }, - Indel { - insert: "Spam", - delete: 75..76, - }, - ], - }, - kind: Enum, - lookup: "Spam", - }, CompletionItem { label: "m::Spam::Foo", source_range: 75..76, diff --git a/crates/completion/src/render/enum_variant.rs b/crates/completion/src/render/enum_variant.rs index e979a090b..8e0fea6c0 100644 --- a/crates/completion/src/render/enum_variant.rs +++ b/crates/completion/src/render/enum_variant.rs @@ -71,7 +71,7 @@ impl<'a> EnumVariantRender<'a> { .kind(CompletionItemKind::EnumVariant) .set_documentation(self.variant.docs(self.ctx.db())) .set_deprecated(self.ctx.is_deprecated(self.variant)) - .add_import(import_to_add, self.ctx.completion.config.resolve_additional_edits_lazily()) + .add_import(import_to_add) .detail(self.detail()); if self.variant_kind == StructKind::Tuple { diff --git a/crates/completion/src/render/function.rs b/crates/completion/src/render/function.rs index dd2c999ef..d16005249 100644 --- a/crates/completion/src/render/function.rs +++ b/crates/completion/src/render/function.rs @@ -47,7 +47,7 @@ impl<'a> FunctionRender<'a> { .set_deprecated(self.ctx.is_deprecated(self.func)) .detail(self.detail()) .add_call_parens(self.ctx.completion, self.name, params) - .add_import(import_to_add, self.ctx.completion.config.resolve_additional_edits_lazily()) + .add_import(import_to_add) .build() } diff --git a/crates/completion/src/render/macro_.rs b/crates/completion/src/render/macro_.rs index bdbc642ca..eb3209bee 100644 --- a/crates/completion/src/render/macro_.rs +++ b/crates/completion/src/render/macro_.rs @@ -50,10 +50,7 @@ impl<'a> MacroRender<'a> { .kind(CompletionItemKind::Macro) .set_documentation(self.docs.clone()) .set_deprecated(self.ctx.is_deprecated(self.macro_)) - .add_import( - import_to_add, - self.ctx.completion.config.resolve_additional_edits_lazily(), - ) + .add_import(import_to_add) .detail(self.detail()); let needs_bang = self.needs_bang(); diff --git a/crates/completion/src/test_utils.rs b/crates/completion/src/test_utils.rs index 4c1b1a839..25f5f4924 100644 --- a/crates/completion/src/test_utils.rs +++ b/crates/completion/src/test_utils.rs @@ -96,7 +96,16 @@ pub(crate) fn check_edit_with_config( .collect_tuple() .unwrap_or_else(|| panic!("can't find {:?} completion in {:#?}", what, completions)); let mut actual = db.file_text(position.file_id).to_string(); - completion.text_edit().apply(&mut actual); + + let mut combined_edit = completion.text_edit().to_owned(); + if let Some(import_text_edit) = completion.import_to_add().and_then(|edit| edit.to_text_edit()) + { + combined_edit.union(import_text_edit).expect( + "Failed to apply completion resolve changes: change ranges overlap, but should not", + ) + } + + combined_edit.apply(&mut actual); assert_eq_text!(&ra_fixture_after, &actual) } -- cgit v1.2.3