From b2764a6641be5caef3d62dfb80c836e4976194f3 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 9 Mar 2021 20:24:09 +0300 Subject: Future proof completion scores --- crates/ide_completion/src/item.rs | 58 +++++++++++++++++---- crates/ide_completion/src/lib.rs | 5 +- crates/ide_completion/src/render.rs | 99 +++++++++++++++--------------------- 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 { /// after completion. trigger_call_info: bool, - /// Score is useful to pre select or display in better order completion items - score: Option, + /// We use this to sort completion. Relevance records facts like "do the + /// types align precisely?". We can't sort by relevances directly, they are + /// only partially ordered. + /// + /// Note that Relevance ignores fuzzy match score. We compute Relevance for + /// all possible items, and then separately build an ordered completion list + /// based on relevance and fuzzy matching with the already typed identifier. + relevance: Relevance, /// Indicates that a reference or mutable reference to this variable is a /// possible match. @@ -101,8 +107,8 @@ impl fmt::Debug for CompletionItem { if self.deprecated { s.field("deprecated", &true); } - if let Some(score) = &self.score { - s.field("score", score); + if self.relevance.is_relevant() { + s.field("relevance", &self.relevance); } if let Some(mutability) = &self.ref_match { s.field("ref_match", &format!("&{}", mutability.as_keyword_for_ref())); @@ -122,6 +128,36 @@ pub enum CompletionScore { TypeAndNameMatch, } +#[derive(Debug, Clone, Copy, Ord, PartialOrd, Eq, PartialEq, Default)] +pub struct Relevance { + /// This is set in cases like these: + /// + /// ``` + /// fn f(spam: String) {} + /// fn main { + /// let spam = 92; + /// f($0) // name of local matches the name of param + /// } + /// ``` + pub exact_name_match: bool, + /// 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 + /// } + /// ``` + pub exact_type_match: bool, +} + +impl Relevance { + pub fn is_relevant(&self) -> bool { + self != &Relevance::default() + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum CompletionItemKind { SymbolKind(SymbolKind), @@ -213,7 +249,7 @@ impl CompletionItem { text_edit: None, deprecated: false, trigger_call_info: None, - score: None, + relevance: Relevance::default(), ref_match: None, import_to_add: None, } @@ -256,8 +292,8 @@ impl CompletionItem { self.deprecated } - pub fn score(&self) -> Option { - self.score + pub fn relevance(&self) -> Relevance { + self.relevance } pub fn trigger_call_info(&self) -> bool { @@ -313,7 +349,7 @@ pub(crate) struct Builder { text_edit: Option, deprecated: bool, trigger_call_info: Option, - score: Option, + relevance: Relevance, ref_match: Option, } @@ -360,7 +396,7 @@ impl Builder { completion_kind: self.completion_kind, deprecated: self.deprecated, trigger_call_info: self.trigger_call_info.unwrap_or(false), - score: self.score, + relevance: self.relevance, ref_match: self.ref_match, import_to_add: self.import_to_add, } @@ -421,8 +457,8 @@ impl Builder { self.deprecated = deprecated; self } - pub(crate) fn set_score(mut self, score: CompletionScore) -> Builder { - self.score = Some(score); + pub(crate) fn set_relevance(mut self, relevance: Relevance) -> Builder { + self.relevance = relevance; self } 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 pub use crate::{ config::CompletionConfig, - item::{CompletionItem, CompletionItemKind, CompletionScore, ImportEdit, InsertTextFormat}, + item::{ + CompletionItem, CompletionItemKind, CompletionScore, ImportEdit, InsertTextFormat, + Relevance, + }, }; //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::{ use syntax::TextRange; use crate::{ - item::ImportEdit, CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, - CompletionScore, + item::{ImportEdit, Relevance}, + CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, }; use crate::render::{enum_variant::render_variant, function::render_fn, macro_::render_macro}; @@ -117,7 +117,7 @@ impl<'a> RenderContext<'a> { node.docs(self.db()) } - fn active_name_and_type(&self) -> Option<(String, Type)> { + fn expected_name_and_type(&self) -> Option<(String, Type)> { if let Some(record_field) = &self.completion.record_field_syntax { cov_mark::hit!(record_field_type_match); let (struct_field, _local) = self.completion.sema.resolve_record_field(record_field)?; @@ -155,8 +155,8 @@ impl<'a> Render<'a> { .set_documentation(field.docs(self.ctx.db())) .set_deprecated(is_deprecated); - if let Some(score) = compute_score(&self.ctx, &ty, &name.to_string()) { - item = item.set_score(score); + if let Some(relevance) = compute_relevance(&self.ctx, &ty, &name.to_string()) { + item = item.set_relevance(relevance); } item.build() @@ -247,18 +247,15 @@ impl<'a> Render<'a> { }; if let ScopeDef::Local(local) = resolution { - if let Some((active_name, active_type)) = self.ctx.active_name_and_type() { - let ty = local.ty(self.ctx.db()); - if let Some(score) = - compute_score_from_active(&active_type, &active_name, &ty, &local_name) - { - item = item.set_score(score); - } - - if let Some(ty_without_ref) = active_type.remove_ref() { + let ty = local.ty(self.ctx.db()); + if let Some(relevance) = compute_relevance(&self.ctx, &ty, &local_name) { + item = item.set_relevance(relevance) + } + if let Some((_expected_name, expected_type)) = self.ctx.expected_name_and_type() { + if let Some(ty_without_ref) = expected_type.remove_ref() { if ty_without_ref == ty { cov_mark::hit!(suggest_ref); - let mutability = if active_type.is_mutable_reference() { + let mutability = if expected_type.is_mutable_reference() { Mutability::Mut } else { Mutability::Shared @@ -326,33 +323,14 @@ impl<'a> Render<'a> { } } -fn compute_score_from_active( - active_type: &Type, - active_name: &str, - ty: &Type, - name: &str, -) -> Option { - // Compute score - // For the same type - if active_type != ty { - return None; - } - - let mut res = CompletionScore::TypeMatch; - - // If same type + same name then go top position - if active_name == name { - res = CompletionScore::TypeAndNameMatch - } - +fn compute_relevance(ctx: &RenderContext, ty: &Type, name: &str) -> Option { + let (expected_name, expected_type) = ctx.expected_name_and_type()?; + let mut res = Relevance::default(); + res.exact_type_match = ty == &expected_type; + res.exact_name_match = name == &expected_name; Some(res) } -fn compute_score(ctx: &RenderContext, ty: &Type, name: &str) -> Option { - let (active_name, active_type) = ctx.active_name_and_type()?; - compute_score_from_active(&active_type, &active_name, ty, name) -} - #[cfg(test)] mod tests { use std::cmp::Reverse; @@ -361,7 +339,7 @@ mod tests { use crate::{ test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG}, - CompletionKind, CompletionScore, + CompletionKind, Relevance, }; fn check(ra_fixture: &str, expect: Expect) { @@ -369,24 +347,25 @@ mod tests { expect.assert_debug_eq(&actual); } - fn check_scores(ra_fixture: &str, expect: Expect) { - fn display_score(score: Option) -> &'static str { - match score { - Some(CompletionScore::TypeMatch) => "[type]", - Some(CompletionScore::TypeAndNameMatch) => "[type+name]", - None => "[]".into(), + fn check_relevance(ra_fixture: &str, expect: Expect) { + fn display_relevance(relevance: Relevance) -> &'static str { + match relevance { + Relevance { exact_type_match: true, exact_name_match: true } => "[type+name]", + Relevance { exact_type_match: true, exact_name_match: false } => "[type]", + Relevance { exact_type_match: false, exact_name_match: true } => "[name]", + Relevance { exact_type_match: false, exact_name_match: false } => "[]", } } let mut completions = get_all_items(TEST_CONFIG, ra_fixture); - completions.sort_by_key(|it| (Reverse(it.score()), it.label().to_string())); + completions.sort_by_key(|it| (Reverse(it.relevance()), it.label().to_string())); let actual = completions .into_iter() .filter(|it| it.completion_kind == CompletionKind::Reference) .map(|it| { let tag = it.kind().unwrap().tag(); - let score = display_score(it.score()); - format!("{} {} {}\n", tag, it.label(), score) + let relevance = display_relevance(it.relevance()); + format!("{} {} {}\n", tag, it.label(), relevance) }) .collect::(); expect.assert_eq(&actual); @@ -849,9 +828,9 @@ fn foo(xs: Vec) } #[test] - fn active_param_score() { + fn active_param_relevance() { cov_mark::check!(active_param_type_match); - check_scores( + check_relevance( r#" struct S { foo: i64, bar: u32, baz: u32 } fn test(bar: u32) { } @@ -866,9 +845,9 @@ fn foo(s: S) { test(s.$0) } } #[test] - fn record_field_scores() { + fn record_field_relevances() { cov_mark::check!(record_field_type_match); - check_scores( + check_relevance( r#" struct A { foo: i64, bar: u32, baz: u32 } struct B { x: (), y: f32, bar: u32 } @@ -883,8 +862,8 @@ fn foo(a: A) { B { bar: a.$0 }; } } #[test] - fn record_field_and_call_scores() { - check_scores( + fn record_field_and_call_relevances() { + check_relevance( r#" struct A { foo: i64, bar: u32, baz: u32 } struct B { x: (), y: f32, bar: u32 } @@ -897,7 +876,7 @@ fn foo(a: A) { B { bar: f(a.$0) }; } fd baz [] "#]], ); - check_scores( + check_relevance( r#" struct A { foo: i64, bar: u32, baz: u32 } struct B { x: (), y: f32, bar: u32 } @@ -914,7 +893,7 @@ fn foo(a: A) { f(B { bar: a.$0 }); } #[test] fn prioritize_exact_ref_match() { - check_scores( + check_relevance( r#" struct WorldSnapshot { _f: () }; fn go(world: &WorldSnapshot) { go(w$0) } @@ -929,7 +908,7 @@ fn go(world: &WorldSnapshot) { go(w$0) } #[test] fn too_many_arguments() { - check_scores( + check_relevance( r#" struct Foo; fn f(foo: &Foo) { f(foo, w$0) } @@ -997,6 +976,10 @@ fn main() { Local, ), detail: "S", + relevance: Relevance { + exact_name_match: true, + exact_type_match: false, + }, ref_match: "&mut ", }, ] 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( ..Default::default() }; - if item.score().is_some() { + if item.relevance().is_relevant() { lsp_item.preselect = Some(true); // HACK: sort preselect items first lsp_item.sort_text = Some(format!(" {}", item.label())); @@ -1106,7 +1106,9 @@ mod tests { [ ( "&arg", - None, + Some( + " arg", + ), ), ( "arg", -- cgit v1.2.3