Skip to content

Commit 087cb62

Browse files
authored
Merge pull request #18653 from SomeoneToIgnore/hash-completions
Hash completion items to properly match them during /resolve
2 parents 002fcea + 4169926 commit 087cb62

File tree

11 files changed

+134
-23
lines changed

11 files changed

+134
-23
lines changed

Cargo.lock

+15
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/ide-completion/src/item.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -346,8 +346,7 @@ pub enum CompletionItemKind {
346346
impl_from!(SymbolKind for CompletionItemKind);
347347

348348
impl CompletionItemKind {
349-
#[cfg(test)]
350-
pub(crate) fn tag(self) -> &'static str {
349+
pub fn tag(self) -> &'static str {
351350
match self {
352351
CompletionItemKind::SymbolKind(kind) => match kind {
353352
SymbolKind::Attribute => "at",

crates/ide-completion/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ pub use crate::{
3434
config::{CallableSnippets, CompletionConfig},
3535
item::{
3636
CompletionItem, CompletionItemKind, CompletionRelevance, CompletionRelevancePostfixMatch,
37+
CompletionRelevanceReturnType, CompletionRelevanceTypeMatch,
3738
},
3839
snippet::{Snippet, SnippetScope},
3940
};

crates/rust-analyzer/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@ path = "src/bin/main.rs"
2121

2222
[dependencies]
2323
anyhow.workspace = true
24+
base64 = "0.22"
2425
crossbeam-channel.workspace = true
2526
dirs = "5.0.1"
2627
dissimilar.workspace = true
28+
ide-completion.workspace = true
2729
itertools.workspace = true
2830
scip = "0.5.1"
2931
lsp-types = { version = "=0.95.0", features = ["proposed"] }
@@ -34,6 +36,7 @@ rayon.workspace = true
3436
rustc-hash.workspace = true
3537
serde_json = { workspace = true, features = ["preserve_order"] }
3638
serde.workspace = true
39+
tenthash = "0.4.0"
3740
num_cpus = "1.15.0"
3841
mimalloc = { version = "0.1.30", default-features = false, optional = true }
3942
lsp-server.workspace = true

crates/rust-analyzer/src/config.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1440,7 +1440,7 @@ impl Config {
14401440
limit: self.completion_limit(source_root).to_owned(),
14411441
enable_term_search: self.completion_termSearch_enable(source_root).to_owned(),
14421442
term_search_fuel: self.completion_termSearch_fuel(source_root).to_owned() as u64,
1443-
fields_to_resolve: if self.client_is_helix() || self.client_is_neovim() {
1443+
fields_to_resolve: if self.client_is_neovim() {
14441444
CompletionFieldsToResolve::empty()
14451445
} else {
14461446
CompletionFieldsToResolve::from_client_capabilities(&client_capability_fields)

crates/rust-analyzer/src/handlers/request.rs

+20-9
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use std::{
1010

1111
use anyhow::Context;
1212

13+
use base64::{prelude::BASE64_STANDARD, Engine};
1314
use ide::{
1415
AnnotationConfig, AssistKind, AssistResolveStrategy, Cancellable, CompletionFieldsToResolve,
1516
FilePosition, FileRange, HoverAction, HoverGotoTypeData, InlayFieldsToResolve, Query,
@@ -36,6 +37,7 @@ use triomphe::Arc;
3637
use vfs::{AbsPath, AbsPathBuf, FileId, VfsPath};
3738

3839
use crate::{
40+
completion_item_hash,
3941
config::{Config, RustfmtConfig, WorkspaceSymbolConfig},
4042
diagnostics::convert_diagnostic,
4143
global_state::{FetchWorkspaceRequest, GlobalState, GlobalStateSnapshot},
@@ -1127,30 +1129,39 @@ pub(crate) fn handle_completion_resolve(
11271129
forced_resolve_completions_config.fields_to_resolve = CompletionFieldsToResolve::empty();
11281130

11291131
let position = FilePosition { file_id, offset };
1130-
let Some(resolved_completions) = snap.analysis.completions(
1132+
let Some(completions) = snap.analysis.completions(
11311133
&forced_resolve_completions_config,
11321134
position,
11331135
resolve_data.trigger_character,
11341136
)?
11351137
else {
11361138
return Ok(original_completion);
11371139
};
1140+
let Ok(resolve_data_hash) = BASE64_STANDARD.decode(resolve_data.hash) else {
1141+
return Ok(original_completion);
1142+
};
1143+
1144+
let Some(corresponding_completion) = completions.into_iter().find(|completion_item| {
1145+
// Avoid computing hashes for items that obviously do not match
1146+
// r-a might append a detail-based suffix to the label, so we cannot check for equality
1147+
original_completion.label.starts_with(completion_item.label.as_str())
1148+
&& resolve_data_hash == completion_item_hash(completion_item, resolve_data.for_ref)
1149+
}) else {
1150+
return Ok(original_completion);
1151+
};
1152+
11381153
let mut resolved_completions = to_proto::completion_items(
11391154
&snap.config,
11401155
&forced_resolve_completions_config.fields_to_resolve,
11411156
&line_index,
11421157
snap.file_version(position.file_id),
11431158
resolve_data.position,
11441159
resolve_data.trigger_character,
1145-
resolved_completions,
1160+
vec![corresponding_completion],
11461161
);
1147-
1148-
let mut resolved_completion =
1149-
if resolved_completions.get(resolve_data.completion_item_index).is_some() {
1150-
resolved_completions.swap_remove(resolve_data.completion_item_index)
1151-
} else {
1152-
return Ok(original_completion);
1153-
};
1162+
let Some(mut resolved_completion) = resolved_completions.pop() else {
1163+
return Ok(original_completion);
1164+
};
11541165

11551166
if !resolve_data.imports.is_empty() {
11561167
let additional_edits = snap

crates/rust-analyzer/src/lib.rs

+78
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ use self::lsp::ext as lsp_ext;
4747
#[cfg(test)]
4848
mod integrated_benchmarks;
4949

50+
use ide::{CompletionItem, CompletionRelevance};
5051
use serde::de::DeserializeOwned;
52+
use tenthash::TentHasher;
5153

5254
pub use crate::{
5355
lsp::capabilities::server_capabilities, main_loop::main_loop, reload::ws_to_crate_graph,
@@ -61,3 +63,79 @@ pub fn from_json<T: DeserializeOwned>(
6163
serde_json::from_value(json.clone())
6264
.map_err(|e| anyhow::format_err!("Failed to deserialize {what}: {e}; {json}"))
6365
}
66+
67+
fn completion_item_hash(item: &CompletionItem, is_ref_completion: bool) -> [u8; 20] {
68+
fn hash_completion_relevance(hasher: &mut TentHasher, relevance: &CompletionRelevance) {
69+
use ide_completion::{
70+
CompletionRelevancePostfixMatch, CompletionRelevanceReturnType,
71+
CompletionRelevanceTypeMatch,
72+
};
73+
74+
hasher.update([
75+
u8::from(relevance.exact_name_match),
76+
u8::from(relevance.is_local),
77+
u8::from(relevance.is_name_already_imported),
78+
u8::from(relevance.requires_import),
79+
u8::from(relevance.is_private_editable),
80+
]);
81+
if let Some(type_match) = &relevance.type_match {
82+
let label = match type_match {
83+
CompletionRelevanceTypeMatch::CouldUnify => "could_unify",
84+
CompletionRelevanceTypeMatch::Exact => "exact",
85+
};
86+
hasher.update(label);
87+
}
88+
if let Some(trait_) = &relevance.trait_ {
89+
hasher.update([u8::from(trait_.is_op_method), u8::from(trait_.notable_trait)]);
90+
}
91+
if let Some(postfix_match) = &relevance.postfix_match {
92+
let label = match postfix_match {
93+
CompletionRelevancePostfixMatch::NonExact => "non_exact",
94+
CompletionRelevancePostfixMatch::Exact => "exact",
95+
};
96+
hasher.update(label);
97+
}
98+
if let Some(function) = &relevance.function {
99+
hasher.update([u8::from(function.has_params), u8::from(function.has_self_param)]);
100+
let label = match function.return_type {
101+
CompletionRelevanceReturnType::Other => "other",
102+
CompletionRelevanceReturnType::DirectConstructor => "direct_constructor",
103+
CompletionRelevanceReturnType::Constructor => "constructor",
104+
CompletionRelevanceReturnType::Builder => "builder",
105+
};
106+
hasher.update(label);
107+
}
108+
}
109+
110+
let mut hasher = TentHasher::new();
111+
hasher.update([
112+
u8::from(is_ref_completion),
113+
u8::from(item.is_snippet),
114+
u8::from(item.deprecated),
115+
u8::from(item.trigger_call_info),
116+
]);
117+
hasher.update(&item.label);
118+
if let Some(label_detail) = &item.label_detail {
119+
hasher.update(label_detail);
120+
}
121+
// NB: do not hash edits or source range, as those may change between the time the client sends the resolve request
122+
// and the time it receives it: some editors do allow changing the buffer between that, leading to ranges being different.
123+
//
124+
// Documentation hashing is skipped too, as it's a large blob to process,
125+
// while not really making completion properties more unique as they are already.
126+
hasher.update(item.kind.tag());
127+
hasher.update(&item.lookup);
128+
if let Some(detail) = &item.detail {
129+
hasher.update(detail);
130+
}
131+
hash_completion_relevance(&mut hasher, &item.relevance);
132+
if let Some((mutability, text_size)) = &item.ref_match {
133+
hasher.update(mutability.as_keyword_for_ref());
134+
hasher.update(u32::from(*text_size).to_le_bytes());
135+
}
136+
for (import_path, import_name) in &item.import_to_add {
137+
hasher.update(import_path);
138+
hasher.update(import_name);
139+
}
140+
hasher.finalize()
141+
}

crates/rust-analyzer/src/lsp/capabilities.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub fn server_capabilities(config: &Config) -> ServerCapabilities {
4141
})),
4242
hover_provider: Some(HoverProviderCapability::Simple(true)),
4343
completion_provider: Some(CompletionOptions {
44-
resolve_provider: if config.client_is_helix() || config.client_is_neovim() {
44+
resolve_provider: if config.client_is_neovim() {
4545
config.completion_item_edit_resolve().then_some(true)
4646
} else {
4747
Some(config.caps().completions_resolve_provider())

crates/rust-analyzer/src/lsp/ext.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -826,7 +826,8 @@ pub struct CompletionResolveData {
826826
pub imports: Vec<CompletionImport>,
827827
pub version: Option<i32>,
828828
pub trigger_character: Option<char>,
829-
pub completion_item_index: usize,
829+
pub for_ref: bool,
830+
pub hash: String,
830831
}
831832

832833
#[derive(Debug, Serialize, Deserialize)]

crates/rust-analyzer/src/lsp/to_proto.rs

+11-8
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use std::{
55
sync::atomic::{AtomicU32, Ordering},
66
};
77

8+
use base64::{prelude::BASE64_STANDARD, Engine};
89
use ide::{
910
Annotation, AnnotationKind, Assist, AssistKind, Cancellable, CompletionFieldsToResolve,
1011
CompletionItem, CompletionItemKind, CompletionRelevance, Documentation, FileId, FileRange,
@@ -21,6 +22,7 @@ use serde_json::to_value;
2122
use vfs::AbsPath;
2223

2324
use crate::{
25+
completion_item_hash,
2426
config::{CallInfoConfig, Config},
2527
global_state::GlobalStateSnapshot,
2628
line_index::{LineEndings, LineIndex, PositionEncoding},
@@ -295,7 +297,7 @@ fn completion_item(
295297
// non-trivial mapping here.
296298
let mut text_edit = None;
297299
let source_range = item.source_range;
298-
for indel in item.text_edit {
300+
for indel in &item.text_edit {
299301
if indel.delete.contains_range(source_range) {
300302
// Extract this indel as the main edit
301303
text_edit = Some(if indel.delete == source_range {
@@ -347,7 +349,7 @@ fn completion_item(
347349
something_to_resolve |= item.documentation.is_some();
348350
None
349351
} else {
350-
item.documentation.map(documentation)
352+
item.documentation.clone().map(documentation)
351353
};
352354

353355
let mut lsp_item = lsp_types::CompletionItem {
@@ -371,10 +373,10 @@ fn completion_item(
371373
} else {
372374
lsp_item.label_details = Some(lsp_types::CompletionItemLabelDetails {
373375
detail: item.label_detail.as_ref().map(ToString::to_string),
374-
description: item.detail,
376+
description: item.detail.clone(),
375377
});
376378
}
377-
} else if let Some(label_detail) = item.label_detail {
379+
} else if let Some(label_detail) = &item.label_detail {
378380
lsp_item.label.push_str(label_detail.as_str());
379381
}
380382

@@ -383,6 +385,7 @@ fn completion_item(
383385
let imports =
384386
if config.completion(None).enable_imports_on_the_fly && !item.import_to_add.is_empty() {
385387
item.import_to_add
388+
.clone()
386389
.into_iter()
387390
.map(|(import_path, import_name)| lsp_ext::CompletionImport {
388391
full_import_path: import_path,
@@ -393,16 +396,15 @@ fn completion_item(
393396
Vec::new()
394397
};
395398
let (ref_resolve_data, resolve_data) = if something_to_resolve || !imports.is_empty() {
396-
let mut item_index = acc.len();
397399
let ref_resolve_data = if ref_match.is_some() {
398400
let ref_resolve_data = lsp_ext::CompletionResolveData {
399401
position: tdpp.clone(),
400402
imports: Vec::new(),
401403
version,
402404
trigger_character: completion_trigger_character,
403-
completion_item_index: item_index,
405+
for_ref: true,
406+
hash: BASE64_STANDARD.encode(completion_item_hash(&item, true)),
404407
};
405-
item_index += 1;
406408
Some(to_value(ref_resolve_data).unwrap())
407409
} else {
408410
None
@@ -412,7 +414,8 @@ fn completion_item(
412414
imports,
413415
version,
414416
trigger_character: completion_trigger_character,
415-
completion_item_index: item_index,
417+
for_ref: false,
418+
hash: BASE64_STANDARD.encode(completion_item_hash(&item, false)),
416419
};
417420
(ref_resolve_data, Some(to_value(resolve_data).unwrap()))
418421
} else {

docs/dev/lsp-extensions.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!---
2-
lsp/ext.rs hash: 96f88b7a5d0080c6
2+
lsp/ext.rs hash: 14b7fb1309f5bb00
33
44
If you need to change the above hash to make the test pass, please check if you
55
need to adjust this doc as well and ping this issue:

0 commit comments

Comments
 (0)