Subject: [PATCH] Small fixes Fix the test Less clones Resolve completion items Omit completion fields to be resolved later Prepare for omittiong parts of completion data that need to be resolved --- Index: crates/ide-completion/src/config.rs IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/crates/ide-completion/src/config.rs b/crates/ide-completion/src/config.rs --- a/crates/ide-completion/src/config.rs (revision 2395a4e9325cb94300e17a023f162d942391ec53) +++ b/crates/ide-completion/src/config.rs (revision 00037c18ed8b57eac49b38495689d8a6b7f53ca3) @@ -7,7 +7,7 @@ use hir::ImportPathConfig; use ide_db::{imports::insert_use::InsertUseConfig, SnippetCap}; -use crate::{snippet::Snippet, CompletionFieldsToResolve}; +use crate::snippet::Snippet; #[derive(Clone, Debug, PartialEq, Eq)] pub struct CompletionConfig { @@ -27,7 +27,6 @@ pub prefer_absolute: bool, pub snippets: Vec, pub limit: Option, - pub fields_to_resolve: CompletionFieldsToResolve, } #[derive(Clone, Debug, PartialEq, Eq)] Index: crates/ide-completion/src/lib.rs IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/crates/ide-completion/src/lib.rs b/crates/ide-completion/src/lib.rs --- a/crates/ide-completion/src/lib.rs (revision ba1c9146fcca65fa9c40d1e4e73bdcb898a8de46) +++ b/crates/ide-completion/src/lib.rs (revision 00037c18ed8b57eac49b38495689d8a6b7f53ca3) @@ -37,31 +37,6 @@ snippet::{Snippet, SnippetScope}, }; -#[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub struct CompletionFieldsToResolve { - pub resolve_label_details: bool, - pub resolve_tags: bool, - pub resolve_detail: bool, - pub resolve_documentation: bool, - pub resolve_filter_text: bool, - pub resolve_text_edit: bool, - pub resolve_command: bool, -} - -impl CompletionFieldsToResolve { - pub const fn empty() -> Self { - Self { - resolve_label_details: false, - resolve_tags: false, - resolve_detail: false, - resolve_documentation: false, - resolve_filter_text: false, - resolve_text_edit: false, - resolve_command: false, - } - } -} - //FIXME: split the following feature into fine-grained features. // Feature: Magic Completions Index: crates/ide-completion/src/tests.rs IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/crates/ide-completion/src/tests.rs b/crates/ide-completion/src/tests.rs --- a/crates/ide-completion/src/tests.rs (revision 2395a4e9325cb94300e17a023f162d942391ec53) +++ b/crates/ide-completion/src/tests.rs (revision 00037c18ed8b57eac49b38495689d8a6b7f53ca3) @@ -37,8 +37,8 @@ use test_utils::assert_eq_text; use crate::{ - resolve_completion_edits, CallableSnippets, CompletionConfig, CompletionFieldsToResolve, - CompletionItem, CompletionItemKind, + resolve_completion_edits, CallableSnippets, CompletionConfig, CompletionItem, + CompletionItemKind, }; /// Lots of basic item definitions @@ -84,7 +84,6 @@ prefer_absolute: false, snippets: Vec::new(), limit: None, - fields_to_resolve: CompletionFieldsToResolve::empty(), }; pub(crate) fn completion_list(ra_fixture: &str) -> String { Index: crates/ide/src/lib.rs IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs --- a/crates/ide/src/lib.rs (revision 2395a4e9325cb94300e17a023f162d942391ec53) +++ b/crates/ide/src/lib.rs (revision 00037c18ed8b57eac49b38495689d8a6b7f53ca3) @@ -119,8 +119,8 @@ Assist, AssistConfig, AssistId, AssistKind, AssistResolveStrategy, SingleResolve, }; pub use ide_completion::{ - CallableSnippets, CompletionConfig, CompletionFieldsToResolve, CompletionItem, - CompletionItemKind, CompletionRelevance, Snippet, SnippetScope, + CallableSnippets, CompletionConfig, CompletionItem, CompletionItemKind, CompletionRelevance, + Snippet, SnippetScope, }; pub use ide_db::{ base_db::{Cancelled, CrateGraph, CrateId, FileChange, SourceRoot, SourceRootId}, Index: crates/rust-analyzer/src/config.rs IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs --- a/crates/rust-analyzer/src/config.rs (revision ba1c9146fcca65fa9c40d1e4e73bdcb898a8de46) +++ b/crates/rust-analyzer/src/config.rs (revision 00037c18ed8b57eac49b38495689d8a6b7f53ca3) @@ -12,10 +12,10 @@ use cfg::{CfgAtom, CfgDiff}; use hir::Symbol; use ide::{ - AssistConfig, CallableSnippets, CompletionConfig, CompletionFieldsToResolve, DiagnosticsConfig, - ExprFillDefaultMode, GenericParameterHints, HighlightConfig, HighlightRelatedConfig, - HoverConfig, HoverDocFormat, InlayFieldsToResolve, InlayHintsConfig, JoinLinesConfig, - MemoryLayoutHoverConfig, MemoryLayoutHoverRenderKind, Snippet, SnippetScope, SourceRootId, + AssistConfig, CallableSnippets, CompletionConfig, DiagnosticsConfig, ExprFillDefaultMode, + GenericParameterHints, HighlightConfig, HighlightRelatedConfig, HoverConfig, HoverDocFormat, + InlayFieldsToResolve, InlayHintsConfig, JoinLinesConfig, MemoryLayoutHoverConfig, + MemoryLayoutHoverRenderKind, Snippet, SnippetScope, SourceRootId, }; use ide_db::{ imports::insert_use::{ImportGranularity, InsertUseConfig, PrefixKind}, @@ -1391,7 +1391,6 @@ } pub fn completion(&self, source_root: Option) -> CompletionConfig { - let client_capability_fields = self.completion_resolve_support_properties(); CompletionConfig { enable_postfix_completions: self.completion_postfix_enable(source_root).to_owned(), enable_imports_on_the_fly: self.completion_autoimport_enable(source_root).to_owned() @@ -1416,15 +1415,6 @@ limit: self.completion_limit(source_root).to_owned(), enable_term_search: self.completion_termSearch_enable(source_root).to_owned(), term_search_fuel: self.completion_termSearch_fuel(source_root).to_owned() as u64, - fields_to_resolve: CompletionFieldsToResolve { - resolve_label_details: client_capability_fields.contains("labelDetails"), - resolve_tags: client_capability_fields.contains("tags"), - resolve_detail: client_capability_fields.contains("detail"), - resolve_documentation: client_capability_fields.contains("documentation"), - resolve_filter_text: client_capability_fields.contains("filterText"), - resolve_text_edit: client_capability_fields.contains("textEdit"), - resolve_command: client_capability_fields.contains("command"), - }, } } Index: crates/rust-analyzer/src/handlers/request.rs IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs --- a/crates/rust-analyzer/src/handlers/request.rs (revision c03f5b63e2975b753d5dbfc29640b533176a9610) +++ b/crates/rust-analyzer/src/handlers/request.rs (revision 00037c18ed8b57eac49b38495689d8a6b7f53ca3) @@ -10,9 +10,9 @@ use anyhow::Context; use ide::{ - AnnotationConfig, AssistKind, AssistResolveStrategy, Cancellable, CompletionFieldsToResolve, - FilePosition, FileRange, HoverAction, HoverGotoTypeData, InlayFieldsToResolve, Query, - RangeInfo, ReferenceCategory, Runnable, RunnableKind, SingleResolve, SourceChange, TextEdit, + AnnotationConfig, AssistKind, AssistResolveStrategy, Cancellable, FilePosition, FileRange, + HoverAction, HoverGotoTypeData, InlayFieldsToResolve, Query, RangeInfo, ReferenceCategory, + Runnable, RunnableKind, SingleResolve, SourceChange, TextEdit, }; use ide_db::SymbolKind; use itertools::Itertools; @@ -1019,11 +1019,9 @@ let items = to_proto::completion_items( &snap.config, - &completion_config.fields_to_resolve, &line_index, snap.file_version(position.file_id), text_document_position, - completion_trigger_character, items, ); @@ -1056,70 +1054,36 @@ }; let source_root = snap.analysis.source_root_id(file_id)?; - let mut forced_resolve_completions_config = snap.config.completion(Some(source_root)); - forced_resolve_completions_config.fields_to_resolve = CompletionFieldsToResolve::empty(); - - let position = FilePosition { file_id, offset }; - let Some(resolved_completions) = snap.analysis.completions( - &forced_resolve_completions_config, - position, - resolve_data.trigger_character, - )? - else { - return Ok(original_completion); - }; - let resolved_completions = to_proto::completion_items( - &snap.config, - &forced_resolve_completions_config.fields_to_resolve, - &line_index, - snap.file_version(position.file_id), - resolve_data.position, - resolve_data.trigger_character, - resolved_completions, - ); - let Some(mut resolved_completion) = resolved_completions.into_iter().find(|completion| { - completion.label == original_completion.label - && completion.kind == original_completion.kind - && completion.deprecated == original_completion.deprecated - && completion.preselect == original_completion.preselect - && completion.sort_text == original_completion.sort_text - }) else { - return Ok(original_completion); - }; - - if !resolve_data.imports.is_empty() { - let additional_edits = snap - .analysis - .resolve_completion_edits( - &forced_resolve_completions_config, - position, - resolve_data - .imports - .into_iter() - .map(|import| (import.full_import_path, import.imported_name)), - )? - .into_iter() - .flat_map(|edit| edit.into_iter().map(|indel| to_proto::text_edit(&line_index, indel))) - .collect::>(); + let additional_edits = snap + .analysis + .resolve_completion_edits( + &snap.config.completion(Some(source_root)), + FilePosition { file_id, offset }, + resolve_data + .imports + .into_iter() + .map(|import| (import.full_import_path, import.imported_name)), + )? + .into_iter() + .flat_map(|edit| edit.into_iter().map(|indel| to_proto::text_edit(&line_index, indel))) + .collect::>(); - if !all_edits_are_disjoint(&resolved_completion, &additional_edits) { - return Err(LspError::new( - ErrorCode::InternalError as i32, - "Import edit overlaps with the original completion edits, this is not LSP-compliant" - .into(), - ) - .into()); - } + if !all_edits_are_disjoint(&original_completion, &additional_edits) { + return Err(LspError::new( + ErrorCode::InternalError as i32, + "Import edit overlaps with the original completion edits, this is not LSP-compliant" + .into(), + ) + .into()); + } - if let Some(original_additional_edits) = resolved_completion.additional_text_edits.as_mut() - { - original_additional_edits.extend(additional_edits) - } else { - resolved_completion.additional_text_edits = Some(additional_edits); - } + if let Some(original_additional_edits) = original_completion.additional_text_edits.as_mut() { + original_additional_edits.extend(additional_edits) + } else { + original_completion.additional_text_edits = Some(additional_edits); } - Ok(resolved_completion) + Ok(original_completion) } pub(crate) fn handle_folding_range( Index: crates/rust-analyzer/src/integrated_benchmarks.rs IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/crates/rust-analyzer/src/integrated_benchmarks.rs b/crates/rust-analyzer/src/integrated_benchmarks.rs --- a/crates/rust-analyzer/src/integrated_benchmarks.rs (revision 2395a4e9325cb94300e17a023f162d942391ec53) +++ b/crates/rust-analyzer/src/integrated_benchmarks.rs (revision 00037c18ed8b57eac49b38495689d8a6b7f53ca3) @@ -12,8 +12,7 @@ use hir::ChangeWithProcMacros; use ide::{ - AnalysisHost, CallableSnippets, CompletionConfig, CompletionFieldsToResolve, DiagnosticsConfig, - FilePosition, TextSize, + AnalysisHost, CallableSnippets, CompletionConfig, DiagnosticsConfig, FilePosition, TextSize, }; use ide_db::{ imports::insert_use::{ImportGranularity, InsertUseConfig}, @@ -169,7 +168,6 @@ snippets: Vec::new(), limit: None, add_semicolon_to_unit: true, - fields_to_resolve: CompletionFieldsToResolve::empty(), }; let position = FilePosition { file_id, offset: TextSize::try_from(completion_offset).unwrap() }; @@ -217,7 +215,6 @@ snippets: Vec::new(), limit: None, add_semicolon_to_unit: true, - fields_to_resolve: CompletionFieldsToResolve::empty(), }; let position = FilePosition { file_id, offset: TextSize::try_from(completion_offset).unwrap() }; @@ -263,7 +260,6 @@ snippets: Vec::new(), limit: None, add_semicolon_to_unit: true, - fields_to_resolve: CompletionFieldsToResolve::empty(), }; let position = FilePosition { file_id, offset: TextSize::try_from(completion_offset).unwrap() }; Index: crates/rust-analyzer/src/lsp/capabilities.rs IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/crates/rust-analyzer/src/lsp/capabilities.rs b/crates/rust-analyzer/src/lsp/capabilities.rs --- a/crates/rust-analyzer/src/lsp/capabilities.rs (revision 983f2abc2ccc53d440f58a5acd8b08c50cd0b7eb) +++ b/crates/rust-analyzer/src/lsp/capabilities.rs (revision 00037c18ed8b57eac49b38495689d8a6b7f53ca3) @@ -448,7 +448,7 @@ .unwrap_or_default() } - pub fn inlay_hint_resolve_support_properties(&self) -> FxHashSet<&str> { + pub fn inlay_hint_resolve_support_properties(&self) -> FxHashSet { self.0 .text_document .as_ref() @@ -457,22 +457,8 @@ .map(|inlay_resolve| inlay_resolve.properties.iter()) .into_iter() .flatten() - .map(|s| s.as_str()) - .collect() - } - - pub fn completion_resolve_support_properties(&self) -> FxHashSet<&str> { - self.0 - .text_document - .as_ref() - .and_then(|text| text.completion.as_ref()) - .and_then(|completion_caps| completion_caps.completion_item.as_ref()) - .and_then(|completion_item_caps| completion_item_caps.resolve_support.as_ref()) - .map(|resolve_support| resolve_support.properties.iter()) - .into_iter() - .flatten() - .map(|s| s.as_str()) - .collect() + .cloned() + .collect::>() } pub fn hover_markdown_support(&self) -> bool { Index: crates/rust-analyzer/src/lsp/ext.rs IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/crates/rust-analyzer/src/lsp/ext.rs b/crates/rust-analyzer/src/lsp/ext.rs --- a/crates/rust-analyzer/src/lsp/ext.rs (revision 950bb83f818342d9f9ed472c1d30492db324f5a6) +++ b/crates/rust-analyzer/src/lsp/ext.rs (revision 00037c18ed8b57eac49b38495689d8a6b7f53ca3) @@ -825,7 +825,6 @@ pub position: lsp_types::TextDocumentPositionParams, pub imports: Vec, pub version: Option, - pub trigger_character: Option, } #[derive(Debug, Serialize, Deserialize)] Index: crates/rust-analyzer/src/lsp/to_proto.rs IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/crates/rust-analyzer/src/lsp/to_proto.rs b/crates/rust-analyzer/src/lsp/to_proto.rs --- a/crates/rust-analyzer/src/lsp/to_proto.rs (revision c03f5b63e2975b753d5dbfc29640b533176a9610) +++ b/crates/rust-analyzer/src/lsp/to_proto.rs (revision 00037c18ed8b57eac49b38495689d8a6b7f53ca3) @@ -6,9 +6,9 @@ }; use ide::{ - Annotation, AnnotationKind, Assist, AssistKind, Cancellable, CompletionFieldsToResolve, - CompletionItem, CompletionItemKind, CompletionRelevance, Documentation, FileId, FileRange, - FileSystemEdit, Fold, FoldKind, Highlight, HlMod, HlOperator, HlPunct, HlRange, HlTag, Indel, + Annotation, AnnotationKind, Assist, AssistKind, Cancellable, CompletionItem, + CompletionItemKind, CompletionRelevance, Documentation, FileId, FileRange, FileSystemEdit, + Fold, FoldKind, Highlight, HlMod, HlOperator, HlPunct, HlRange, HlTag, Indel, InlayFieldsToResolve, InlayHint, InlayHintLabel, InlayHintLabelPart, InlayKind, Markup, NavigationTarget, ReferenceCategory, RenameError, Runnable, Severity, SignatureHelp, SnippetEdit, SourceChange, StructureNodeKind, SymbolKind, TextEdit, TextRange, TextSize, @@ -227,11 +227,9 @@ pub(crate) fn completion_items( config: &Config, - fields_to_resolve: &CompletionFieldsToResolve, line_index: &LineIndex, version: Option, tdpp: lsp_types::TextDocumentPositionParams, - completion_trigger_character: Option, mut items: Vec, ) -> Vec { if config.completion_hide_deprecated() { @@ -241,17 +239,7 @@ let max_relevance = items.iter().map(|it| it.relevance.score()).max().unwrap_or_default(); let mut res = Vec::with_capacity(items.len()); for item in items { - completion_item( - &mut res, - config, - fields_to_resolve, - line_index, - version, - &tdpp, - max_relevance, - completion_trigger_character, - item, - ); + completion_item(&mut res, config, line_index, version, &tdpp, max_relevance, item); } if let Some(limit) = config.completion(None).limit { @@ -265,33 +253,21 @@ fn completion_item( acc: &mut Vec, config: &Config, - fields_to_resolve: &CompletionFieldsToResolve, line_index: &LineIndex, version: Option, tdpp: &lsp_types::TextDocumentPositionParams, max_relevance: u32, - completion_trigger_character: Option, item: CompletionItem, ) { let insert_replace_support = config.insert_replace_support().then_some(tdpp.position); let ref_match = item.ref_match(); + let lookup = item.lookup().to_owned(); let mut additional_text_edits = Vec::new(); - let mut something_to_resolve = false; - let filter_text = if fields_to_resolve.resolve_filter_text { - something_to_resolve = !item.lookup().is_empty(); - None - } else { - Some(item.lookup().to_owned()) - }; - - let text_edit = if fields_to_resolve.resolve_text_edit { - something_to_resolve = true; - None - } else { - // LSP does not allow arbitrary edits in completion, so we have to do a - // non-trivial mapping here. + // LSP does not allow arbitrary edits in completion, so we have to do a + // non-trivial mapping here. + let text_edit = { let mut text_edit = None; let source_range = item.source_range; for indel in item.text_edit { @@ -314,49 +290,25 @@ additional_text_edits.push(text_edit); } } - Some(text_edit.unwrap()) + text_edit.unwrap() }; let insert_text_format = item.is_snippet.then_some(lsp_types::InsertTextFormat::SNIPPET); - let tags = if fields_to_resolve.resolve_tags { - something_to_resolve = item.deprecated; - None - } else { - item.deprecated.then(|| vec![lsp_types::CompletionItemTag::DEPRECATED]) - }; + let tags = item.deprecated.then(|| vec![lsp_types::CompletionItemTag::DEPRECATED]); let command = if item.trigger_call_info && config.client_commands().trigger_parameter_hints { - if fields_to_resolve.resolve_command { - something_to_resolve = true; - None - } else { - Some(command::trigger_parameter_hints()) - } + Some(command::trigger_parameter_hints()) } else { None }; - let detail = if fields_to_resolve.resolve_detail { - something_to_resolve = item.detail.is_some(); - None - } else { - item.detail - }; - - let documentation = if fields_to_resolve.resolve_documentation { - something_to_resolve = item.documentation.is_some(); - None - } else { - item.documentation.map(documentation) - }; - let mut lsp_item = lsp_types::CompletionItem { label: item.label.to_string(), - detail, - filter_text, + detail: item.detail, + filter_text: Some(lookup), kind: Some(completion_item_kind(item.kind)), - text_edit, + text_edit: Some(text_edit), additional_text_edits: Some(additional_text_edits), - documentation, + documentation: item.documentation.map(documentation), deprecated: Some(item.deprecated), tags, command, @@ -365,40 +317,29 @@ }; if config.completion_label_details_support() { - if fields_to_resolve.resolve_label_details { - something_to_resolve = true; - } else { - lsp_item.label_details = Some(lsp_types::CompletionItemLabelDetails { - detail: item.label_detail.as_ref().map(ToString::to_string), - description: lsp_item.detail.clone(), - }); - } + lsp_item.label_details = Some(lsp_types::CompletionItemLabelDetails { + detail: item.label_detail.as_ref().map(ToString::to_string), + description: lsp_item.detail.clone(), + }); } else if let Some(label_detail) = item.label_detail { lsp_item.label.push_str(label_detail.as_str()); } set_score(&mut lsp_item, max_relevance, item.relevance); - let imports = - if config.completion(None).enable_imports_on_the_fly && !item.import_to_add.is_empty() { - item.import_to_add - .into_iter() - .map(|(import_path, import_name)| lsp_ext::CompletionImport { - full_import_path: import_path, - imported_name: import_name, - }) - .collect() - } else { - Vec::new() - }; - if something_to_resolve || !imports.is_empty() { - let data = lsp_ext::CompletionResolveData { - position: tdpp.clone(), - imports, - version, - trigger_character: completion_trigger_character, - }; - lsp_item.data = Some(to_value(data).unwrap()); + if config.completion(None).enable_imports_on_the_fly && !item.import_to_add.is_empty() { + let imports = item + .import_to_add + .into_iter() + .map(|(import_path, import_name)| lsp_ext::CompletionImport { + full_import_path: import_path, + imported_name: import_name, + }) + .collect::>(); + if !imports.is_empty() { + let data = lsp_ext::CompletionResolveData { position: tdpp.clone(), imports, version }; + lsp_item.data = Some(to_value(data).unwrap()); + } } if let Some((label, indel, relevance)) = ref_match { Index: docs/dev/lsp-extensions.md IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md --- a/docs/dev/lsp-extensions.md (revision 01ad45328574bcda58189b12cb8c5642cc387809) +++ b/docs/dev/lsp-extensions.md (revision 983f2abc2ccc53d440f58a5acd8b08c50cd0b7eb) @@ -1,5 +1,5 @@