From 0e31ae2cef92f973a6ee3def3f1feaa94dcb7a4f Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Mon, 22 Mar 2021 21:18:34 -0700 Subject: completion relevance distinguish between exact type match and could unify --- crates/ide_completion/src/item.rs | 59 ++++++++++++++++-------- crates/ide_completion/src/render.rs | 56 ++++++++++++++-------- crates/ide_completion/src/render/enum_variant.rs | 4 +- crates/ide_completion/src/render/function.rs | 4 +- crates/rust-analyzer/src/to_proto.rs | 2 +- 5 files changed, 81 insertions(+), 44 deletions(-) (limited to 'crates') 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 { } } -#[derive(Debug, Clone, Copy, Ord, PartialOrd, Eq, PartialEq, Default)] +#[derive(Debug, Clone, Copy, Eq, PartialEq, Default)] pub struct CompletionRelevance { /// This is set in cases like these: /// @@ -134,31 +134,41 @@ pub struct CompletionRelevance { /// } /// ``` pub exact_name_match: bool, + /// See CompletionRelevanceTypeMatch doc comments for cases where this is set. + pub type_match: Option, /// This is set in cases like these: /// /// ``` - /// fn f(spam: String) {} - /// fn main { - /// let foo = String::new(); - /// f($0) // type of local matches the type of param + /// fn foo(a: u32) { + /// let b = 0; + /// $0 // `a` and `b` are local /// } /// ``` - pub exact_type_match: bool, + pub is_local: bool, +} + +#[derive(Debug, Clone, Copy, Eq, PartialEq)] +pub enum CompletionRelevanceTypeMatch { /// This is set in cases like these: /// /// ``` - /// fn foo(bar: u32) { - /// $0 // `bar` is local + /// enum Option { Some(T), None } + /// fn f(a: Option) {} + /// fn main { + /// f(Option::N$0) // type `Option` could unify with `Option` /// } /// ``` + CouldUnify, + /// This is set in cases like these: /// /// ``` - /// fn foo() { - /// let bar = 0; - /// $0 // `bar` is local + /// fn f(spam: String) {} + /// fn main { + /// let foo = String::new(); + /// f($0) // type of local matches the type of param /// } /// ``` - pub is_local: bool, + Exact, } impl CompletionRelevance { @@ -177,9 +187,11 @@ impl CompletionRelevance { if self.exact_name_match { score += 1; } - if self.exact_type_match { - score += 3; - } + score += match self.type_match { + Some(CompletionRelevanceTypeMatch::Exact) => 4, + Some(CompletionRelevanceTypeMatch::CouldUnify) => 3, + None => 0, + }; if self.is_local { score += 1; } @@ -342,7 +354,7 @@ impl CompletionItem { // match, but with exact type match set because self.ref_match // is only set if there is an exact type match. let mut relevance = self.relevance; - relevance.exact_type_match = true; + relevance.type_match = Some(CompletionRelevanceTypeMatch::Exact); self.ref_match.map(|mutability| (mutability, relevance)) } @@ -523,7 +535,7 @@ mod tests { use itertools::Itertools; use test_utils::assert_eq_text; - use super::CompletionRelevance; + use super::{CompletionRelevance, CompletionRelevanceTypeMatch}; /// Check that these are CompletionRelevance are sorted in ascending order /// by their relevance score. @@ -576,15 +588,22 @@ mod tests { is_local: true, ..CompletionRelevance::default() }], - vec![CompletionRelevance { exact_type_match: true, ..CompletionRelevance::default() }], + vec![CompletionRelevance { + type_match: Some(CompletionRelevanceTypeMatch::CouldUnify), + ..CompletionRelevance::default() + }], + vec![CompletionRelevance { + type_match: Some(CompletionRelevanceTypeMatch::Exact), + ..CompletionRelevance::default() + }], vec![CompletionRelevance { exact_name_match: true, - exact_type_match: true, + type_match: Some(CompletionRelevanceTypeMatch::Exact), ..CompletionRelevance::default() }], vec![CompletionRelevance { exact_name_match: true, - exact_type_match: true, + type_match: Some(CompletionRelevanceTypeMatch::Exact), is_local: true, }], ]; diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index 12453f889..1a762d3dc 100644 --- a/crates/ide_completion/src/render.rs +++ b/crates/ide_completion/src/render.rs @@ -20,8 +20,8 @@ use ide_db::{ use syntax::TextRange; use crate::{ - item::ImportEdit, CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, - CompletionRelevance, + item::{CompletionRelevanceTypeMatch, ImportEdit}, + CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, CompletionRelevance, }; use crate::render::{enum_variant::render_variant, function::render_fn, macro_::render_macro}; @@ -143,7 +143,7 @@ impl<'a> Render<'a> { .set_deprecated(is_deprecated); item.set_relevance(CompletionRelevance { - exact_type_match: compute_exact_type_match(self.ctx.completion, ty), + type_match: compute_type_match(self.ctx.completion, ty), exact_name_match: compute_exact_name_match(self.ctx.completion, name.to_string()), ..CompletionRelevance::default() }); @@ -245,7 +245,7 @@ impl<'a> Render<'a> { } item.set_relevance(CompletionRelevance { - exact_type_match: compute_exact_type_match(self.ctx.completion, &ty), + type_match: compute_type_match(self.ctx.completion, &ty), exact_name_match: compute_exact_name_match(self.ctx.completion, &local_name), is_local: true, ..CompletionRelevance::default() @@ -309,15 +309,24 @@ impl<'a> Render<'a> { } } -fn compute_exact_type_match(ctx: &CompletionContext, completion_ty: &hir::Type) -> bool { - match ctx.expected_type.as_ref() { - Some(expected_type) => { - // We don't ever consider unit type to be an exact type match, since - // nearly always this is not meaningful to the user. - (completion_ty == expected_type || expected_type.could_unify_with(completion_ty)) - && !expected_type.is_unit() - } - None => false, +fn compute_type_match( + ctx: &CompletionContext, + completion_ty: &hir::Type, +) -> Option { + let expected_type = ctx.expected_type.as_ref()?; + + // We don't ever consider unit type to be an exact type match, since + // nearly always this is not meaningful to the user. + if expected_type.is_unit() { + return None; + } + + if completion_ty == expected_type { + Some(CompletionRelevanceTypeMatch::Exact) + } else if expected_type.could_unify_with(completion_ty) { + Some(CompletionRelevanceTypeMatch::CouldUnify) + } else { + None } } @@ -349,6 +358,7 @@ mod tests { use itertools::Itertools; use crate::{ + item::CompletionRelevanceTypeMatch, test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG}, CompletionKind, CompletionRelevance, }; @@ -361,7 +371,11 @@ mod tests { fn check_relevance(ra_fixture: &str, expect: Expect) { fn display_relevance(relevance: CompletionRelevance) -> String { let relevance_factors = vec![ - (relevance.exact_type_match, "type"), + (relevance.type_match == Some(CompletionRelevanceTypeMatch::Exact), "type"), + ( + relevance.type_match == Some(CompletionRelevanceTypeMatch::CouldUnify), + "type_could_unify", + ), (relevance.exact_name_match, "name"), (relevance.is_local, "local"), ] @@ -534,7 +548,9 @@ fn main() { let _: m::Spam = S$0 } detail: "(i32)", relevance: CompletionRelevance { exact_name_match: false, - exact_type_match: true, + type_match: Some( + Exact, + ), is_local: false, }, trigger_call_info: true, @@ -560,7 +576,9 @@ fn main() { let _: m::Spam = S$0 } detail: "()", relevance: CompletionRelevance { exact_name_match: false, - exact_type_match: true, + type_match: Some( + Exact, + ), is_local: false, }, }, @@ -1109,7 +1127,7 @@ fn main() { detail: "S", relevance: CompletionRelevance { exact_name_match: true, - exact_type_match: false, + type_match: None, is_local: true, }, ref_match: "&mut ", @@ -1374,8 +1392,8 @@ fn foo() { } "#, expect![[r#" - ev Foo::A(…) [type] - ev Foo::B [type] + ev Foo::A(…) [type_could_unify] + ev Foo::B [type_could_unify] lc foo [type+local] en Foo [] fn baz() [] 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; use crate::{ item::{CompletionItem, CompletionKind, ImportEdit}, - render::{builder_ext::Params, compute_exact_type_match, compute_ref_match, RenderContext}, + render::{builder_ext::Params, compute_ref_match, compute_type_match, RenderContext}, CompletionRelevance, }; @@ -77,7 +77,7 @@ impl<'a> EnumRender<'a> { let ty = self.variant.parent_enum(self.ctx.completion.db).ty(self.ctx.completion.db); item.set_relevance(CompletionRelevance { - exact_type_match: compute_exact_type_match(self.ctx.completion, &ty), + type_match: compute_type_match(self.ctx.completion, &ty), ..CompletionRelevance::default() }); 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; use crate::{ item::{CompletionItem, CompletionItemKind, CompletionKind, CompletionRelevance, ImportEdit}, render::{ - builder_ext::Params, compute_exact_name_match, compute_exact_type_match, compute_ref_match, + builder_ext::Params, compute_exact_name_match, compute_ref_match, compute_type_match, RenderContext, }, }; @@ -73,7 +73,7 @@ impl<'a> FunctionRender<'a> { let ret_type = self.func.ret_type(self.ctx.db()); item.set_relevance(CompletionRelevance { - exact_type_match: compute_exact_type_match(self.ctx.completion, &ret_type), + type_match: compute_type_match(self.ctx.completion, &ret_type), exact_name_match: compute_exact_name_match(self.ctx.completion, self.name.clone()), ..CompletionRelevance::default() }); 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 { ( "&arg", Some( - "fffffffa", + "fffffff9", ), ), ( -- cgit v1.2.3