diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2021-03-14 15:56:29 +0000 |
---|---|---|
committer | GitHub <[email protected]> | 2021-03-14 15:56:29 +0000 |
commit | af8440b84851febd805b6b774bfb326cbdb7175e (patch) | |
tree | 0af571afec0cf8825b77d326221c3dc8009a3b7f | |
parent | 406e4be04c2e74d58bcaa7e823e2509d1a7803d4 (diff) | |
parent | ba924d04b3b9f7c9fe846847ecd6604157f748f6 (diff) |
Merge #8014
8014: increase completion relevance for items in local scope r=matklad a=JoshMcguigan
This PR provides a small completion relevance score bonus for items in local scope. The changes here are relatively minimal, since `coc` by default pre-sorts by position in file. But as we move toward fully server side sorting #7935 I think we'll want some relevance score bump for items in local scope.
### Before
Note `let~` and `syntax` are both ahead of locals. Ultimately we may decide that `let~` is a high relevance completion given my cursor position here, but that should be done with some explicit scoring on the server side, rather than being caused by (I think) `coc` preferring shorter completions.
![pre-local-score](https://user-images.githubusercontent.com/22216761/111073414-c97ad600-849b-11eb-84e7-fcee130536f0.png)
### After
![post-local-score](https://user-images.githubusercontent.com/22216761/111073422-d0094d80-849b-11eb-92ec-7ae5ec3b190d.png)
Co-authored-by: Josh Mcguigan <[email protected]>
-rw-r--r-- | crates/ide_completion/src/item.rs | 37 | ||||
-rw-r--r-- | crates/ide_completion/src/render.rs | 79 | ||||
-rw-r--r-- | crates/rust-analyzer/src/to_proto.rs | 4 |
3 files changed, 88 insertions, 32 deletions
diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 3febab32b..9a4b5217a 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs | |||
@@ -144,6 +144,21 @@ pub struct CompletionRelevance { | |||
144 | /// } | 144 | /// } |
145 | /// ``` | 145 | /// ``` |
146 | pub exact_type_match: bool, | 146 | pub exact_type_match: bool, |
147 | /// This is set in cases like these: | ||
148 | /// | ||
149 | /// ``` | ||
150 | /// fn foo(bar: u32) { | ||
151 | /// $0 // `bar` is local | ||
152 | /// } | ||
153 | /// ``` | ||
154 | /// | ||
155 | /// ``` | ||
156 | /// fn foo() { | ||
157 | /// let bar = 0; | ||
158 | /// $0 // `bar` is local | ||
159 | /// } | ||
160 | /// ``` | ||
161 | pub is_local: bool, | ||
147 | } | 162 | } |
148 | 163 | ||
149 | impl CompletionRelevance { | 164 | impl CompletionRelevance { |
@@ -163,6 +178,9 @@ impl CompletionRelevance { | |||
163 | score += 1; | 178 | score += 1; |
164 | } | 179 | } |
165 | if self.exact_type_match { | 180 | if self.exact_type_match { |
181 | score += 3; | ||
182 | } | ||
183 | if self.is_local { | ||
166 | score += 1; | 184 | score += 1; |
167 | } | 185 | } |
168 | 186 | ||
@@ -551,9 +569,24 @@ mod tests { | |||
551 | vec![CompletionRelevance::default()], | 569 | vec![CompletionRelevance::default()], |
552 | vec![ | 570 | vec![ |
553 | CompletionRelevance { exact_name_match: true, ..CompletionRelevance::default() }, | 571 | CompletionRelevance { exact_name_match: true, ..CompletionRelevance::default() }, |
554 | CompletionRelevance { exact_type_match: true, ..CompletionRelevance::default() }, | 572 | CompletionRelevance { is_local: true, ..CompletionRelevance::default() }, |
555 | ], | 573 | ], |
556 | vec![CompletionRelevance { exact_name_match: true, exact_type_match: true }], | 574 | vec![CompletionRelevance { |
575 | exact_name_match: true, | ||
576 | is_local: true, | ||
577 | ..CompletionRelevance::default() | ||
578 | }], | ||
579 | vec![CompletionRelevance { exact_type_match: true, ..CompletionRelevance::default() }], | ||
580 | vec![CompletionRelevance { | ||
581 | exact_name_match: true, | ||
582 | exact_type_match: true, | ||
583 | ..CompletionRelevance::default() | ||
584 | }], | ||
585 | vec![CompletionRelevance { | ||
586 | exact_name_match: true, | ||
587 | exact_type_match: true, | ||
588 | is_local: true, | ||
589 | }], | ||
557 | ]; | 590 | ]; |
558 | 591 | ||
559 | check_relevance_score_ordered(expected_relevance_order); | 592 | check_relevance_score_ordered(expected_relevance_order); |
diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index fcb8115fb..905f0b197 100644 --- a/crates/ide_completion/src/render.rs +++ b/crates/ide_completion/src/render.rs | |||
@@ -157,9 +157,7 @@ impl<'a> Render<'a> { | |||
157 | .set_documentation(field.docs(self.ctx.db())) | 157 | .set_documentation(field.docs(self.ctx.db())) |
158 | .set_deprecated(is_deprecated); | 158 | .set_deprecated(is_deprecated); |
159 | 159 | ||
160 | if let Some(relevance) = compute_relevance(&self.ctx, &ty, &name.to_string()) { | 160 | item.set_relevance(compute_relevance(&self.ctx, &ty, &name.to_string())); |
161 | item.set_relevance(relevance); | ||
162 | } | ||
163 | 161 | ||
164 | item.build() | 162 | item.build() |
165 | } | 163 | } |
@@ -254,9 +252,9 @@ impl<'a> Render<'a> { | |||
254 | if let ScopeDef::Local(local) = resolution { | 252 | if let ScopeDef::Local(local) = resolution { |
255 | let ty = local.ty(self.ctx.db()); | 253 | let ty = local.ty(self.ctx.db()); |
256 | 254 | ||
257 | if let Some(relevance) = compute_relevance(&self.ctx, &ty, &local_name) { | 255 | let mut relevance = compute_relevance(&self.ctx, &ty, &local_name); |
258 | item.set_relevance(relevance); | 256 | relevance.is_local = true; |
259 | } | 257 | item.set_relevance(relevance); |
260 | 258 | ||
261 | if let Some((_expected_name, expected_type)) = self.ctx.expected_name_and_type() { | 259 | if let Some((_expected_name, expected_type)) = self.ctx.expected_name_and_type() { |
262 | if ty != expected_type { | 260 | if ty != expected_type { |
@@ -328,12 +326,15 @@ impl<'a> Render<'a> { | |||
328 | } | 326 | } |
329 | } | 327 | } |
330 | 328 | ||
331 | fn compute_relevance(ctx: &RenderContext, ty: &Type, name: &str) -> Option<CompletionRelevance> { | 329 | fn compute_relevance(ctx: &RenderContext, ty: &Type, name: &str) -> CompletionRelevance { |
332 | let (expected_name, expected_type) = ctx.expected_name_and_type()?; | ||
333 | let mut res = CompletionRelevance::default(); | 330 | let mut res = CompletionRelevance::default(); |
334 | res.exact_type_match = ty == &expected_type; | 331 | |
335 | res.exact_name_match = name == &expected_name; | 332 | if let Some((expected_name, expected_type)) = ctx.expected_name_and_type() { |
336 | Some(res) | 333 | res.exact_type_match = ty == &expected_type; |
334 | res.exact_name_match = name == &expected_name; | ||
335 | } | ||
336 | |||
337 | res | ||
337 | } | 338 | } |
338 | 339 | ||
339 | fn relevance_type_match(db: &dyn HirDatabase, ty: &Type, expected_type: &Type) -> bool { | 340 | fn relevance_type_match(db: &dyn HirDatabase, ty: &Type, expected_type: &Type) -> bool { |
@@ -343,6 +344,7 @@ fn relevance_type_match(db: &dyn HirDatabase, ty: &Type, expected_type: &Type) - | |||
343 | #[cfg(test)] | 344 | #[cfg(test)] |
344 | mod tests { | 345 | mod tests { |
345 | use expect_test::{expect, Expect}; | 346 | use expect_test::{expect, Expect}; |
347 | use itertools::Itertools; | ||
346 | 348 | ||
347 | use crate::{ | 349 | use crate::{ |
348 | test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG}, | 350 | test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG}, |
@@ -355,15 +357,17 @@ mod tests { | |||
355 | } | 357 | } |
356 | 358 | ||
357 | fn check_relevance(ra_fixture: &str, expect: Expect) { | 359 | fn check_relevance(ra_fixture: &str, expect: Expect) { |
358 | fn display_relevance(relevance: CompletionRelevance) -> &'static str { | 360 | fn display_relevance(relevance: CompletionRelevance) -> String { |
359 | match relevance { | 361 | let relevance_factors = vec![ |
360 | CompletionRelevance { exact_type_match: true, exact_name_match: true } => { | 362 | (relevance.exact_type_match, "type"), |
361 | "[type+name]" | 363 | (relevance.exact_name_match, "name"), |
362 | } | 364 | (relevance.is_local, "local"), |
363 | CompletionRelevance { exact_type_match: true, exact_name_match: false } => "[type]", | 365 | ] |
364 | CompletionRelevance { exact_type_match: false, exact_name_match: true } => "[name]", | 366 | .into_iter() |
365 | CompletionRelevance { exact_type_match: false, exact_name_match: false } => "[]", | 367 | .filter_map(|(cond, desc)| if cond { Some(desc) } else { None }) |
366 | } | 368 | .join("+"); |
369 | |||
370 | format!("[{}]", relevance_factors) | ||
367 | } | 371 | } |
368 | 372 | ||
369 | let actual = get_all_items(TEST_CONFIG, ra_fixture) | 373 | let actual = get_all_items(TEST_CONFIG, ra_fixture) |
@@ -918,7 +922,7 @@ struct WorldSnapshot { _f: () }; | |||
918 | fn go(world: &WorldSnapshot) { go(w$0) } | 922 | fn go(world: &WorldSnapshot) { go(w$0) } |
919 | "#, | 923 | "#, |
920 | expect![[r#" | 924 | expect![[r#" |
921 | lc world [type+name] | 925 | lc world [type+name+local] |
922 | st WorldSnapshot [] | 926 | st WorldSnapshot [] |
923 | fn go(…) [] | 927 | fn go(…) [] |
924 | "#]], | 928 | "#]], |
@@ -933,7 +937,7 @@ struct Foo; | |||
933 | fn f(foo: &Foo) { f(foo, w$0) } | 937 | fn f(foo: &Foo) { f(foo, w$0) } |
934 | "#, | 938 | "#, |
935 | expect![[r#" | 939 | expect![[r#" |
936 | lc foo [] | 940 | lc foo [local] |
937 | st Foo [] | 941 | st Foo [] |
938 | fn f(…) [] | 942 | fn f(…) [] |
939 | "#]], | 943 | "#]], |
@@ -998,6 +1002,7 @@ fn main() { | |||
998 | relevance: CompletionRelevance { | 1002 | relevance: CompletionRelevance { |
999 | exact_name_match: true, | 1003 | exact_name_match: true, |
1000 | exact_type_match: false, | 1004 | exact_type_match: false, |
1005 | is_local: true, | ||
1001 | }, | 1006 | }, |
1002 | ref_match: "&mut ", | 1007 | ref_match: "&mut ", |
1003 | }, | 1008 | }, |
@@ -1037,9 +1042,9 @@ fn main() { | |||
1037 | } | 1042 | } |
1038 | "#, | 1043 | "#, |
1039 | expect![[r#" | 1044 | expect![[r#" |
1040 | lc m [] | 1045 | lc m [local] |
1041 | lc t [] | 1046 | lc t [local] |
1042 | lc &t [type] | 1047 | lc &t [type+local] |
1043 | st T [] | 1048 | st T [] |
1044 | st S [] | 1049 | st S [] |
1045 | fn main() [] | 1050 | fn main() [] |
@@ -1091,9 +1096,9 @@ fn main() { | |||
1091 | } | 1096 | } |
1092 | "#, | 1097 | "#, |
1093 | expect![[r#" | 1098 | expect![[r#" |
1094 | lc m [] | 1099 | lc m [local] |
1095 | lc t [] | 1100 | lc t [local] |
1096 | lc &mut t [type] | 1101 | lc &mut t [type+local] |
1097 | tt DerefMut [] | 1102 | tt DerefMut [] |
1098 | tt Deref [] | 1103 | tt Deref [] |
1099 | fn foo(…) [] | 1104 | fn foo(…) [] |
@@ -1103,4 +1108,22 @@ fn main() { | |||
1103 | "#]], | 1108 | "#]], |
1104 | ) | 1109 | ) |
1105 | } | 1110 | } |
1111 | |||
1112 | #[test] | ||
1113 | fn locals() { | ||
1114 | check_relevance( | ||
1115 | r#" | ||
1116 | fn foo(bar: u32) { | ||
1117 | let baz = 0; | ||
1118 | |||
1119 | f$0 | ||
1120 | } | ||
1121 | "#, | ||
1122 | expect![[r#" | ||
1123 | lc baz [local] | ||
1124 | lc bar [local] | ||
1125 | fn foo(…) [] | ||
1126 | "#]], | ||
1127 | ); | ||
1128 | } | ||
1106 | } | 1129 | } |
diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs index 5736875d3..d415ed4d3 100644 --- a/crates/rust-analyzer/src/to_proto.rs +++ b/crates/rust-analyzer/src/to_proto.rs | |||
@@ -1117,13 +1117,13 @@ mod tests { | |||
1117 | ( | 1117 | ( |
1118 | "&arg", | 1118 | "&arg", |
1119 | Some( | 1119 | Some( |
1120 | "fffffffd", | 1120 | "fffffffa", |
1121 | ), | 1121 | ), |
1122 | ), | 1122 | ), |
1123 | ( | 1123 | ( |
1124 | "arg", | 1124 | "arg", |
1125 | Some( | 1125 | Some( |
1126 | "fffffffe", | 1126 | "fffffffd", |
1127 | ), | 1127 | ), |
1128 | ), | 1128 | ), |
1129 | ] | 1129 | ] |