diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2021-03-09 17:25:23 +0000 |
---|---|---|
committer | GitHub <[email protected]> | 2021-03-09 17:25:23 +0000 |
commit | 84eed2136b1c69d50ddf4bcf313ea3aa66ed12f4 (patch) | |
tree | 1d52c94044ac8ad9e73875398cfbdfd86908cca7 | |
parent | c45ac6effe37562daa97e6675c54963252f65664 (diff) | |
parent | b2764a6641be5caef3d62dfb80c836e4976194f3 (diff) |
Merge #7945
7945: Future proof completion scores r=matklad a=matklad
bors r+
🤖
Co-authored-by: Aleksey Kladov <[email protected]>
-rw-r--r-- | crates/ide_completion/src/item.rs | 58 | ||||
-rw-r--r-- | crates/ide_completion/src/lib.rs | 5 | ||||
-rw-r--r-- | crates/ide_completion/src/render.rs | 99 | ||||
-rw-r--r-- | crates/rust-analyzer/src/to_proto.rs | 6 |
4 files changed, 96 insertions, 72 deletions
diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 5e8ed75f1..14afec603 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs | |||
@@ -63,8 +63,14 @@ pub struct CompletionItem { | |||
63 | /// after completion. | 63 | /// after completion. |
64 | trigger_call_info: bool, | 64 | trigger_call_info: bool, |
65 | 65 | ||
66 | /// Score is useful to pre select or display in better order completion items | 66 | /// We use this to sort completion. Relevance records facts like "do the |
67 | score: Option<CompletionScore>, | 67 | /// types align precisely?". We can't sort by relevances directly, they are |
68 | /// only partially ordered. | ||
69 | /// | ||
70 | /// Note that Relevance ignores fuzzy match score. We compute Relevance for | ||
71 | /// all possible items, and then separately build an ordered completion list | ||
72 | /// based on relevance and fuzzy matching with the already typed identifier. | ||
73 | relevance: Relevance, | ||
68 | 74 | ||
69 | /// Indicates that a reference or mutable reference to this variable is a | 75 | /// Indicates that a reference or mutable reference to this variable is a |
70 | /// possible match. | 76 | /// possible match. |
@@ -101,8 +107,8 @@ impl fmt::Debug for CompletionItem { | |||
101 | if self.deprecated { | 107 | if self.deprecated { |
102 | s.field("deprecated", &true); | 108 | s.field("deprecated", &true); |
103 | } | 109 | } |
104 | if let Some(score) = &self.score { | 110 | if self.relevance.is_relevant() { |
105 | s.field("score", score); | 111 | s.field("relevance", &self.relevance); |
106 | } | 112 | } |
107 | if let Some(mutability) = &self.ref_match { | 113 | if let Some(mutability) = &self.ref_match { |
108 | s.field("ref_match", &format!("&{}", mutability.as_keyword_for_ref())); | 114 | s.field("ref_match", &format!("&{}", mutability.as_keyword_for_ref())); |
@@ -122,6 +128,36 @@ pub enum CompletionScore { | |||
122 | TypeAndNameMatch, | 128 | TypeAndNameMatch, |
123 | } | 129 | } |
124 | 130 | ||
131 | #[derive(Debug, Clone, Copy, Ord, PartialOrd, Eq, PartialEq, Default)] | ||
132 | pub struct Relevance { | ||
133 | /// This is set in cases like these: | ||
134 | /// | ||
135 | /// ``` | ||
136 | /// fn f(spam: String) {} | ||
137 | /// fn main { | ||
138 | /// let spam = 92; | ||
139 | /// f($0) // name of local matches the name of param | ||
140 | /// } | ||
141 | /// ``` | ||
142 | pub exact_name_match: bool, | ||
143 | /// This is set in cases like these: | ||
144 | /// | ||
145 | /// ``` | ||
146 | /// fn f(spam: String) {} | ||
147 | /// fn main { | ||
148 | /// let foo = String::new(); | ||
149 | /// f($0) // type of local matches the type of param | ||
150 | /// } | ||
151 | /// ``` | ||
152 | pub exact_type_match: bool, | ||
153 | } | ||
154 | |||
155 | impl Relevance { | ||
156 | pub fn is_relevant(&self) -> bool { | ||
157 | self != &Relevance::default() | ||
158 | } | ||
159 | } | ||
160 | |||
125 | #[derive(Debug, Clone, Copy, PartialEq, Eq)] | 161 | #[derive(Debug, Clone, Copy, PartialEq, Eq)] |
126 | pub enum CompletionItemKind { | 162 | pub enum CompletionItemKind { |
127 | SymbolKind(SymbolKind), | 163 | SymbolKind(SymbolKind), |
@@ -213,7 +249,7 @@ impl CompletionItem { | |||
213 | text_edit: None, | 249 | text_edit: None, |
214 | deprecated: false, | 250 | deprecated: false, |
215 | trigger_call_info: None, | 251 | trigger_call_info: None, |
216 | score: None, | 252 | relevance: Relevance::default(), |
217 | ref_match: None, | 253 | ref_match: None, |
218 | import_to_add: None, | 254 | import_to_add: None, |
219 | } | 255 | } |
@@ -256,8 +292,8 @@ impl CompletionItem { | |||
256 | self.deprecated | 292 | self.deprecated |
257 | } | 293 | } |
258 | 294 | ||
259 | pub fn score(&self) -> Option<CompletionScore> { | 295 | pub fn relevance(&self) -> Relevance { |
260 | self.score | 296 | self.relevance |
261 | } | 297 | } |
262 | 298 | ||
263 | pub fn trigger_call_info(&self) -> bool { | 299 | pub fn trigger_call_info(&self) -> bool { |
@@ -313,7 +349,7 @@ pub(crate) struct Builder { | |||
313 | text_edit: Option<TextEdit>, | 349 | text_edit: Option<TextEdit>, |
314 | deprecated: bool, | 350 | deprecated: bool, |
315 | trigger_call_info: Option<bool>, | 351 | trigger_call_info: Option<bool>, |
316 | score: Option<CompletionScore>, | 352 | relevance: Relevance, |
317 | ref_match: Option<Mutability>, | 353 | ref_match: Option<Mutability>, |
318 | } | 354 | } |
319 | 355 | ||
@@ -360,7 +396,7 @@ impl Builder { | |||
360 | completion_kind: self.completion_kind, | 396 | completion_kind: self.completion_kind, |
361 | deprecated: self.deprecated, | 397 | deprecated: self.deprecated, |
362 | trigger_call_info: self.trigger_call_info.unwrap_or(false), | 398 | trigger_call_info: self.trigger_call_info.unwrap_or(false), |
363 | score: self.score, | 399 | relevance: self.relevance, |
364 | ref_match: self.ref_match, | 400 | ref_match: self.ref_match, |
365 | import_to_add: self.import_to_add, | 401 | import_to_add: self.import_to_add, |
366 | } | 402 | } |
@@ -421,8 +457,8 @@ impl Builder { | |||
421 | self.deprecated = deprecated; | 457 | self.deprecated = deprecated; |
422 | self | 458 | self |
423 | } | 459 | } |
424 | pub(crate) fn set_score(mut self, score: CompletionScore) -> Builder { | 460 | pub(crate) fn set_relevance(mut self, relevance: Relevance) -> Builder { |
425 | self.score = Some(score); | 461 | self.relevance = relevance; |
426 | self | 462 | self |
427 | } | 463 | } |
428 | pub(crate) fn trigger_call_info(mut self) -> Builder { | 464 | pub(crate) fn trigger_call_info(mut self) -> Builder { |
diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs index a0c8c374d..d46f521a0 100644 --- a/crates/ide_completion/src/lib.rs +++ b/crates/ide_completion/src/lib.rs | |||
@@ -23,7 +23,10 @@ use crate::{completions::Completions, context::CompletionContext, item::Completi | |||
23 | 23 | ||
24 | pub use crate::{ | 24 | pub use crate::{ |
25 | config::CompletionConfig, | 25 | config::CompletionConfig, |
26 | item::{CompletionItem, CompletionItemKind, CompletionScore, ImportEdit, InsertTextFormat}, | 26 | item::{ |
27 | CompletionItem, CompletionItemKind, CompletionScore, ImportEdit, InsertTextFormat, | ||
28 | Relevance, | ||
29 | }, | ||
27 | }; | 30 | }; |
28 | 31 | ||
29 | //FIXME: split the following feature into fine-grained features. | 32 | //FIXME: split the following feature into fine-grained features. |
diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index 0a1b0f95d..8c8b149a1 100644 --- a/crates/ide_completion/src/render.rs +++ b/crates/ide_completion/src/render.rs | |||
@@ -20,8 +20,8 @@ use ide_db::{ | |||
20 | use syntax::TextRange; | 20 | use syntax::TextRange; |
21 | 21 | ||
22 | use crate::{ | 22 | use crate::{ |
23 | item::ImportEdit, CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, | 23 | item::{ImportEdit, Relevance}, |
24 | CompletionScore, | 24 | CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, |
25 | }; | 25 | }; |
26 | 26 | ||
27 | use crate::render::{enum_variant::render_variant, function::render_fn, macro_::render_macro}; | 27 | use crate::render::{enum_variant::render_variant, function::render_fn, macro_::render_macro}; |
@@ -117,7 +117,7 @@ impl<'a> RenderContext<'a> { | |||
117 | node.docs(self.db()) | 117 | node.docs(self.db()) |
118 | } | 118 | } |
119 | 119 | ||
120 | fn active_name_and_type(&self) -> Option<(String, Type)> { | 120 | fn expected_name_and_type(&self) -> Option<(String, Type)> { |
121 | if let Some(record_field) = &self.completion.record_field_syntax { | 121 | if let Some(record_field) = &self.completion.record_field_syntax { |
122 | cov_mark::hit!(record_field_type_match); | 122 | cov_mark::hit!(record_field_type_match); |
123 | let (struct_field, _local) = self.completion.sema.resolve_record_field(record_field)?; | 123 | let (struct_field, _local) = self.completion.sema.resolve_record_field(record_field)?; |
@@ -155,8 +155,8 @@ impl<'a> Render<'a> { | |||
155 | .set_documentation(field.docs(self.ctx.db())) | 155 | .set_documentation(field.docs(self.ctx.db())) |
156 | .set_deprecated(is_deprecated); | 156 | .set_deprecated(is_deprecated); |
157 | 157 | ||
158 | if let Some(score) = compute_score(&self.ctx, &ty, &name.to_string()) { | 158 | if let Some(relevance) = compute_relevance(&self.ctx, &ty, &name.to_string()) { |
159 | item = item.set_score(score); | 159 | item = item.set_relevance(relevance); |
160 | } | 160 | } |
161 | 161 | ||
162 | item.build() | 162 | item.build() |
@@ -247,18 +247,15 @@ impl<'a> Render<'a> { | |||
247 | }; | 247 | }; |
248 | 248 | ||
249 | if let ScopeDef::Local(local) = resolution { | 249 | if let ScopeDef::Local(local) = resolution { |
250 | if let Some((active_name, active_type)) = self.ctx.active_name_and_type() { | 250 | let ty = local.ty(self.ctx.db()); |
251 | let ty = local.ty(self.ctx.db()); | 251 | if let Some(relevance) = compute_relevance(&self.ctx, &ty, &local_name) { |
252 | if let Some(score) = | 252 | item = item.set_relevance(relevance) |
253 | compute_score_from_active(&active_type, &active_name, &ty, &local_name) | 253 | } |
254 | { | 254 | if let Some((_expected_name, expected_type)) = self.ctx.expected_name_and_type() { |
255 | item = item.set_score(score); | 255 | if let Some(ty_without_ref) = expected_type.remove_ref() { |
256 | } | ||
257 | |||
258 | if let Some(ty_without_ref) = active_type.remove_ref() { | ||
259 | if ty_without_ref == ty { | 256 | if ty_without_ref == ty { |
260 | cov_mark::hit!(suggest_ref); | 257 | cov_mark::hit!(suggest_ref); |
261 | let mutability = if active_type.is_mutable_reference() { | 258 | let mutability = if expected_type.is_mutable_reference() { |
262 | Mutability::Mut | 259 | Mutability::Mut |
263 | } else { | 260 | } else { |
264 | Mutability::Shared | 261 | Mutability::Shared |
@@ -326,33 +323,14 @@ impl<'a> Render<'a> { | |||
326 | } | 323 | } |
327 | } | 324 | } |
328 | 325 | ||
329 | fn compute_score_from_active( | 326 | fn compute_relevance(ctx: &RenderContext, ty: &Type, name: &str) -> Option<Relevance> { |
330 | active_type: &Type, | 327 | let (expected_name, expected_type) = ctx.expected_name_and_type()?; |
331 | active_name: &str, | 328 | let mut res = Relevance::default(); |
332 | ty: &Type, | 329 | res.exact_type_match = ty == &expected_type; |
333 | name: &str, | 330 | res.exact_name_match = name == &expected_name; |
334 | ) -> Option<CompletionScore> { | ||
335 | // Compute score | ||
336 | // For the same type | ||
337 | if active_type != ty { | ||
338 | return None; | ||
339 | } | ||
340 | |||
341 | let mut res = CompletionScore::TypeMatch; | ||
342 | |||
343 | // If same type + same name then go top position | ||
344 | if active_name == name { | ||
345 | res = CompletionScore::TypeAndNameMatch | ||
346 | } | ||
347 | |||
348 | Some(res) | 331 | Some(res) |
349 | } | 332 | } |
350 | 333 | ||
351 | fn compute_score(ctx: &RenderContext, ty: &Type, name: &str) -> Option<CompletionScore> { | ||
352 | let (active_name, active_type) = ctx.active_name_and_type()?; | ||
353 | compute_score_from_active(&active_type, &active_name, ty, name) | ||
354 | } | ||
355 | |||
356 | #[cfg(test)] | 334 | #[cfg(test)] |
357 | mod tests { | 335 | mod tests { |
358 | use std::cmp::Reverse; | 336 | use std::cmp::Reverse; |
@@ -361,7 +339,7 @@ mod tests { | |||
361 | 339 | ||
362 | use crate::{ | 340 | use crate::{ |
363 | test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG}, | 341 | test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG}, |
364 | CompletionKind, CompletionScore, | 342 | CompletionKind, Relevance, |
365 | }; | 343 | }; |
366 | 344 | ||
367 | fn check(ra_fixture: &str, expect: Expect) { | 345 | fn check(ra_fixture: &str, expect: Expect) { |
@@ -369,24 +347,25 @@ mod tests { | |||
369 | expect.assert_debug_eq(&actual); | 347 | expect.assert_debug_eq(&actual); |
370 | } | 348 | } |
371 | 349 | ||
372 | fn check_scores(ra_fixture: &str, expect: Expect) { | 350 | fn check_relevance(ra_fixture: &str, expect: Expect) { |
373 | fn display_score(score: Option<CompletionScore>) -> &'static str { | 351 | fn display_relevance(relevance: Relevance) -> &'static str { |
374 | match score { | 352 | match relevance { |
375 | Some(CompletionScore::TypeMatch) => "[type]", | 353 | Relevance { exact_type_match: true, exact_name_match: true } => "[type+name]", |
376 | Some(CompletionScore::TypeAndNameMatch) => "[type+name]", | 354 | Relevance { exact_type_match: true, exact_name_match: false } => "[type]", |
377 | None => "[]".into(), | 355 | Relevance { exact_type_match: false, exact_name_match: true } => "[name]", |
356 | Relevance { exact_type_match: false, exact_name_match: false } => "[]", | ||
378 | } | 357 | } |
379 | } | 358 | } |
380 | 359 | ||
381 | let mut completions = get_all_items(TEST_CONFIG, ra_fixture); | 360 | let mut completions = get_all_items(TEST_CONFIG, ra_fixture); |
382 | completions.sort_by_key(|it| (Reverse(it.score()), it.label().to_string())); | 361 | completions.sort_by_key(|it| (Reverse(it.relevance()), it.label().to_string())); |
383 | let actual = completions | 362 | let actual = completions |
384 | .into_iter() | 363 | .into_iter() |
385 | .filter(|it| it.completion_kind == CompletionKind::Reference) | 364 | .filter(|it| it.completion_kind == CompletionKind::Reference) |
386 | .map(|it| { | 365 | .map(|it| { |
387 | let tag = it.kind().unwrap().tag(); | 366 | let tag = it.kind().unwrap().tag(); |
388 | let score = display_score(it.score()); | 367 | let relevance = display_relevance(it.relevance()); |
389 | format!("{} {} {}\n", tag, it.label(), score) | 368 | format!("{} {} {}\n", tag, it.label(), relevance) |
390 | }) | 369 | }) |
391 | .collect::<String>(); | 370 | .collect::<String>(); |
392 | expect.assert_eq(&actual); | 371 | expect.assert_eq(&actual); |
@@ -849,9 +828,9 @@ fn foo(xs: Vec<i128>) | |||
849 | } | 828 | } |
850 | 829 | ||
851 | #[test] | 830 | #[test] |
852 | fn active_param_score() { | 831 | fn active_param_relevance() { |
853 | cov_mark::check!(active_param_type_match); | 832 | cov_mark::check!(active_param_type_match); |
854 | check_scores( | 833 | check_relevance( |
855 | r#" | 834 | r#" |
856 | struct S { foo: i64, bar: u32, baz: u32 } | 835 | struct S { foo: i64, bar: u32, baz: u32 } |
857 | fn test(bar: u32) { } | 836 | fn test(bar: u32) { } |
@@ -866,9 +845,9 @@ fn foo(s: S) { test(s.$0) } | |||
866 | } | 845 | } |
867 | 846 | ||
868 | #[test] | 847 | #[test] |
869 | fn record_field_scores() { | 848 | fn record_field_relevances() { |
870 | cov_mark::check!(record_field_type_match); | 849 | cov_mark::check!(record_field_type_match); |
871 | check_scores( | 850 | check_relevance( |
872 | r#" | 851 | r#" |
873 | struct A { foo: i64, bar: u32, baz: u32 } | 852 | struct A { foo: i64, bar: u32, baz: u32 } |
874 | struct B { x: (), y: f32, bar: u32 } | 853 | struct B { x: (), y: f32, bar: u32 } |
@@ -883,8 +862,8 @@ fn foo(a: A) { B { bar: a.$0 }; } | |||
883 | } | 862 | } |
884 | 863 | ||
885 | #[test] | 864 | #[test] |
886 | fn record_field_and_call_scores() { | 865 | fn record_field_and_call_relevances() { |
887 | check_scores( | 866 | check_relevance( |
888 | r#" | 867 | r#" |
889 | struct A { foo: i64, bar: u32, baz: u32 } | 868 | struct A { foo: i64, bar: u32, baz: u32 } |
890 | struct B { x: (), y: f32, bar: u32 } | 869 | struct B { x: (), y: f32, bar: u32 } |
@@ -897,7 +876,7 @@ fn foo(a: A) { B { bar: f(a.$0) }; } | |||
897 | fd baz [] | 876 | fd baz [] |
898 | "#]], | 877 | "#]], |
899 | ); | 878 | ); |
900 | check_scores( | 879 | check_relevance( |
901 | r#" | 880 | r#" |
902 | struct A { foo: i64, bar: u32, baz: u32 } | 881 | struct A { foo: i64, bar: u32, baz: u32 } |
903 | struct B { x: (), y: f32, bar: u32 } | 882 | struct B { x: (), y: f32, bar: u32 } |
@@ -914,7 +893,7 @@ fn foo(a: A) { f(B { bar: a.$0 }); } | |||
914 | 893 | ||
915 | #[test] | 894 | #[test] |
916 | fn prioritize_exact_ref_match() { | 895 | fn prioritize_exact_ref_match() { |
917 | check_scores( | 896 | check_relevance( |
918 | r#" | 897 | r#" |
919 | struct WorldSnapshot { _f: () }; | 898 | struct WorldSnapshot { _f: () }; |
920 | fn go(world: &WorldSnapshot) { go(w$0) } | 899 | fn go(world: &WorldSnapshot) { go(w$0) } |
@@ -929,7 +908,7 @@ fn go(world: &WorldSnapshot) { go(w$0) } | |||
929 | 908 | ||
930 | #[test] | 909 | #[test] |
931 | fn too_many_arguments() { | 910 | fn too_many_arguments() { |
932 | check_scores( | 911 | check_relevance( |
933 | r#" | 912 | r#" |
934 | struct Foo; | 913 | struct Foo; |
935 | fn f(foo: &Foo) { f(foo, w$0) } | 914 | fn f(foo: &Foo) { f(foo, w$0) } |
@@ -997,6 +976,10 @@ fn main() { | |||
997 | Local, | 976 | Local, |
998 | ), | 977 | ), |
999 | detail: "S", | 978 | detail: "S", |
979 | relevance: Relevance { | ||
980 | exact_name_match: true, | ||
981 | exact_type_match: false, | ||
982 | }, | ||
1000 | ref_match: "&mut ", | 983 | ref_match: "&mut ", |
1001 | }, | 984 | }, |
1002 | ] | 985 | ] |
diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 2380e021a..a9846fa70 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs | |||
@@ -213,7 +213,7 @@ pub(crate) fn completion_item( | |||
213 | ..Default::default() | 213 | ..Default::default() |
214 | }; | 214 | }; |
215 | 215 | ||
216 | if item.score().is_some() { | 216 | if item.relevance().is_relevant() { |
217 | lsp_item.preselect = Some(true); | 217 | lsp_item.preselect = Some(true); |
218 | // HACK: sort preselect items first | 218 | // HACK: sort preselect items first |
219 | lsp_item.sort_text = Some(format!(" {}", item.label())); | 219 | lsp_item.sort_text = Some(format!(" {}", item.label())); |
@@ -1106,7 +1106,9 @@ mod tests { | |||
1106 | [ | 1106 | [ |
1107 | ( | 1107 | ( |
1108 | "&arg", | 1108 | "&arg", |
1109 | None, | 1109 | Some( |
1110 | " arg", | ||
1111 | ), | ||
1110 | ), | 1112 | ), |
1111 | ( | 1113 | ( |
1112 | "arg", | 1114 | "arg", |