Auto merge of #18026 - Veykril:completions, r=Veykril

internal:  Adjust completions scoring

4 files changed, 125 insertions, 134 deletions

diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/completions/item_list/trait_impl.rs b/src/tools/rust-analyzer/crates/ide-completion/src/completions/item_list/trait_impl.rs
index e93bb8db716..b5bc5533c79 100644
--- a/src/tools/rust-analyzer/crates/ide-completion/src/completions/item_list/trait_impl.rs
+++ b/src/tools/rust-analyzer/crates/ide-completion/src/completions/item_list/trait_impl.rs
@@ -221,7 +221,7 @@ fn add_function_impl_(
     let mut item = CompletionItem::new(completion_kind, replacement_range, label, ctx.edition);
     item.lookup_by(format!("{}fn {}", async_, fn_name.display(ctx.db, ctx.edition)))
         .set_documentation(func.docs(ctx.db))
-        .set_relevance(CompletionRelevance { is_item_from_trait: true, ..Default::default() });
+        .set_relevance(CompletionRelevance { exact_name_match: true, ..Default::default() });
 
     if let Some(source) = ctx.sema.source(func) {
         if let Some(transformed_fn) =
@@ -366,7 +366,7 @@ fn add_type_alias_impl(
         CompletionItem::new(SymbolKind::TypeAlias, replacement_range, label, ctx.edition);
     item.lookup_by(format!("type {alias_name}"))
         .set_documentation(type_alias.docs(ctx.db))
-        .set_relevance(CompletionRelevance { is_item_from_trait: true, ..Default::default() });
+        .set_relevance(CompletionRelevance { exact_name_match: true, ..Default::default() });
 
     if let Some(source) = ctx.sema.source(type_alias) {
         let assoc_item = ast::AssocItem::TypeAlias(source.value);
@@ -440,7 +440,7 @@ fn add_const_impl(
                 item.lookup_by(format_smolstr!("const {const_name}"))
                     .set_documentation(const_.docs(ctx.db))
                     .set_relevance(CompletionRelevance {
-                        is_item_from_trait: true,
+                        exact_name_match: true,
                         ..Default::default()
                     });
                 match ctx.config.snippet_cap {
diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/item.rs b/src/tools/rust-analyzer/crates/ide-completion/src/item.rs
index a30a115da1f..f8c8b12bd25 100644
--- a/src/tools/rust-analyzer/crates/ide-completion/src/item.rs
+++ b/src/tools/rust-analyzer/crates/ide-completion/src/item.rs
@@ -19,8 +19,10 @@ use crate::{
 };
 
 /// `CompletionItem` describes a single completion entity which expands to 1 or more entries in the
-/// editor pop-up. It is basically a POD with various properties. To construct a
-/// [`CompletionItem`], use [`Builder::new`] method and the [`Builder`] struct.
+/// editor pop-up.
+///
+/// It is basically a POD with various properties. To construct a [`CompletionItem`],
+/// use [`Builder::new`] method and the [`Builder`] struct.
 #[derive(Clone)]
 #[non_exhaustive]
 pub struct CompletionItem {
@@ -129,7 +131,8 @@ impl fmt::Debug for CompletionItem {
 
 #[derive(Debug, Clone, Copy, Eq, PartialEq, Default)]
 pub struct CompletionRelevance {
-    /// This is set in cases like these:
+    /// This is set when the identifier being completed matches up with the name that is expected,
+    /// like in a function argument.
     ///
     /// ```
     /// fn f(spam: String) {}
@@ -139,9 +142,9 @@ pub struct CompletionRelevance {
     /// }
     /// ```
     pub exact_name_match: bool,
-    /// See CompletionRelevanceTypeMatch doc comments for cases where this is set.
+    /// See [`CompletionRelevanceTypeMatch`].
     pub type_match: Option<CompletionRelevanceTypeMatch>,
-    /// This is set in cases like these:
+    /// Set for local variables.
     ///
     /// ```
     /// fn foo(a: u32) {
@@ -150,25 +153,26 @@ pub struct CompletionRelevance {
     /// }
     /// ```
     pub is_local: bool,
-    /// This is set when trait items are completed in an impl of that trait.
-    pub is_item_from_trait: bool,
-    /// This is set for when trait items are from traits with `#[doc(notable_trait)]`
-    pub is_item_from_notable_trait: bool,
-    /// This is set when an import is suggested whose name is already imported.
+    /// Populated when the completion item comes from a trait (impl).
+    pub trait_: Option<CompletionRelevanceTraitInfo>,
+    /// This is set when an import is suggested in a use item whose name is already imported.
     pub is_name_already_imported: bool,
     /// This is set for completions that will insert a `use` item.
     pub requires_import: bool,
-    /// Set for method completions of the `core::ops` and `core::cmp` family.
-    pub is_op_method: bool,
     /// Set for item completions that are private but in the workspace.
     pub is_private_editable: bool,
     /// Set for postfix snippet item completions
     pub postfix_match: Option<CompletionRelevancePostfixMatch>,
-    /// This is set for type inference results
-    pub is_definite: bool,
     /// This is set for items that are function (associated or method)
     pub function: Option<CompletionRelevanceFn>,
 }
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+pub struct CompletionRelevanceTraitInfo {
+    /// The trait this item is from is a `#[doc(notable_trait)]`
+    pub notable_trait: bool,
+    /// Set for method completions of the `core::ops` and `core::cmp` family.
+    pub is_op_method: bool,
+}
 
 #[derive(Debug, Clone, Copy, Eq, PartialEq)]
 pub enum CompletionRelevanceTypeMatch {
@@ -182,7 +186,7 @@ pub enum CompletionRelevanceTypeMatch {
     /// }
     /// ```
     CouldUnify,
-    /// This is set in cases like these:
+    /// This is set in cases where the type matches the expected type, like:
     ///
     /// ```
     /// fn f(spam: String) {}
@@ -238,90 +242,82 @@ impl CompletionRelevance {
     /// 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;
+        let mut score = !0 / 2;
         let CompletionRelevance {
             exact_name_match,
             type_match,
             is_local,
-            is_item_from_trait,
             is_name_already_imported,
             requires_import,
-            is_op_method,
             is_private_editable,
             postfix_match,
-            is_definite,
-            is_item_from_notable_trait,
+            trait_,
             function,
         } = self;
 
+        // only applicable for completions within use items
+        // lower rank for conflicting import names
+        if is_name_already_imported {
+            score -= 1;
+        }
+        // slightly prefer locals
+        if is_local {
+            score += 1;
+        }
+
         // lower rank private things
         if !is_private_editable {
             score += 1;
         }
-        // lower rank trait op methods
-        if !is_op_method {
-            score += 10;
+
+        if let Some(trait_) = trait_ {
+            // lower rank trait methods unless its notable
+            if !trait_.notable_trait {
+                score -= 5;
+            }
+            // lower rank trait op methods
+            if trait_.is_op_method {
+                score -= 5;
+            }
         }
-        // lower rank for conflicting import names
-        if !is_name_already_imported {
-            score += 1;
-        }
-        // lower rank for items that don't need an import
-        if !requires_import {
-            score += 1;
+        // lower rank for items that need an import
+        if requires_import {
+            score -= 1;
         }
         if exact_name_match {
-            score += 10;
+            score += 20;
         }
-        score += match postfix_match {
-            Some(CompletionRelevancePostfixMatch::Exact) => 100,
-            Some(CompletionRelevancePostfixMatch::NonExact) => 0,
-            None => 3,
+        match postfix_match {
+            Some(CompletionRelevancePostfixMatch::Exact) => score += 100,
+            Some(CompletionRelevancePostfixMatch::NonExact) => score -= 5,
+            None => (),
         };
         score += match type_match {
-            Some(CompletionRelevanceTypeMatch::Exact) => 8,
-            Some(CompletionRelevanceTypeMatch::CouldUnify) => 3,
+            Some(CompletionRelevanceTypeMatch::Exact) => 18,
+            Some(CompletionRelevanceTypeMatch::CouldUnify) => 5,
             None => 0,
         };
-        // slightly prefer locals
-        if is_local {
-            score += 1;
-        }
-        if is_item_from_trait {
-            score += 1;
-        }
-        if is_item_from_notable_trait {
-            score += 1;
-        }
-        if is_definite {
-            score += 10;
-        }
-
-        score += function
-            .map(|asf| {
-                let mut fn_score = match asf.return_type {
-                    CompletionRelevanceReturnType::DirectConstructor => 15,
-                    CompletionRelevanceReturnType::Builder => 10,
-                    CompletionRelevanceReturnType::Constructor => 5,
-                    CompletionRelevanceReturnType::Other => 0,
-                };
-
-                // When a fn is bumped due to return type:
-                // Bump Constructor or Builder methods with no arguments,
-                // over them than with self arguments
-                if fn_score > 0 {
-                    if !asf.has_params {
-                        // bump associated functions
-                        fn_score += 1;
-                    } else if asf.has_self_param {
-                        // downgrade methods (below Constructor)
-                        fn_score = 1;
-                    }
-                }
+        if let Some(function) = function {
+            let mut fn_score = match function.return_type {
+                CompletionRelevanceReturnType::DirectConstructor => 15,
+                CompletionRelevanceReturnType::Builder => 10,
+                CompletionRelevanceReturnType::Constructor => 5,
+                CompletionRelevanceReturnType::Other => 0u32,
+            };
+
+            // When a fn is bumped due to return type:
+            // Bump Constructor or Builder methods with no arguments,
+            // over them than with self arguments
+            if function.has_params {
+                // bump associated functions
+                fn_score = fn_score.saturating_sub(1);
+            } else if function.has_self_param {
+                // downgrade methods (below Constructor)
+                fn_score = fn_score.min(1);
+            }
 
-                fn_score
-            })
-            .unwrap_or_default();
+            score += fn_score;
+        };
 
         score
     }
@@ -701,8 +697,21 @@ mod tests {
         // that any items in the same vec have the same score.
         let expected_relevance_order = vec![
             vec![],
-            vec![Cr { is_op_method: true, is_private_editable: true, ..default }],
-            vec![Cr { is_op_method: true, ..default }],
+            vec![Cr {
+                trait_: Some(crate::item::CompletionRelevanceTraitInfo {
+                    notable_trait: false,
+                    is_op_method: true,
+                }),
+                is_private_editable: true,
+                ..default
+            }],
+            vec![Cr {
+                trait_: Some(crate::item::CompletionRelevanceTraitInfo {
+                    notable_trait: false,
+                    is_op_method: true,
+                }),
+                ..default
+            }],
             vec![Cr { postfix_match: Some(CompletionRelevancePostfixMatch::NonExact), ..default }],
             vec![Cr { is_private_editable: true, ..default }],
             vec![default],
diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/render.rs b/src/tools/rust-analyzer/crates/ide-completion/src/render.rs
index ff5ec3a29f3..2bec2eb87bc 100644
--- a/src/tools/rust-analyzer/crates/ide-completion/src/render.rs
+++ b/src/tools/rust-analyzer/crates/ide-completion/src/render.rs
@@ -249,7 +249,11 @@ pub(crate) fn render_type_inference(
         ty_string,
         ctx.edition,
     );
-    builder.set_relevance(CompletionRelevance { is_definite: true, ..Default::default() });
+    builder.set_relevance(CompletionRelevance {
+        type_match: Some(CompletionRelevanceTypeMatch::Exact),
+        exact_name_match: true,
+        ..Default::default()
+    });
     builder.build(ctx.db)
 }
 
@@ -756,7 +760,7 @@ mod tests {
                     relevance.postfix_match == Some(CompletionRelevancePostfixMatch::Exact),
                     "snippet",
                 ),
-                (relevance.is_op_method, "op_method"),
+                (relevance.trait_.map_or(false, |it| it.is_op_method), "op_method"),
                 (relevance.requires_import, "requires_import"),
             ]
             .into_iter()
@@ -1272,14 +1276,11 @@ fn main() { let _: m::Spam = S$0 }
                                 Exact,
                             ),
                             is_local: false,
-                            is_item_from_trait: false,
-                            is_item_from_notable_trait: false,
+                            trait_: None,
                             is_name_already_imported: false,
                             requires_import: false,
-                            is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
-                            is_definite: false,
                             function: None,
                         },
                         trigger_call_info: true,
@@ -1300,14 +1301,11 @@ fn main() { let _: m::Spam = S$0 }
                                 Exact,
                             ),
                             is_local: false,
-                            is_item_from_trait: false,
-                            is_item_from_notable_trait: false,
+                            trait_: None,
                             is_name_already_imported: false,
                             requires_import: false,
-                            is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
-                            is_definite: false,
                             function: None,
                         },
                         trigger_call_info: true,
@@ -1380,14 +1378,11 @@ fn foo() { A { the$0 } }
                                 CouldUnify,
                             ),
                             is_local: false,
-                            is_item_from_trait: false,
-                            is_item_from_notable_trait: false,
+                            trait_: None,
                             is_name_already_imported: false,
                             requires_import: false,
-                            is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
-                            is_definite: false,
                             function: None,
                         },
                     },
@@ -1431,14 +1426,11 @@ impl S {
                             exact_name_match: false,
                             type_match: None,
                             is_local: false,
-                            is_item_from_trait: false,
-                            is_item_from_notable_trait: false,
+                            trait_: None,
                             is_name_already_imported: false,
                             requires_import: false,
-                            is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
-                            is_definite: false,
                             function: Some(
                                 CompletionRelevanceFn {
                                     has_params: true,
@@ -1558,14 +1550,11 @@ fn foo(s: S) { s.$0 }
                             exact_name_match: false,
                             type_match: None,
                             is_local: false,
-                            is_item_from_trait: false,
-                            is_item_from_notable_trait: false,
+                            trait_: None,
                             is_name_already_imported: false,
                             requires_import: false,
-                            is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
-                            is_definite: false,
                             function: Some(
                                 CompletionRelevanceFn {
                                     has_params: true,
@@ -1774,14 +1763,11 @@ fn f() -> i32 {
                                 Exact,
                             ),
                             is_local: false,
-                            is_item_from_trait: false,
-                            is_item_from_notable_trait: false,
+                            trait_: None,
                             is_name_already_imported: false,
                             requires_import: false,
-                            is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
-                            is_definite: false,
                             function: None,
                         },
                     },
@@ -2492,14 +2478,11 @@ fn foo(f: Foo) { let _: &u32 = f.b$0 }
                             exact_name_match: false,
                             type_match: None,
                             is_local: false,
-                            is_item_from_trait: false,
-                            is_item_from_notable_trait: false,
+                            trait_: None,
                             is_name_already_imported: false,
                             requires_import: false,
-                            is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
-                            is_definite: false,
                             function: Some(
                                 CompletionRelevanceFn {
                                     has_params: true,
@@ -2574,14 +2557,11 @@ fn foo() {
                                 Exact,
                             ),
                             is_local: false,
-                            is_item_from_trait: false,
-                            is_item_from_notable_trait: false,
+                            trait_: None,
                             is_name_already_imported: false,
                             requires_import: false,
-                            is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
-                            is_definite: false,
                             function: None,
                         },
                     },
@@ -2624,14 +2604,11 @@ fn main() {
                             exact_name_match: false,
                             type_match: None,
                             is_local: false,
-                            is_item_from_trait: false,
-                            is_item_from_notable_trait: false,
+                            trait_: None,
                             is_name_already_imported: false,
                             requires_import: false,
-                            is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
-                            is_definite: false,
                             function: Some(
                                 CompletionRelevanceFn {
                                     has_params: false,
@@ -2996,14 +2973,16 @@ fn main() {
                             exact_name_match: false,
                             type_match: None,
                             is_local: false,
-                            is_item_from_trait: false,
-                            is_item_from_notable_trait: true,
+                            trait_: Some(
+                                CompletionRelevanceTraitInfo {
+                                    notable_trait: true,
+                                    is_op_method: false,
+                                },
+                            ),
                             is_name_already_imported: false,
                             requires_import: false,
-                            is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
-                            is_definite: false,
                             function: None,
                         },
                     },
@@ -3021,14 +3000,16 @@ fn main() {
                             exact_name_match: false,
                             type_match: None,
                             is_local: false,
-                            is_item_from_trait: false,
-                            is_item_from_notable_trait: true,
+                            trait_: Some(
+                                CompletionRelevanceTraitInfo {
+                                    notable_trait: true,
+                                    is_op_method: false,
+                                },
+                            ),
                             is_name_already_imported: false,
                             requires_import: false,
-                            is_op_method: false,
                             is_private_editable: false,
                             postfix_match: None,
-                            is_definite: false,
                             function: None,
                         },
                     },
diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/render/function.rs b/src/tools/rust-analyzer/crates/ide-completion/src/render/function.rs
index 74092b53f52..29820cb2036 100644
--- a/src/tools/rust-analyzer/crates/ide-completion/src/render/function.rs
+++ b/src/tools/rust-analyzer/crates/ide-completion/src/render/function.rs
@@ -10,7 +10,7 @@ use crate::{
     context::{CompletionContext, DotAccess, DotAccessKind, PathCompletionCtx, PathKind},
     item::{
         Builder, CompletionItem, CompletionItemKind, CompletionRelevance, CompletionRelevanceFn,
-        CompletionRelevanceReturnType,
+        CompletionRelevanceReturnType, CompletionRelevanceTraitInfo,
     },
     render::{
         compute_exact_name_match, compute_ref_match, compute_type_match, match_types, RenderContext,
@@ -88,11 +88,13 @@ fn render(
     let ret_type = func.ret_type(db);
     let assoc_item = func.as_assoc_item(db);
 
-    let trait_ = assoc_item.and_then(|trait_| trait_.container_or_implemented_trait(db));
-    let is_op_method = trait_.map_or(false, |trait_| completion.is_ops_trait(trait_));
-
-    let is_item_from_notable_trait =
-        trait_.map_or(false, |trait_| completion.is_doc_notable_trait(trait_));
+    let trait_info =
+        assoc_item.and_then(|trait_| trait_.container_or_implemented_trait(db)).map(|trait_| {
+            CompletionRelevanceTraitInfo {
+                notable_trait: completion.is_doc_notable_trait(trait_),
+                is_op_method: completion.is_ops_trait(trait_),
+            }
+        });
 
     let (has_dot_receiver, has_call_parens, cap) = match func_kind {
         FuncKind::Function(&PathCompletionCtx {
@@ -129,8 +131,7 @@ fn render(
         },
         exact_name_match: compute_exact_name_match(completion, &call),
         function,
-        is_op_method,
-        is_item_from_notable_trait,
+        trait_: trait_info,
         ..ctx.completion_relevance()
     });