Auto merge of #18167 - SomeoneToIgnore:fat-completions, r=Veykril

internal: Send less data during `textDocument/completion` if possible Similar to https://github.com/rust-lang/rust-analyzer/pull/15522, stops sending extra data during `textDocument/completion` if that data was set in the client completions resolve capabilities, and sends those only during `completionItem/resolve` requests. Currently, rust-analyzer sends back all fields (including potentially huge docs) for every completion item which might get large. Same as the other one, this PR aims to keep the changes minimal and does not remove extra computations for such fields — instead, it just filters them out before sending to the client. The PR omits primitive, boolean and integer, types such as `deprecated`, `preselect`, `insertTextFormat`, `insertTextMode`, etc. AND `additionalTextEdits` — this one looks very dangerous to compute for each completion item (as the spec says we ought to if there's no corresponding resolve capabilities provided) due to the diff computations and the fact that this code had been in the resolution for some time. It would be good to resolve this lazily too, please let me know if it's ok to do. When tested with Zed which only defines `documentation` and `additionalTextEdits` in its client completion resolve capabilities, rust-analyzer starts to send almost 3 times less characters: Request: ```json {"jsonrpc":"2.0","id":104,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///Users/someonetoignore/work/rust-analyzer/crates/ide/src/inlay_hints.rs"},"position":{"line":90,"character":14},"context":{"triggerKind":1}}} ``` <img width="1338" alt="image" src="https://github.com/user-attachments/assets/104f19b5-7095-4fc1-b008-5d829623b2e2"> Before: 381944 characters [before.json](https://github.com/user-attachments/files/17092385/before.json) After: 140503 characters [after.json](https://github.com/user-attachments/files/17092386/after.json) After Zed's [patch](https://github.com/zed-industries/zed/pull/18212) to enable all resolving possible: 84452 characters [after-after.json](https://github.com/user-attachments/files/17092755/after-after.json)
2024-12-25 12:33:33 +00:00 · 2024-09-30 08:36:54 +00:00 · 2024-09-30 08:36:54 +00:00 · dfe6d503b8
commit dfe6d503b8
parent 7b60339273 c03f5b63e2
11 changed files with 227 additions and 76 deletions
--- a/crates/ide-completion/src/config.rs
+++ b/crates/ide-completion/src/config.rs
@ -7,7 +7,7 @@
 use hir::ImportPathConfig;
 use ide_db::{imports::insert_use::InsertUseConfig, SnippetCap};

-use crate::snippet::Snippet;
+use crate::{snippet::Snippet, CompletionFieldsToResolve};

 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct CompletionConfig {
@ -27,6 +27,7 @@ pub struct CompletionConfig {
    pub prefer_absolute: bool,
    pub snippets: Vec<Snippet>,
    pub limit: Option<usize>,
+    pub fields_to_resolve: CompletionFieldsToResolve,
 }

 #[derive(Clone, Debug, PartialEq, Eq)]
--- a/crates/ide-completion/src/lib.rs
+++ b/crates/ide-completion/src/lib.rs
@ -37,6 +37,31 @@ pub use crate::{
    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
--- a/crates/ide-completion/src/tests.rs
+++ b/crates/ide-completion/src/tests.rs
@ -37,8 +37,8 @@ use test_fixture::ChangeFixture;
 use test_utils::assert_eq_text;

 use crate::{
-    resolve_completion_edits, CallableSnippets, CompletionConfig, CompletionItem,
-    CompletionItemKind,
+    resolve_completion_edits, CallableSnippets, CompletionConfig, CompletionFieldsToResolve,
+    CompletionItem, CompletionItemKind,
 };

 /// Lots of basic item definitions
@ -84,6 +84,7 @@ pub(crate) const TEST_CONFIG: CompletionConfig = CompletionConfig {
    prefer_absolute: false,
    snippets: Vec::new(),
    limit: None,
+    fields_to_resolve: CompletionFieldsToResolve::empty(),
 };

 pub(crate) fn completion_list(ra_fixture: &str) -> String {
--- a/crates/ide/src/lib.rs
+++ b/crates/ide/src/lib.rs
@ -119,8 +119,8 @@ pub use ide_assists::{
    Assist, AssistConfig, AssistId, AssistKind, AssistResolveStrategy, SingleResolve,
 };
 pub use ide_completion::{
-    CallableSnippets, CompletionConfig, CompletionItem, CompletionItemKind, CompletionRelevance,
-    Snippet, SnippetScope,
+    CallableSnippets, CompletionConfig, CompletionFieldsToResolve, CompletionItem,
+    CompletionItemKind, CompletionRelevance, Snippet, SnippetScope,
 };
 pub use ide_db::{
    base_db::{Cancelled, CrateGraph, CrateId, FileChange, SourceRoot, SourceRootId},
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@ -12,10 +12,10 @@ use std::{
 use cfg::{CfgAtom, CfgDiff};
 use hir::Symbol;
 use ide::{
-    AssistConfig, CallableSnippets, CompletionConfig, DiagnosticsConfig, ExprFillDefaultMode,
-    GenericParameterHints, HighlightConfig, HighlightRelatedConfig, HoverConfig, HoverDocFormat,
-    InlayFieldsToResolve, InlayHintsConfig, JoinLinesConfig, MemoryLayoutHoverConfig,
-    MemoryLayoutHoverRenderKind, Snippet, SnippetScope, SourceRootId,
+    AssistConfig, CallableSnippets, CompletionConfig, CompletionFieldsToResolve, DiagnosticsConfig,
+    ExprFillDefaultMode, GenericParameterHints, HighlightConfig, HighlightRelatedConfig,
+    HoverConfig, HoverDocFormat, InlayFieldsToResolve, InlayHintsConfig, JoinLinesConfig,
+    MemoryLayoutHoverConfig, MemoryLayoutHoverRenderKind, Snippet, SnippetScope, SourceRootId,
 };
 use ide_db::{
    imports::insert_use::{ImportGranularity, InsertUseConfig, PrefixKind},
@ -1393,6 +1393,7 @@ impl Config {
    }

    pub fn completion(&self, source_root: Option<SourceRootId>) -> 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()
@ -1417,6 +1418,15 @@ impl Config {
            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"),
+            },
        }
    }

--- a/crates/rust-analyzer/src/handlers/request.rs
+++ b/crates/rust-analyzer/src/handlers/request.rs
@ -10,9 +10,9 @@ use std::{
 use anyhow::Context;

 use ide::{
-    AnnotationConfig, AssistKind, AssistResolveStrategy, Cancellable, FilePosition, FileRange,
-    HoverAction, HoverGotoTypeData, InlayFieldsToResolve, Query, RangeInfo, ReferenceCategory,
-    Runnable, RunnableKind, SingleResolve, SourceChange, TextEdit,
+    AnnotationConfig, AssistKind, AssistResolveStrategy, Cancellable, CompletionFieldsToResolve,
+    FilePosition, FileRange, HoverAction, HoverGotoTypeData, InlayFieldsToResolve, Query,
+    RangeInfo, ReferenceCategory, Runnable, RunnableKind, SingleResolve, SourceChange, TextEdit,
 };
 use ide_db::SymbolKind;
 use itertools::Itertools;
@ -1019,9 +1019,11 @@ pub(crate) fn handle_completion(

    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,
    );

@ -1054,36 +1056,70 @@ pub(crate) fn handle_completion_resolve(
    };
    let source_root = snap.analysis.source_root_id(file_id)?;

-    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::<Vec<_>>();
+    let mut forced_resolve_completions_config = snap.config.completion(Some(source_root));
+    forced_resolve_completions_config.fields_to_resolve = CompletionFieldsToResolve::empty();

-    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());
+    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::<Vec<_>>();
+
+        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 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(original_completion)
+    Ok(resolved_completion)
 }

 pub(crate) fn handle_folding_range(
--- a/crates/rust-analyzer/src/integrated_benchmarks.rs
+++ b/crates/rust-analyzer/src/integrated_benchmarks.rs
@ -12,7 +12,8 @@

 use hir::ChangeWithProcMacros;
 use ide::{
-    AnalysisHost, CallableSnippets, CompletionConfig, DiagnosticsConfig, FilePosition, TextSize,
+    AnalysisHost, CallableSnippets, CompletionConfig, CompletionFieldsToResolve, DiagnosticsConfig,
+    FilePosition, TextSize,
 };
 use ide_db::{
    imports::insert_use::{ImportGranularity, InsertUseConfig},
@ -172,6 +173,7 @@ fn integrated_completion_benchmark() {
            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() };
@ -219,6 +221,7 @@ fn integrated_completion_benchmark() {
            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() };
@ -264,6 +267,7 @@ fn integrated_completion_benchmark() {
            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() };
--- a/crates/rust-analyzer/src/lsp/capabilities.rs
+++ b/crates/rust-analyzer/src/lsp/capabilities.rs
@ -448,7 +448,7 @@ impl ClientCapabilities {
            .unwrap_or_default()
    }

-    pub fn inlay_hint_resolve_support_properties(&self) -> FxHashSet<String> {
+    pub fn inlay_hint_resolve_support_properties(&self) -> FxHashSet<&str> {
        self.0
            .text_document
            .as_ref()
@ -457,8 +457,22 @@ impl ClientCapabilities {
            .map(|inlay_resolve| inlay_resolve.properties.iter())
            .into_iter()
            .flatten()
-            .cloned()
-            .collect::<FxHashSet<_>>()
+            .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()
    }

    pub fn hover_markdown_support(&self) -> bool {
--- a/crates/rust-analyzer/src/lsp/ext.rs
+++ b/crates/rust-analyzer/src/lsp/ext.rs
@ -825,6 +825,7 @@ pub struct CompletionResolveData {
    pub position: lsp_types::TextDocumentPositionParams,
    pub imports: Vec<CompletionImport>,
    pub version: Option<i32>,
+    pub trigger_character: Option<char>,
 }

 #[derive(Debug, Serialize, Deserialize)]
--- a/crates/rust-analyzer/src/lsp/to_proto.rs
+++ b/crates/rust-analyzer/src/lsp/to_proto.rs
@ -6,9 +6,9 @@ use std::{
 };

 use ide::{
-    Annotation, AnnotationKind, Assist, AssistKind, Cancellable, CompletionItem,
-    CompletionItemKind, CompletionRelevance, Documentation, FileId, FileRange, FileSystemEdit,
-    Fold, FoldKind, Highlight, HlMod, HlOperator, HlPunct, HlRange, HlTag, Indel,
+    Annotation, AnnotationKind, Assist, AssistKind, Cancellable, CompletionFieldsToResolve,
+    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,9 +227,11 @@ pub(crate) fn snippet_text_edit_vec(

 pub(crate) fn completion_items(
    config: &Config,
+    fields_to_resolve: &CompletionFieldsToResolve,
    line_index: &LineIndex,
    version: Option<i32>,
    tdpp: lsp_types::TextDocumentPositionParams,
+    completion_trigger_character: Option<char>,
    mut items: Vec<CompletionItem>,
 ) -> Vec<lsp_types::CompletionItem> {
    if config.completion_hide_deprecated() {
@ -239,7 +241,17 @@ pub(crate) fn completion_items(
    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, line_index, version, &tdpp, max_relevance, item);
+        completion_item(
+            &mut res,
+            config,
+            fields_to_resolve,
+            line_index,
+            version,
+            &tdpp,
+            max_relevance,
+            completion_trigger_character,
+            item,
+        );
    }

    if let Some(limit) = config.completion(None).limit {
@ -253,21 +265,33 @@ pub(crate) fn completion_items(
 fn completion_item(
    acc: &mut Vec<lsp_types::CompletionItem>,
    config: &Config,
+    fields_to_resolve: &CompletionFieldsToResolve,
    line_index: &LineIndex,
    version: Option<i32>,
    tdpp: &lsp_types::TextDocumentPositionParams,
    max_relevance: u32,
+    completion_trigger_character: Option<char>,
    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;

-    // LSP does not allow arbitrary edits in completion, so we have to do a
-    // non-trivial mapping here.
-    let text_edit = {
+    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.
        let mut text_edit = None;
        let source_range = item.source_range;
        for indel in item.text_edit {
@ -290,25 +314,49 @@ fn completion_item(
                additional_text_edits.push(text_edit);
            }
        }
-        text_edit.unwrap()
+        Some(text_edit.unwrap())
    };

    let insert_text_format = item.is_snippet.then_some(lsp_types::InsertTextFormat::SNIPPET);
-    let tags = item.deprecated.then(|| vec![lsp_types::CompletionItemTag::DEPRECATED]);
+    let tags = if fields_to_resolve.resolve_tags {
+        something_to_resolve = item.deprecated;
+        None
+    } else {
+        item.deprecated.then(|| vec![lsp_types::CompletionItemTag::DEPRECATED])
+    };
    let command = if item.trigger_call_info && config.client_commands().trigger_parameter_hints {
-        Some(command::trigger_parameter_hints())
+        if fields_to_resolve.resolve_command {
+            something_to_resolve = true;
+            None
+        } else {
+            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: item.detail,
-        filter_text: Some(lookup),
+        detail,
+        filter_text,
        kind: Some(completion_item_kind(item.kind)),
-        text_edit: Some(text_edit),
+        text_edit,
        additional_text_edits: Some(additional_text_edits),
-        documentation: item.documentation.map(documentation),
+        documentation,
        deprecated: Some(item.deprecated),
        tags,
        command,
@ -317,29 +365,40 @@ fn completion_item(
    };

    if config.completion_label_details_support() {
-        lsp_item.label_details = Some(lsp_types::CompletionItemLabelDetails {
-            detail: item.label_detail.as_ref().map(ToString::to_string),
-            description: lsp_item.detail.clone(),
-        });
+        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(),
+            });
+        }
    } 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);

-    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::<Vec<_>>();
-        if !imports.is_empty() {
-            let data = lsp_ext::CompletionResolveData { position: tdpp.clone(), imports, version };
-            lsp_item.data = Some(to_value(data).unwrap());
-        }
+    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 let Some((label, indel, relevance)) = ref_match {
--- a/docs/dev/lsp-extensions.md
+++ b/docs/dev/lsp-extensions.md
@ -1,5 +1,5 @@
 <!---
-lsp/ext.rs hash: 6292ee8d88d4c9ec
+lsp/ext.rs hash: 90cf7718d54fe3c2

 If you need to change the above hash to make the test pass, please check if you
 need to adjust this doc as well and ping this issue: