1 files changed, 166 insertions, 45 deletions

diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs
index 9b039e3e5..3febab32b 100644
--- a/crates/ide_completion/src/item.rs
+++ b/crates/ide_completion/src/item.rs
@@ -63,12 +63,18 @@ pub struct CompletionItem {
     /// after completion.
     trigger_call_info: bool,
 
-    /// Score is useful to pre select or display in better order completion items
-    score: Option<CompletionScore>,
+    /// 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: CompletionRelevance,
 
     /// Indicates that a reference or mutable reference to this variable is a
     /// possible match.
-    ref_match: Option<(Mutability, CompletionScore)>,
+    ref_match: Option<Mutability>,
 
     /// The import data to add to completion's edits.
     import_to_add: Option<ImportEdit>,
@@ -101,8 +107,13 @@ 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 != CompletionRelevance::default() {
+            s.field("relevance", &self.relevance);
+        }
+
+        if let Some(mutability) = &self.ref_match {
+            s.field("ref_match", &format!("&{}", mutability.as_keyword_for_ref()));
         }
         if self.trigger_call_info {
             s.field("trigger_call_info", &true);
@@ -111,12 +122,59 @@ 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:
+    ///
+    /// ```
+    /// 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 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) -> u32 {
+        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.score() > 0
+    }
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -208,9 +266,9 @@ impl CompletionItem {
             lookup: None,
             kind: None,
             text_edit: None,
-            deprecated: None,
+            deprecated: false,
             trigger_call_info: None,
-            score: None,
+            relevance: CompletionRelevance::default(),
             ref_match: None,
             import_to_add: None,
         }
@@ -253,16 +311,22 @@ impl CompletionItem {
         self.deprecated
     }
 
-    pub fn score(&self) -> Option<CompletionScore> {
-        self.score
+    pub fn relevance(&self) -> CompletionRelevance {
+        self.relevance
     }
 
     pub fn trigger_call_info(&self) -> bool {
         self.trigger_call_info
     }
 
-    pub fn ref_match(&self) -> Option<(Mutability, CompletionScore)> {
-        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> {
@@ -308,10 +372,10 @@ pub(crate) struct Builder {
     lookup: Option<String>,
     kind: Option<CompletionItemKind>,
     text_edit: Option<TextEdit>,
-    deprecated: Option<bool>,
+    deprecated: bool,
     trigger_call_info: Option<bool>,
-    score: Option<CompletionScore>,
-    ref_match: Option<(Mutability, CompletionScore)>,
+    relevance: CompletionRelevance,
+    ref_match: Option<Mutability>,
 }
 
 impl Builder {
@@ -355,49 +419,49 @@ impl Builder {
             lookup,
             kind: self.kind,
             completion_kind: self.completion_kind,
-            deprecated: self.deprecated.unwrap_or(false),
+            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,
         }
     }
-    pub(crate) fn lookup_by(mut self, lookup: impl Into<String>) -> Builder {
+    pub(crate) fn lookup_by(&mut self, lookup: impl Into<String>) -> &mut Builder {
         self.lookup = Some(lookup.into());
         self
     }
-    pub(crate) fn label(mut self, label: impl Into<String>) -> Builder {
+    pub(crate) fn label(&mut self, label: impl Into<String>) -> &mut Builder {
         self.label = label.into();
         self
     }
-    pub(crate) fn insert_text(mut self, insert_text: impl Into<String>) -> Builder {
+    pub(crate) fn insert_text(&mut self, insert_text: impl Into<String>) -> &mut Builder {
         self.insert_text = Some(insert_text.into());
         self
     }
     pub(crate) fn insert_snippet(
-        mut self,
+        &mut self,
         _cap: SnippetCap,
         snippet: impl Into<String>,
-    ) -> Builder {
+    ) -> &mut Builder {
         self.insert_text_format = InsertTextFormat::Snippet;
         self.insert_text(snippet)
     }
-    pub(crate) fn kind(mut self, kind: impl Into<CompletionItemKind>) -> Builder {
+    pub(crate) fn kind(&mut self, kind: impl Into<CompletionItemKind>) -> &mut Builder {
         self.kind = Some(kind.into());
         self
     }
-    pub(crate) fn text_edit(mut self, edit: TextEdit) -> Builder {
+    pub(crate) fn text_edit(&mut self, edit: TextEdit) -> &mut Builder {
         self.text_edit = Some(edit);
         self
     }
-    pub(crate) fn snippet_edit(mut self, _cap: SnippetCap, edit: TextEdit) -> Builder {
+    pub(crate) fn snippet_edit(&mut self, _cap: SnippetCap, edit: TextEdit) -> &mut Builder {
         self.insert_text_format = InsertTextFormat::Snippet;
         self.text_edit(edit)
     }
-    pub(crate) fn detail(self, detail: impl Into<String>) -> Builder {
+    pub(crate) fn detail(&mut self, detail: impl Into<String>) -> &mut Builder {
         self.set_detail(Some(detail))
     }
-    pub(crate) fn set_detail(mut self, detail: Option<impl Into<String>>) -> Builder {
+    pub(crate) fn set_detail(&mut self, detail: Option<impl Into<String>>) -> &mut Builder {
         self.detail = detail.map(Into::into);
         if let Some(detail) = &self.detail {
             if never!(detail.contains('\n'), "multiline detail:\n{}", detail) {
@@ -407,34 +471,91 @@ impl Builder {
         self
     }
     #[allow(unused)]
-    pub(crate) fn documentation(self, docs: Documentation) -> Builder {
+    pub(crate) fn documentation(&mut self, docs: Documentation) -> &mut Builder {
         self.set_documentation(Some(docs))
     }
-    pub(crate) fn set_documentation(mut self, docs: Option<Documentation>) -> Builder {
+    pub(crate) fn set_documentation(&mut self, docs: Option<Documentation>) -> &mut Builder {
         self.documentation = docs.map(Into::into);
         self
     }
-    pub(crate) fn set_deprecated(mut self, deprecated: bool) -> Builder {
-        self.deprecated = Some(deprecated);
+    pub(crate) fn set_deprecated(&mut self, deprecated: bool) -> &mut 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: CompletionRelevance) -> &mut Builder {
+        self.relevance = relevance;
         self
     }
-    pub(crate) fn trigger_call_info(mut self) -> Builder {
+    pub(crate) fn trigger_call_info(&mut self) -> &mut Builder {
         self.trigger_call_info = Some(true);
         self
     }
-    pub(crate) fn add_import(mut self, import_to_add: Option<ImportEdit>) -> Builder {
+    pub(crate) fn add_import(&mut self, import_to_add: Option<ImportEdit>) -> &mut Builder {
         self.import_to_add = import_to_add;
         self
     }
-    pub(crate) fn set_ref_match(
-        mut self,
-        ref_match: Option<(Mutability, CompletionScore)>,
-    ) -> Builder {
-        self.ref_match = ref_match;
+    pub(crate) fn ref_match(&mut self, mutability: Mutability) -> &mut Builder {
+        self.ref_match = Some(mutability);
         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<Vec<CompletionRelevance>>) {
+        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(
+                (u32::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);
+    }
+}