feat: improve dot completions with scoring

Signed-off-by: Benjamin Coenen <[email protected]>

6 files changed, 91 insertions, 108 deletions

diff --git a/crates/ra_ide/src/completion.rs b/crates/ra_ide/src/completion.rs
index 185450508..38c8aed8d 100644
--- a/crates/ra_ide/src/completion.rs
+++ b/crates/ra_ide/src/completion.rs
@@ -29,7 +29,7 @@ use crate::{
 };
 
 pub use crate::completion::completion_item::{
-    CompletionItem, CompletionItemKind, InsertTextFormat,
+    CompletionItem, CompletionItemKind, CompletionScore, InsertTextFormat,
 };
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -94,8 +94,8 @@ pub(crate) fn completions(
     complete_macro_in_item_position::complete_macro_in_item_position(&mut acc, &ctx);
     complete_trait_impl::complete_trait_impl(&mut acc, &ctx);
 
-    // Reorder completion items if there is a sort_option
-    acc.sort(&ctx);
+    // Compute score for completion items
+    acc.compute_score(&ctx);
 
     Some(acc)
 }
diff --git a/crates/ra_ide/src/completion/complete_dot.rs b/crates/ra_ide/src/completion/complete_dot.rs
index 2e228b638..174b39964 100644
--- a/crates/ra_ide/src/completion/complete_dot.rs
+++ b/crates/ra_ide/src/completion/complete_dot.rs
@@ -6,12 +6,11 @@ use hir::{
     Type,
 };
 
-use crate::completion::completion_item::CompletionKind;
 use crate::{
     call_info::call_info,
     completion::{
         completion_context::CompletionContext,
-        completion_item::{Completions, SortOption},
+        completion_item::{CompletionKind, Completions, ScoreOption},
     },
     // CallInfo,
     CompletionItem,
@@ -49,40 +48,12 @@ fn complete_fields(acc: &mut Completions, ctx: &CompletionContext, receiver: &Ty
     for receiver in receiver.autoderef(ctx.db) {
         let fields = receiver.fields(ctx.db);
 
-        // If we use this implementation we can delete call_info in the CompletionContext
         if let Some(record_field) = &ctx.record_field_syntax {
-            acc.with_sort_option(SortOption::RecordField(record_field.clone()));
+            acc.with_score_option(ScoreOption::RecordField(record_field.clone()));
         } else if let Some(call_info) = call_info(ctx.db, ctx.file_position) {
-            acc.with_sort_option(SortOption::CallFn(call_info));
+            acc.with_score_option(ScoreOption::CallFn(call_info));
         }
 
-        // // For Call Fn
-        // if let Some(call_info) = &ctx.call_info {
-        //     if let Some(active_parameter_type) = call_info.active_parameter_type() {
-        //         let active_parameter_name = call_info.active_parameter_name().unwrap();
-        //         fields.sort_by(|a, b| {
-        //             // For the same type
-        //             if active_parameter_type == a.1.display(ctx.db).to_string() {
-        //                 // If same type + same name then go top position
-        //                 if active_parameter_name == a.0.name(ctx.db).to_string() {
-        //                     Ordering::Less
-        //                 } else {
-        //                     if active_parameter_type == b.1.display(ctx.db).to_string() {
-        //                         Ordering::Equal
-        //                     } else {
-        //                         Ordering::Less
-        //                     }
-        //                 }
-        //             } else {
-        //                 Ordering::Greater
-        //             }
-        //         });
-        //     }
-        // }
-
-        // For Lit struct fields
-        // ---
-
         for (field, ty) in fields {
             if ctx.scope().module().map_or(false, |m| !field.is_visible_from(ctx.db, m)) {
                 // Skip private field. FIXME: If the definition location of the
@@ -116,20 +87,13 @@ fn complete_methods(acc: &mut Completions, ctx: &CompletionContext, receiver: &T
 
 #[cfg(test)]
 mod tests {
-    use crate::completion::{
-        test_utils::{do_completion, do_completion_without_sort},
-        CompletionItem, CompletionKind,
-    };
+    use crate::completion::{test_utils::do_completion, CompletionItem, CompletionKind};
     use insta::assert_debug_snapshot;
 
     fn do_ref_completion(code: &str) -> Vec<CompletionItem> {
         do_completion(code, CompletionKind::Reference)
     }
 
-    fn do_ref_completion_without_sort(code: &str) -> Vec<CompletionItem> {
-        do_completion_without_sort(code, CompletionKind::Reference)
-    }
-
     #[test]
     fn test_struct_field_completion() {
         assert_debug_snapshot!(
@@ -159,7 +123,7 @@ mod tests {
     #[test]
     fn test_struct_field_completion_in_func_call() {
         assert_debug_snapshot!(
-        do_ref_completion_without_sort(
+        do_ref_completion(
                 r"
                 struct A { another_field: i64, the_field: u32, my_string: String }
                 fn test(my_param: u32) -> u32 { my_param }
@@ -171,14 +135,6 @@ mod tests {
             @r###"
         [
             CompletionItem {
-                label: "the_field",
-                source_range: [201; 201),
-                delete: [201; 201),
-                insert: "the_field",
-                kind: Field,
-                detail: "u32",
-            },
-            CompletionItem {
                 label: "another_field",
                 source_range: [201; 201),
                 delete: [201; 201),
@@ -194,6 +150,15 @@ mod tests {
                 kind: Field,
                 detail: "{unknown}",
             },
+            CompletionItem {
+                label: "the_field",
+                source_range: [201; 201),
+                delete: [201; 201),
+                insert: "the_field",
+                kind: Field,
+                detail: "u32",
+                score: TypeMatch,
+            },
         ]
         "###
         );
@@ -202,7 +167,7 @@ mod tests {
     #[test]
     fn test_struct_field_completion_in_func_call_with_type_and_name() {
         assert_debug_snapshot!(
-        do_ref_completion_without_sort(
+        do_ref_completion(
                 r"
                 struct A { another_field: i64, another_good_type: u32, the_field: u32 }
                 fn test(the_field: u32) -> u32 { the_field }
@@ -214,12 +179,12 @@ mod tests {
             @r###"
         [
             CompletionItem {
-                label: "the_field",
+                label: "another_field",
                 source_range: [208; 208),
                 delete: [208; 208),
-                insert: "the_field",
+                insert: "another_field",
                 kind: Field,
-                detail: "u32",
+                detail: "i64",
             },
             CompletionItem {
                 label: "another_good_type",
@@ -228,14 +193,16 @@ mod tests {
                 insert: "another_good_type",
                 kind: Field,
                 detail: "u32",
+                score: TypeMatch,
             },
             CompletionItem {
-                label: "another_field",
+                label: "the_field",
                 source_range: [208; 208),
                 delete: [208; 208),
-                insert: "another_field",
+                insert: "the_field",
                 kind: Field,
-                detail: "i64",
+                detail: "u32",
+                score: TypeAndNameMatch,
             },
         ]
         "###
@@ -245,7 +212,7 @@ mod tests {
     #[test]
     fn test_struct_field_completion_in_record_lit() {
         assert_debug_snapshot!(
-        do_ref_completion_without_sort(
+        do_ref_completion(
                 r"
                 struct A { another_field: i64, another_good_type: u32, the_field: u32 }
                 struct B { my_string: String, my_vec: Vec<u32>, the_field: u32 }
@@ -259,12 +226,12 @@ mod tests {
             @r###"
         [
             CompletionItem {
-                label: "the_field",
+                label: "another_field",
                 source_range: [270; 270),
                 delete: [270; 270),
-                insert: "the_field",
+                insert: "another_field",
                 kind: Field,
-                detail: "u32",
+                detail: "i64",
             },
             CompletionItem {
                 label: "another_good_type",
@@ -273,14 +240,16 @@ mod tests {
                 insert: "another_good_type",
                 kind: Field,
                 detail: "u32",
+                score: TypeMatch,
             },
             CompletionItem {
-                label: "another_field",
+                label: "the_field",
                 source_range: [270; 270),
                 delete: [270; 270),
-                insert: "another_field",
+                insert: "the_field",
                 kind: Field,
-                detail: "i64",
+                detail: "u32",
+                score: TypeAndNameMatch,
             },
         ]
         "###
@@ -290,7 +259,7 @@ mod tests {
     #[test]
     fn test_struct_field_completion_in_record_lit_and_fn_call() {
         assert_debug_snapshot!(
-        do_ref_completion_without_sort(
+        do_ref_completion(
                 r"
                 struct A { another_field: i64, another_good_type: u32, the_field: u32 }
                 struct B { my_string: String, my_vec: Vec<u32>, the_field: u32 }
@@ -311,6 +280,7 @@ mod tests {
                 insert: "another_field",
                 kind: Field,
                 detail: "i64",
+                score: TypeMatch,
             },
             CompletionItem {
                 label: "another_good_type",
@@ -336,7 +306,7 @@ mod tests {
     #[test]
     fn test_struct_field_completion_in_fn_call_and_record_lit() {
         assert_debug_snapshot!(
-        do_ref_completion_without_sort(
+        do_ref_completion(
                 r"
                 struct A { another_field: i64, another_good_type: u32, the_field: u32 }
                 struct B { my_string: String, my_vec: Vec<u32>, the_field: u32 }
@@ -351,12 +321,12 @@ mod tests {
             @r###"
         [
             CompletionItem {
-                label: "the_field",
+                label: "another_field",
                 source_range: [328; 328),
                 delete: [328; 328),
-                insert: "the_field",
+                insert: "another_field",
                 kind: Field,
-                detail: "u32",
+                detail: "i64",
             },
             CompletionItem {
                 label: "another_good_type",
@@ -365,14 +335,16 @@ mod tests {
                 insert: "another_good_type",
                 kind: Field,
                 detail: "u32",
+                score: TypeMatch,
             },
             CompletionItem {
-                label: "another_field",
+                label: "the_field",
                 source_range: [328; 328),
                 delete: [328; 328),
-                insert: "another_field",
+                insert: "the_field",
                 kind: Field,
-                detail: "i64",
+                detail: "u32",
+                score: TypeAndNameMatch,
             },
         ]
         "###
diff --git a/crates/ra_ide/src/completion/completion_item.rs b/crates/ra_ide/src/completion/completion_item.rs
index c9c3fdc0e..84d51bafe 100644
--- a/crates/ra_ide/src/completion/completion_item.rs
+++ b/crates/ra_ide/src/completion/completion_item.rs
@@ -53,6 +53,9 @@ pub struct CompletionItem {
     /// If completing a function call, ask the editor to show parameter popup
     /// after completion.
     trigger_call_info: bool,
+
+    /// Score is usefull to pre select or display in better order completion items
+    score: Option<CompletionScore>,
 }
 
 // We use custom debug for CompletionItem to make `insta`'s diffs more readable.
@@ -82,6 +85,9 @@ impl fmt::Debug for CompletionItem {
         if self.deprecated {
             s.field("deprecated", &true);
         }
+        if let Some(score) = &self.score {
+            s.field("score", score);
+        }
         if self.trigger_call_info {
             s.field("trigger_call_info", &true);
         }
@@ -149,6 +155,7 @@ impl CompletionItem {
             text_edit: None,
             deprecated: None,
             trigger_call_info: None,
+            score: None,
         }
     }
     /// What user sees in pop-up in the UI.
@@ -188,6 +195,10 @@ impl CompletionItem {
         self.deprecated
     }
 
+    pub fn score(&self) -> Option<CompletionScore> {
+        self.score.clone()
+    }
+
     pub fn trigger_call_info(&self) -> bool {
         self.trigger_call_info
     }
@@ -208,6 +219,7 @@ pub(crate) struct Builder {
     text_edit: Option<TextEdit>,
     deprecated: Option<bool>,
     trigger_call_info: Option<bool>,
+    score: Option<CompletionScore>,
 }
 
 impl Builder {
@@ -237,6 +249,7 @@ impl Builder {
             completion_kind: self.completion_kind,
             deprecated: self.deprecated.unwrap_or(false),
             trigger_call_info: self.trigger_call_info.unwrap_or(false),
+            score: self.score,
         }
     }
     pub(crate) fn lookup_by(mut self, lookup: impl Into<String>) -> Builder {
@@ -287,6 +300,10 @@ impl Builder {
         self.deprecated = Some(deprecated);
         self
     }
+    pub(crate) fn set_score(mut self, score: CompletionScore) -> Builder {
+        self.score = Some(score);
+        self
+    }
     pub(crate) fn trigger_call_info(mut self) -> Builder {
         self.trigger_call_info = Some(true);
         self
@@ -300,16 +317,22 @@ impl<'a> Into<CompletionItem> for Builder {
 }
 
 #[derive(Debug)]
-pub(crate) enum SortOption {
+pub(crate) enum ScoreOption {
     CallFn(CallInfo),
     RecordField(RecordField),
 }
 
+#[derive(Debug, Clone)]
+pub enum CompletionScore {
+    TypeMatch,
+    TypeAndNameMatch,
+}
+
 /// Represents an in-progress set of completions being built.
 #[derive(Debug, Default)]
 pub(crate) struct Completions {
     buf: Vec<CompletionItem>,
-    sort_option: Option<SortOption>,
+    score_option: Option<ScoreOption>,
 }
 
 impl Completions {
@@ -324,17 +347,17 @@ impl Completions {
         items.into_iter().for_each(|item| self.add(item.into()))
     }
 
-    pub(crate) fn with_sort_option(&mut self, sort_option: SortOption) {
-        self.sort_option = Some(sort_option);
+    pub(crate) fn with_score_option(&mut self, score_option: ScoreOption) {
+        self.score_option = Some(score_option);
     }
 
-    pub(crate) fn sort(&mut self, ctx: &CompletionContext) {
-        if self.sort_option.is_none() {
+    pub(crate) fn compute_score(&mut self, ctx: &CompletionContext) {
+        if self.score_option.is_none() {
             return;
         }
 
-        let (active_name, active_type) = match self.sort_option.as_ref().unwrap() {
-            SortOption::CallFn(call_info) => {
+        let (active_name, active_type) = match self.score_option.as_ref().unwrap() {
+            ScoreOption::CallFn(call_info) => {
                 if call_info.active_parameter_type().is_none()
                     || call_info.active_parameter_name().is_none()
                 {
@@ -345,7 +368,7 @@ impl Completions {
                     call_info.active_parameter_type().unwrap(),
                 )
             }
-            SortOption::RecordField(record_field) => {
+            ScoreOption::RecordField(record_field) => {
                 if let Some((struct_field, _)) = ctx.sema.resolve_record_field(record_field) {
                     (
                         struct_field.name(ctx.db).to_string(),
@@ -357,26 +380,19 @@ impl Completions {
             }
         };
 
-        self.buf.sort_by(|a, b| {
+        for completion_item in &mut self.buf {
             // For the same type
-            if let Some(a_parameter_type) = &a.detail {
+            if let Some(a_parameter_type) = &completion_item.detail {
                 if &active_type == a_parameter_type {
                     // If same type + same name then go top position
-                    if active_name != a.label {
-                        if let Some(b_parameter_type) = &b.detail {
-                            if &active_type == b_parameter_type {
-                                return Ordering::Equal;
-                            }
-                        }
+                    if active_name == completion_item.label {
+                        completion_item.score = Some(CompletionScore::TypeAndNameMatch);
+                    } else {
+                        completion_item.score = Some(CompletionScore::TypeMatch);
                     }
-                    Ordering::Less
-                } else {
-                    Ordering::Greater
                 }
-            } else {
-                Ordering::Greater
             }
-        });
+        }
     }
 }
 
diff --git a/crates/ra_ide/src/completion/presentation.rs b/crates/ra_ide/src/completion/presentation.rs
index 8be2d02d0..55f75b15a 100644
--- a/crates/ra_ide/src/completion/presentation.rs
+++ b/crates/ra_ide/src/completion/presentation.rs
@@ -367,7 +367,7 @@ mod tests {
         ra_fixture: &str,
         options: CompletionConfig,
     ) -> Vec<CompletionItem> {
-        do_completion_with_options(ra_fixture, CompletionKind::Reference, &options, true)
+        do_completion_with_options(ra_fixture, CompletionKind::Reference, &options)
     }
 
     #[test]
diff --git a/crates/ra_ide/src/completion/test_utils.rs b/crates/ra_ide/src/completion/test_utils.rs
index f54d15a90..eb90b5279 100644
--- a/crates/ra_ide/src/completion/test_utils.rs
+++ b/crates/ra_ide/src/completion/test_utils.rs
@@ -7,18 +7,13 @@ use crate::{
 };
 
 pub(crate) fn do_completion(code: &str, kind: CompletionKind) -> Vec<CompletionItem> {
-    do_completion_with_options(code, kind, &CompletionConfig::default(), true)
-}
-
-pub(crate) fn do_completion_without_sort(code: &str, kind: CompletionKind) -> Vec<CompletionItem> {
-    do_completion_with_options(code, kind, &CompletionConfig::default(), false)
+    do_completion_with_options(code, kind, &CompletionConfig::default())
 }
 
 pub(crate) fn do_completion_with_options(
     code: &str,
     kind: CompletionKind,
     options: &CompletionConfig,
-    sort_by_key: bool,
 ) -> Vec<CompletionItem> {
     let (analysis, position) = if code.contains("//-") {
         analysis_and_position(code)
@@ -29,8 +24,6 @@ pub(crate) fn do_completion_with_options(
     let completion_items: Vec<CompletionItem> = completions.into();
     let mut kind_completions: Vec<CompletionItem> =
         completion_items.into_iter().filter(|c| c.completion_kind == kind).collect();
-    if sort_by_key {
-        kind_completions.sort_by_key(|c| c.label().to_owned());
-    }
+    kind_completions.sort_by_key(|c| c.label().to_owned());
     kind_completions
 }
diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs
index 357c01cdf..ddaa30a16 100644
--- a/crates/ra_ide/src/lib.rs
+++ b/crates/ra_ide/src/lib.rs
@@ -67,7 +67,9 @@ use crate::display::ToNav;
 pub use crate::{
     assists::{Assist, AssistId},
     call_hierarchy::CallItem,
-    completion::{CompletionConfig, CompletionItem, CompletionItemKind, InsertTextFormat},
+    completion::{
+        CompletionConfig, CompletionItem, CompletionItemKind, CompletionScore, InsertTextFormat,
+    },
     diagnostics::Severity,
     display::{file_structure, FunctionSignature, NavigationTarget, StructureNode},
     expand_macro::ExpandedMacro,