From 3679821eea94f30f2582ea7bca7569dad3ca31be Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Thu, 11 Mar 2021 19:11:14 -0800 Subject: add completion relevance score --- crates/ide_completion/src/item.rs | 55 +++++++++++++++++++++++++++++-------- crates/ide_completion/src/lib.rs | 4 +-- crates/ide_completion/src/render.rs | 22 ++++++++------- 3 files changed, 58 insertions(+), 23 deletions(-) (limited to 'crates/ide_completion/src') diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index cf1aaa131..9a4dc915c 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs @@ -70,7 +70,7 @@ pub struct CompletionItem { /// 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, + relevance: CompletionRelevance, /// Indicates that a reference or mutable reference to this variable is a /// possible match. @@ -107,9 +107,11 @@ impl fmt::Debug for CompletionItem { if self.deprecated { s.field("deprecated", &true); } - if self.relevance.is_relevant() { + + if self.relevance != CompletionRelevance::default() { s.field("relevance", &self.relevance); } + if let Some(mutability) = &self.ref_match { s.field("ref_match", &format!("&{}", mutability.as_keyword_for_ref())); } @@ -129,7 +131,7 @@ pub enum CompletionScore { } #[derive(Debug, Clone, Copy, Ord, PartialOrd, Eq, PartialEq, Default)] -pub struct Relevance { +pub struct CompletionRelevance { /// This is set in cases like these: /// /// ``` @@ -152,9 +154,34 @@ pub struct Relevance { pub exact_type_match: bool, } -impl Relevance { +impl CompletionRelevance { + /// Provides a relevance score. Higher values are more relevant. + /// + /// The absolute value of the relevance score is not meaningful, for + /// example a value of 0 doesn't mean "not relevant", rather + /// it means "least relevant". The score value should only be used + /// for relative ordering. + /// + /// See is_relevant if you need to make some judgement about score + /// in an absolute sense. + pub fn score(&self) -> u8 { + let mut score = 0; + + if self.exact_name_match { + score += 1; + } + if self.exact_type_match { + score += 1; + } + + score + } + + /// Returns true when the score for this threshold is above + /// some threshold such that we think it is especially likely + /// to be relevant. pub fn is_relevant(&self) -> bool { - self != &Relevance::default() + self.score() > 0 } } @@ -249,7 +276,7 @@ impl CompletionItem { text_edit: None, deprecated: false, trigger_call_info: None, - relevance: Relevance::default(), + relevance: CompletionRelevance::default(), ref_match: None, import_to_add: None, } @@ -292,7 +319,7 @@ impl CompletionItem { self.deprecated } - pub fn relevance(&self) -> Relevance { + pub fn relevance(&self) -> CompletionRelevance { self.relevance } @@ -300,8 +327,14 @@ impl CompletionItem { self.trigger_call_info } - pub fn ref_match(&self) -> Option { - self.ref_match + pub fn ref_match(&self) -> Option<(Mutability, CompletionRelevance)> { + // Relevance of the ref match should be the same as the original + // 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; + + self.ref_match.map(|mutability| (mutability, relevance)) } pub fn import_to_add(&self) -> Option<&ImportEdit> { @@ -349,7 +382,7 @@ pub(crate) struct Builder { text_edit: Option, deprecated: bool, trigger_call_info: Option, - relevance: Relevance, + relevance: CompletionRelevance, ref_match: Option, } @@ -457,7 +490,7 @@ impl Builder { self.deprecated = deprecated; self } - pub(crate) fn set_relevance(&mut self, relevance: Relevance) -> &mut Builder { + pub(crate) fn set_relevance(&mut self, relevance: CompletionRelevance) -> &mut Builder { self.relevance = relevance; self } diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs index d46f521a0..21e489755 100644 --- a/crates/ide_completion/src/lib.rs +++ b/crates/ide_completion/src/lib.rs @@ -24,8 +24,8 @@ use crate::{completions::Completions, context::CompletionContext, item::Completi pub use crate::{ config::CompletionConfig, item::{ - CompletionItem, CompletionItemKind, CompletionScore, ImportEdit, InsertTextFormat, - Relevance, + CompletionItem, CompletionItemKind, CompletionRelevance, CompletionScore, ImportEdit, + InsertTextFormat, }, }; diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index f7f9084d9..db31896e5 100644 --- a/crates/ide_completion/src/render.rs +++ b/crates/ide_completion/src/render.rs @@ -20,7 +20,7 @@ use ide_db::{ use syntax::TextRange; use crate::{ - item::{ImportEdit, Relevance}, + item::{CompletionRelevance, ImportEdit}, CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, }; @@ -322,9 +322,9 @@ impl<'a> Render<'a> { } } -fn compute_relevance(ctx: &RenderContext, ty: &Type, name: &str) -> Option { +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(); + let mut res = CompletionRelevance::default(); res.exact_type_match = ty == &expected_type; res.exact_name_match = name == &expected_name; Some(res) @@ -338,7 +338,7 @@ mod tests { use crate::{ test_utils::{check_edit, do_completion, get_all_items, TEST_CONFIG}, - CompletionKind, Relevance, + CompletionKind, CompletionRelevance, }; fn check(ra_fixture: &str, expect: Expect) { @@ -347,12 +347,14 @@ mod tests { } fn check_relevance(ra_fixture: &str, expect: Expect) { - fn display_relevance(relevance: Relevance) -> &'static str { + fn display_relevance(relevance: CompletionRelevance) -> &'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 } => "[]", + CompletionRelevance { exact_type_match: true, exact_name_match: true } => { + "[type+name]" + } + CompletionRelevance { exact_type_match: true, exact_name_match: false } => "[type]", + CompletionRelevance { exact_type_match: false, exact_name_match: true } => "[name]", + CompletionRelevance { exact_type_match: false, exact_name_match: false } => "[]", } } @@ -975,7 +977,7 @@ fn main() { Local, ), detail: "S", - relevance: Relevance { + relevance: CompletionRelevance { exact_name_match: true, exact_type_match: false, }, -- cgit v1.2.3 From 9ee3914c61f0476d28d3f74f046f487900e2a6fb Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Thu, 11 Mar 2021 19:13:27 -0800 Subject: remove unused CompletionScore enum --- crates/ide_completion/src/item.rs | 8 -------- crates/ide_completion/src/lib.rs | 5 +---- 2 files changed, 1 insertion(+), 12 deletions(-) (limited to 'crates/ide_completion/src') diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 9a4dc915c..30317842d 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs @@ -122,14 +122,6 @@ impl fmt::Debug for CompletionItem { } } -#[derive(Debug, Clone, Copy, Ord, PartialOrd, Eq, PartialEq)] -pub enum CompletionScore { - /// If only type match - TypeMatch, - /// If type and name match - TypeAndNameMatch, -} - #[derive(Debug, Clone, Copy, Ord, PartialOrd, Eq, PartialEq, Default)] pub struct CompletionRelevance { /// This is set in cases like these: diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs index 21e489755..263554ecf 100644 --- a/crates/ide_completion/src/lib.rs +++ b/crates/ide_completion/src/lib.rs @@ -23,10 +23,7 @@ use crate::{completions::Completions, context::CompletionContext, item::Completi pub use crate::{ config::CompletionConfig, - item::{ - CompletionItem, CompletionItemKind, CompletionRelevance, CompletionScore, ImportEdit, - InsertTextFormat, - }, + item::{CompletionItem, CompletionItemKind, CompletionRelevance, ImportEdit, InsertTextFormat}, }; //FIXME: split the following feature into fine-grained features. -- cgit v1.2.3 From 10fb065b146d7d03a65bbc73cdb34d32523dafa1 Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Thu, 11 Mar 2021 20:39:25 -0800 Subject: add relevance score test --- crates/ide_completion/src/item.rs | 60 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) (limited to 'crates/ide_completion/src') diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 30317842d..352640e3b 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs @@ -499,3 +499,63 @@ impl Builder { self } } + +#[cfg(test)] +mod tests { + use itertools::Itertools; + use test_utils::assert_eq_text; + + use super::CompletionRelevance; + + /// Check that these are CompletionRelevance are sorted in ascending order + /// by their relevance score. + /// + /// We want to avoid making assertions about the absolute score of any + /// item, but we do want to assert whether each is >, <, or == to the + /// others. + /// + /// If provided vec![vec![a], vec![b, c], vec![d]], then this will assert: + /// a.score < b.score == c.score < d.score + fn check_relevance_score_ordered(expected_relevance_order: Vec>) { + let expected = format!("{:#?}", &expected_relevance_order); + + let actual_relevance_order = expected_relevance_order + .into_iter() + .flatten() + .map(|r| (r.score(), r)) + .sorted_by_key(|(score, _r)| *score) + .fold( + (u8::MIN, vec![vec![]]), + |(mut currently_collecting_score, mut out), (score, r)| { + if currently_collecting_score == score { + out.last_mut().unwrap().push(r); + } else { + currently_collecting_score = score; + out.push(vec![r]); + } + (currently_collecting_score, out) + }, + ) + .1; + + let actual = format!("{:#?}", &actual_relevance_order); + + assert_eq_text!(&expected, &actual); + } + + #[test] + fn relevance_score() { + // This test asserts that the relevance score for these items is ascending, and + // that any items in the same vec have the same score. + let expected_relevance_order = vec![ + vec![CompletionRelevance::default()], + vec![ + CompletionRelevance { exact_name_match: true, ..CompletionRelevance::default() }, + CompletionRelevance { exact_type_match: true, ..CompletionRelevance::default() }, + ], + vec![CompletionRelevance { exact_name_match: true, exact_type_match: true }], + ]; + + check_relevance_score_ordered(expected_relevance_order); + } +} -- cgit v1.2.3 From acbe297fbd10250e0d99d1e3e98751dd6ef77adc Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Fri, 12 Mar 2021 06:08:07 -0800 Subject: update relevance score u8 -> u32 --- crates/ide_completion/src/item.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'crates/ide_completion/src') diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 352640e3b..3febab32b 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs @@ -156,7 +156,7 @@ impl CompletionRelevance { /// /// See is_relevant if you need to make some judgement about score /// in an absolute sense. - pub fn score(&self) -> u8 { + pub fn score(&self) -> u32 { let mut score = 0; if self.exact_name_match { @@ -525,7 +525,7 @@ mod tests { .map(|r| (r.score(), r)) .sorted_by_key(|(score, _r)| *score) .fold( - (u8::MIN, vec![vec![]]), + (u32::MIN, vec![vec![]]), |(mut currently_collecting_score, mut out), (score, r)| { if currently_collecting_score == score { out.last_mut().unwrap().push(r); -- cgit v1.2.3