Skip to content

Commit

Permalink
Auto merge of #18167 - SomeoneToIgnore:fat-completions, r=Veykril
Browse files Browse the repository at this point in the history
internal: Send less data during `textDocument/completion` if possible

Similar to #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](zed-industries/zed#18212) to enable all resolving possible: 84452 characters
[after-after.json](https://github.com/user-attachments/files/17092755/after-after.json)
  • Loading branch information
bors committed Sep 30, 2024
2 parents 7b60339 + c03f5b6 commit dfe6d50
Show file tree
Hide file tree
Showing 11 changed files with 226 additions and 75 deletions.
3 changes: 2 additions & 1 deletion crates/ide-completion/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)]
Expand Down
25 changes: 25 additions & 0 deletions crates/ide-completion/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions crates/ide-completion/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions crates/ide/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
18 changes: 14 additions & 4 deletions crates/rust-analyzer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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()
Expand All @@ -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"),
},
}
}

Expand Down
94 changes: 65 additions & 29 deletions crates/rust-analyzer/src/handlers/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
);

Expand Down Expand Up @@ -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 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);
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);
}
}

Ok(original_completion)
Ok(resolved_completion)
}

pub(crate) fn handle_folding_range(
Expand Down
6 changes: 5 additions & 1 deletion crates/rust-analyzer/src/integrated_benchmarks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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() };
Expand Down Expand Up @@ -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() };
Expand Down Expand Up @@ -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() };
Expand Down
20 changes: 17 additions & 3 deletions crates/rust-analyzer/src/lsp/capabilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions crates/rust-analyzer/src/lsp/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
Loading

0 comments on commit dfe6d50

Please sign in to comment.