aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2021-03-26 16:29:20 +0000
committerGitHub <[email protected]>2021-03-26 16:29:20 +0000
commit4ecaad98e074c42dbf637a11afcb630aafffd7b3 (patch)
tree50832b85e65752b040affcc8d34dc5cd8c2625b9
parent20e32fc946010f8c46728d6cb8bab1b96b3f48b9 (diff)
parent0e31ae2cef92f973a6ee3def3f1feaa94dcb7a4f (diff)
Merge #8056
8056: completion relevance consider if types can be unified r=JoshMcguigan a=JoshMcguigan This PR improves completion relevance scoring for generic types, in cases where the types could unify. ### Before ![pre-could-unify](https://user-images.githubusercontent.com/22216761/111338556-46d94e80-8634-11eb-9936-2b20eb9e6756.png) ### After ![post-could-unify](https://user-images.githubusercontent.com/22216761/111338598-4e005c80-8634-11eb-92e0-69c2c1cda6fc.png) changelog feature improve completions Co-authored-by: Josh Mcguigan <[email protected]>
-rw-r--r--crates/hir/src/lib.rs6
-rw-r--r--crates/hir_ty/src/infer.rs5
-rw-r--r--crates/hir_ty/src/infer/unify.rs4
-rw-r--r--crates/hir_ty/src/lib.rs2
-rw-r--r--crates/ide_completion/src/item.rs59
-rw-r--r--crates/ide_completion/src/render.rs81
-rw-r--r--crates/ide_completion/src/render/enum_variant.rs4
-rw-r--r--crates/ide_completion/src/render/function.rs4
-rw-r--r--crates/rust-analyzer/src/to_proto.rs2
9 files changed, 124 insertions, 43 deletions
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 05a60e158..4ee08ef21 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -51,7 +51,7 @@ use hir_def::{
51}; 51};
52use hir_expand::{diagnostics::DiagnosticSink, name::name, MacroDefKind}; 52use hir_expand::{diagnostics::DiagnosticSink, name::name, MacroDefKind};
53use hir_ty::{ 53use hir_ty::{
54 autoderef, 54 autoderef, could_unify,
55 method_resolution::{self, TyFingerprint}, 55 method_resolution::{self, TyFingerprint},
56 primitive::UintTy, 56 primitive::UintTy,
57 to_assoc_type_id, 57 to_assoc_type_id,
@@ -2154,6 +2154,10 @@ impl Type {
2154 2154
2155 walk_type(db, self, &mut cb); 2155 walk_type(db, self, &mut cb);
2156 } 2156 }
2157
2158 pub fn could_unify_with(&self, other: &Type) -> bool {
2159 could_unify(&self.ty, &other.ty)
2160 }
2157} 2161}
2158 2162
2159// FIXME: closures 2163// FIXME: closures
diff --git a/crates/hir_ty/src/infer.rs b/crates/hir_ty/src/infer.rs
index 8f9cf7480..e4407ff50 100644
--- a/crates/hir_ty/src/infer.rs
+++ b/crates/hir_ty/src/infer.rs
@@ -45,6 +45,11 @@ use crate::{
45 to_assoc_type_id, to_chalk_trait_id, AliasEq, AliasTy, Interner, TyKind, 45 to_assoc_type_id, to_chalk_trait_id, AliasEq, AliasTy, Interner, TyKind,
46}; 46};
47 47
48// This lint has a false positive here. See the link below for details.
49//
50// https://github.com/rust-lang/rust/issues/57411
51#[allow(unreachable_pub)]
52pub use unify::could_unify;
48pub(crate) use unify::unify; 53pub(crate) use unify::unify;
49 54
50mod unify; 55mod unify;
diff --git a/crates/hir_ty/src/infer/unify.rs b/crates/hir_ty/src/infer/unify.rs
index 75250a369..6e7b0f5a6 100644
--- a/crates/hir_ty/src/infer/unify.rs
+++ b/crates/hir_ty/src/infer/unify.rs
@@ -157,6 +157,10 @@ impl<T> Canonicalized<T> {
157 } 157 }
158} 158}
159 159
160pub fn could_unify(t1: &Ty, t2: &Ty) -> bool {
161 InferenceTable::new().unify(t1, t2)
162}
163
160pub(crate) fn unify(tys: &Canonical<(Ty, Ty)>) -> Option<Substitution> { 164pub(crate) fn unify(tys: &Canonical<(Ty, Ty)>) -> Option<Substitution> {
161 let mut table = InferenceTable::new(); 165 let mut table = InferenceTable::new();
162 let vars = Substitution( 166 let vars = Substitution(
diff --git a/crates/hir_ty/src/lib.rs b/crates/hir_ty/src/lib.rs
index 69265286f..6f9c698e6 100644
--- a/crates/hir_ty/src/lib.rs
+++ b/crates/hir_ty/src/lib.rs
@@ -41,7 +41,7 @@ use crate::{
41}; 41};
42 42
43pub use autoderef::autoderef; 43pub use autoderef::autoderef;
44pub use infer::{InferenceResult, InferenceVar}; 44pub use infer::{could_unify, InferenceResult, InferenceVar};
45pub use lower::{ 45pub use lower::{
46 associated_type_shorthand_candidates, callable_item_sig, CallableDefId, ImplTraitLoweringMode, 46 associated_type_shorthand_candidates, callable_item_sig, CallableDefId, ImplTraitLoweringMode,
47 TyDefId, TyLoweringContext, ValueTyDefId, 47 TyDefId, TyLoweringContext, ValueTyDefId,
diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs
index 9a4b5217a..cc4ac9ea2 100644
--- a/crates/ide_completion/src/item.rs
+++ b/crates/ide_completion/src/item.rs
@@ -122,7 +122,7 @@ impl fmt::Debug for CompletionItem {
122 } 122 }
123} 123}
124 124
125#[derive(Debug, Clone, Copy, Ord, PartialOrd, Eq, PartialEq, Default)] 125#[derive(Debug, Clone, Copy, Eq, PartialEq, Default)]
126pub struct CompletionRelevance { 126pub struct CompletionRelevance {
127 /// This is set in cases like these: 127 /// This is set in cases like these:
128 /// 128 ///
@@ -134,31 +134,41 @@ pub struct CompletionRelevance {
134 /// } 134 /// }
135 /// ``` 135 /// ```
136 pub exact_name_match: bool, 136 pub exact_name_match: bool,
137 /// See CompletionRelevanceTypeMatch doc comments for cases where this is set.
138 pub type_match: Option<CompletionRelevanceTypeMatch>,
137 /// This is set in cases like these: 139 /// This is set in cases like these:
138 /// 140 ///
139 /// ``` 141 /// ```
140 /// fn f(spam: String) {} 142 /// fn foo(a: u32) {
141 /// fn main { 143 /// let b = 0;
142 /// let foo = String::new(); 144 /// $0 // `a` and `b` are local
143 /// f($0) // type of local matches the type of param
144 /// } 145 /// }
145 /// ``` 146 /// ```
146 pub exact_type_match: bool, 147 pub is_local: bool,
148}
149
150#[derive(Debug, Clone, Copy, Eq, PartialEq)]
151pub enum CompletionRelevanceTypeMatch {
147 /// This is set in cases like these: 152 /// This is set in cases like these:
148 /// 153 ///
149 /// ``` 154 /// ```
150 /// fn foo(bar: u32) { 155 /// enum Option<T> { Some(T), None }
151 /// $0 // `bar` is local 156 /// fn f(a: Option<u32>) {}
157 /// fn main {
158 /// f(Option::N$0) // type `Option<T>` could unify with `Option<u32>`
152 /// } 159 /// }
153 /// ``` 160 /// ```
161 CouldUnify,
162 /// This is set in cases like these:
154 /// 163 ///
155 /// ``` 164 /// ```
156 /// fn foo() { 165 /// fn f(spam: String) {}
157 /// let bar = 0; 166 /// fn main {
158 /// $0 // `bar` is local 167 /// let foo = String::new();
168 /// f($0) // type of local matches the type of param
159 /// } 169 /// }
160 /// ``` 170 /// ```
161 pub is_local: bool, 171 Exact,
162} 172}
163 173
164impl CompletionRelevance { 174impl CompletionRelevance {
@@ -177,9 +187,11 @@ impl CompletionRelevance {
177 if self.exact_name_match { 187 if self.exact_name_match {
178 score += 1; 188 score += 1;
179 } 189 }
180 if self.exact_type_match { 190 score += match self.type_match {
181 score += 3; 191 Some(CompletionRelevanceTypeMatch::Exact) => 4,
182 } 192 Some(CompletionRelevanceTypeMatch::CouldUnify) => 3,
193 None => 0,
194 };
183 if self.is_local { 195 if self.is_local {
184 score += 1; 196 score += 1;
185 } 197 }
@@ -342,7 +354,7 @@ impl CompletionItem {
342 // match, but with exact type match set because self.ref_match 354 // match, but with exact type match set because self.ref_match
343 // is only set if there is an exact type match. 355 // is only set if there is an exact type match.
344 let mut relevance = self.relevance; 356 let mut relevance = self.relevance;
345 relevance.exact_type_match = true; 357 relevance.type_match = Some(CompletionRelevanceTypeMatch::Exact);
346 358
347 self.ref_match.map(|mutability| (mutability, relevance)) 359 self.ref_match.map(|mutability| (mutability, relevance))
348 } 360 }
@@ -523,7 +535,7 @@ mod tests {
523 use itertools::Itertools; 535 use itertools::Itertools;
524 use test_utils::assert_eq_text; 536 use test_utils::assert_eq_text;
525 537
526 use super::CompletionRelevance; 538 use super::{CompletionRelevance, CompletionRelevanceTypeMatch};
527 539
528 /// Check that these are CompletionRelevance are sorted in ascending order 540 /// Check that these are CompletionRelevance are sorted in ascending order
529 /// by their relevance score. 541 /// by their relevance score.
@@ -576,15 +588,22 @@ mod tests {
576 is_local: true, 588 is_local: true,
577 ..CompletionRelevance::default() 589 ..CompletionRelevance::default()
578 }], 590 }],
579 vec![CompletionRelevance { exact_type_match: true, ..CompletionRelevance::default() }], 591 vec![CompletionRelevance {
592 type_match: Some(CompletionRelevanceTypeMatch::CouldUnify),
593 ..CompletionRelevance::default()
594 }],
595 vec![CompletionRelevance {
596 type_match: Some(CompletionRelevanceTypeMatch::Exact),
597 ..CompletionRelevance::default()
598 }],
580 vec![CompletionRelevance { 599 vec![CompletionRelevance {
581 exact_name_match: true, 600 exact_name_match: true,
582 exact_type_match: true, 601 type_match: Some(CompletionRelevanceTypeMatch::Exact),
583 ..CompletionRelevance::default() 602 ..CompletionRelevance::default()
584 }], 603 }],
585 vec![CompletionRelevance { 604 vec![CompletionRelevance {
586 exact_name_match: true, 605 exact_name_match: true,
587 exact_type_match: true, 606 type_match: Some(CompletionRelevanceTypeMatch::Exact),
588 is_local: true, 607 is_local: true,
589 }], 608 }],
590 ]; 609 ];
diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs
index 9ce49074f..1a762d3dc 100644
--- a/crates/ide_completion/src/render.rs
+++ b/crates/ide_completion/src/render.rs
@@ -20,8 +20,8 @@ use ide_db::{
20use syntax::TextRange; 20use syntax::TextRange;
21 21
22use crate::{ 22use crate::{
23 item::ImportEdit, CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, 23 item::{CompletionRelevanceTypeMatch, ImportEdit},
24 CompletionRelevance, 24 CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, CompletionRelevance,
25}; 25};
26 26
27use crate::render::{enum_variant::render_variant, function::render_fn, macro_::render_macro}; 27use crate::render::{enum_variant::render_variant, function::render_fn, macro_::render_macro};
@@ -143,7 +143,7 @@ impl<'a> Render<'a> {
143 .set_deprecated(is_deprecated); 143 .set_deprecated(is_deprecated);
144 144
145 item.set_relevance(CompletionRelevance { 145 item.set_relevance(CompletionRelevance {
146 exact_type_match: compute_exact_type_match(self.ctx.completion, ty), 146 type_match: compute_type_match(self.ctx.completion, ty),
147 exact_name_match: compute_exact_name_match(self.ctx.completion, name.to_string()), 147 exact_name_match: compute_exact_name_match(self.ctx.completion, name.to_string()),
148 ..CompletionRelevance::default() 148 ..CompletionRelevance::default()
149 }); 149 });
@@ -245,7 +245,7 @@ impl<'a> Render<'a> {
245 } 245 }
246 246
247 item.set_relevance(CompletionRelevance { 247 item.set_relevance(CompletionRelevance {
248 exact_type_match: compute_exact_type_match(self.ctx.completion, &ty), 248 type_match: compute_type_match(self.ctx.completion, &ty),
249 exact_name_match: compute_exact_name_match(self.ctx.completion, &local_name), 249 exact_name_match: compute_exact_name_match(self.ctx.completion, &local_name),
250 is_local: true, 250 is_local: true,
251 ..CompletionRelevance::default() 251 ..CompletionRelevance::default()
@@ -309,14 +309,24 @@ impl<'a> Render<'a> {
309 } 309 }
310} 310}
311 311
312fn compute_exact_type_match(ctx: &CompletionContext, completion_ty: &hir::Type) -> bool { 312fn compute_type_match(
313 match ctx.expected_type.as_ref() { 313 ctx: &CompletionContext,
314 Some(expected_type) => { 314 completion_ty: &hir::Type,
315 // We don't ever consider unit type to be an exact type match, since 315) -> Option<CompletionRelevanceTypeMatch> {
316 // nearly always this is not meaningful to the user. 316 let expected_type = ctx.expected_type.as_ref()?;
317 completion_ty == expected_type && !expected_type.is_unit() 317
318 } 318 // We don't ever consider unit type to be an exact type match, since
319 None => false, 319 // nearly always this is not meaningful to the user.
320 if expected_type.is_unit() {
321 return None;
322 }
323
324 if completion_ty == expected_type {
325 Some(CompletionRelevanceTypeMatch::Exact)
326 } else if expected_type.could_unify_with(completion_ty) {
327 Some(CompletionRelevanceTypeMatch::CouldUnify)
328 } else {
329 None
320 } 330 }
321} 331}
322 332
@@ -348,6 +358,7 @@ mod tests {
348 use itertools::Itertools; 358 use itertools::Itertools;
349 359
350 use crate::{ 360 use crate::{
361 item::CompletionRelevanceTypeMatch,
351 test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG}, 362 test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG},
352 CompletionKind, CompletionRelevance, 363 CompletionKind, CompletionRelevance,
353 }; 364 };
@@ -360,7 +371,11 @@ mod tests {
360 fn check_relevance(ra_fixture: &str, expect: Expect) { 371 fn check_relevance(ra_fixture: &str, expect: Expect) {
361 fn display_relevance(relevance: CompletionRelevance) -> String { 372 fn display_relevance(relevance: CompletionRelevance) -> String {
362 let relevance_factors = vec![ 373 let relevance_factors = vec![
363 (relevance.exact_type_match, "type"), 374 (relevance.type_match == Some(CompletionRelevanceTypeMatch::Exact), "type"),
375 (
376 relevance.type_match == Some(CompletionRelevanceTypeMatch::CouldUnify),
377 "type_could_unify",
378 ),
364 (relevance.exact_name_match, "name"), 379 (relevance.exact_name_match, "name"),
365 (relevance.is_local, "local"), 380 (relevance.is_local, "local"),
366 ] 381 ]
@@ -533,7 +548,9 @@ fn main() { let _: m::Spam = S$0 }
533 detail: "(i32)", 548 detail: "(i32)",
534 relevance: CompletionRelevance { 549 relevance: CompletionRelevance {
535 exact_name_match: false, 550 exact_name_match: false,
536 exact_type_match: true, 551 type_match: Some(
552 Exact,
553 ),
537 is_local: false, 554 is_local: false,
538 }, 555 },
539 trigger_call_info: true, 556 trigger_call_info: true,
@@ -559,7 +576,9 @@ fn main() { let _: m::Spam = S$0 }
559 detail: "()", 576 detail: "()",
560 relevance: CompletionRelevance { 577 relevance: CompletionRelevance {
561 exact_name_match: false, 578 exact_name_match: false,
562 exact_type_match: true, 579 type_match: Some(
580 Exact,
581 ),
563 is_local: false, 582 is_local: false,
564 }, 583 },
565 }, 584 },
@@ -1108,7 +1127,7 @@ fn main() {
1108 detail: "S", 1127 detail: "S",
1109 relevance: CompletionRelevance { 1128 relevance: CompletionRelevance {
1110 exact_name_match: true, 1129 exact_name_match: true,
1111 exact_type_match: false, 1130 type_match: None,
1112 is_local: true, 1131 is_local: true,
1113 }, 1132 },
1114 ref_match: "&mut ", 1133 ref_match: "&mut ",
@@ -1353,4 +1372,34 @@ fn foo(f: Foo) { let _: &u32 = f.b$0 }
1353 "#]], 1372 "#]],
1354 ); 1373 );
1355 } 1374 }
1375
1376 #[test]
1377 fn generic_enum() {
1378 check_relevance(
1379 r#"
1380enum Foo<T> { A(T), B }
1381// bar() should not be an exact type match
1382// because the generic parameters are different
1383fn bar() -> Foo<u8> { Foo::B }
1384// FIXME baz() should be an exact type match
1385// because the types could unify, but it currently
1386// is not. This is due to the T here being
1387// TyKind::Placeholder rather than TyKind::Missing.
1388fn baz<T>() -> Foo<T> { Foo::B }
1389fn foo() {
1390 let foo: Foo<u32> = Foo::B;
1391 let _: Foo<u32> = f$0;
1392}
1393"#,
1394 expect![[r#"
1395 ev Foo::A(…) [type_could_unify]
1396 ev Foo::B [type_could_unify]
1397 lc foo [type+local]
1398 en Foo []
1399 fn baz() []
1400 fn bar() []
1401 fn foo() []
1402 "#]],
1403 );
1404 }
1356} 1405}
diff --git a/crates/ide_completion/src/render/enum_variant.rs b/crates/ide_completion/src/render/enum_variant.rs
index 374247b05..832f5ced1 100644
--- a/crates/ide_completion/src/render/enum_variant.rs
+++ b/crates/ide_completion/src/render/enum_variant.rs
@@ -6,7 +6,7 @@ use itertools::Itertools;
6 6
7use crate::{ 7use crate::{
8 item::{CompletionItem, CompletionKind, ImportEdit}, 8 item::{CompletionItem, CompletionKind, ImportEdit},
9 render::{builder_ext::Params, compute_exact_type_match, compute_ref_match, RenderContext}, 9 render::{builder_ext::Params, compute_ref_match, compute_type_match, RenderContext},
10 CompletionRelevance, 10 CompletionRelevance,
11}; 11};
12 12
@@ -77,7 +77,7 @@ impl<'a> EnumRender<'a> {
77 77
78 let ty = self.variant.parent_enum(self.ctx.completion.db).ty(self.ctx.completion.db); 78 let ty = self.variant.parent_enum(self.ctx.completion.db).ty(self.ctx.completion.db);
79 item.set_relevance(CompletionRelevance { 79 item.set_relevance(CompletionRelevance {
80 exact_type_match: compute_exact_type_match(self.ctx.completion, &ty), 80 type_match: compute_type_match(self.ctx.completion, &ty),
81 ..CompletionRelevance::default() 81 ..CompletionRelevance::default()
82 }); 82 });
83 83
diff --git a/crates/ide_completion/src/render/function.rs b/crates/ide_completion/src/render/function.rs
index b1eba20e8..d681e2c91 100644
--- a/crates/ide_completion/src/render/function.rs
+++ b/crates/ide_completion/src/render/function.rs
@@ -8,7 +8,7 @@ use syntax::ast::Fn;
8use crate::{ 8use crate::{
9 item::{CompletionItem, CompletionItemKind, CompletionKind, CompletionRelevance, ImportEdit}, 9 item::{CompletionItem, CompletionItemKind, CompletionKind, CompletionRelevance, ImportEdit},
10 render::{ 10 render::{
11 builder_ext::Params, compute_exact_name_match, compute_exact_type_match, compute_ref_match, 11 builder_ext::Params, compute_exact_name_match, compute_ref_match, compute_type_match,
12 RenderContext, 12 RenderContext,
13 }, 13 },
14}; 14};
@@ -73,7 +73,7 @@ impl<'a> FunctionRender<'a> {
73 73
74 let ret_type = self.func.ret_type(self.ctx.db()); 74 let ret_type = self.func.ret_type(self.ctx.db());
75 item.set_relevance(CompletionRelevance { 75 item.set_relevance(CompletionRelevance {
76 exact_type_match: compute_exact_type_match(self.ctx.completion, &ret_type), 76 type_match: compute_type_match(self.ctx.completion, &ret_type),
77 exact_name_match: compute_exact_name_match(self.ctx.completion, self.name.clone()), 77 exact_name_match: compute_exact_name_match(self.ctx.completion, self.name.clone()),
78 ..CompletionRelevance::default() 78 ..CompletionRelevance::default()
79 }); 79 });
diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs
index 25169005f..530c8a5a4 100644
--- a/crates/rust-analyzer/src/to_proto.rs
+++ b/crates/rust-analyzer/src/to_proto.rs
@@ -1138,7 +1138,7 @@ mod tests {
1138 ( 1138 (
1139 "&arg", 1139 "&arg",
1140 Some( 1140 Some(
1141 "fffffffa", 1141 "fffffff9",
1142 ), 1142 ),
1143 ), 1143 ),
1144 ( 1144 (