Merge #11210

11210: feat: Deprioritize ops methods in completion r=Veykril a=Veykril

Fixes https://github.com/rust-analyzer/rust-analyzer/issues/10593



Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

crates/hir/src/lib.rs

@@ -1610,6 +1610,12 @@ pub struct Trait {
 }
 
 impl Trait {
+    pub fn lang(db: &dyn HirDatabase, krate: Crate, name: &Name) -> Option<Trait> {
+        db.lang_item(krate.into(), name.to_smol_str())
+            .and_then(LangItemTarget::as_trait)
+            .map(Into::into)
+    }
+
     pub fn module(self, db: &dyn HirDatabase) -> Module {
         Module { id: self.id.lookup(db.upcast()).container }
     }

crates/hir_def/src/attr.rs

@@ -255,6 +255,10 @@ impl Attrs {
         }
     }
 
+    pub fn lang(&self) -> Option<&SmolStr> {
+        self.by_key("lang").string_value()
+    }
+
     pub fn docs(&self) -> Option<Documentation> {
         let docs = self.by_key("doc").attrs().flat_map(|attr| match attr.input.as_deref()? {
             AttrInput::Literal(s) => Some(s),
@@ -775,20 +779,20 @@ impl Attr {
 }
 
 #[derive(Debug, Clone, Copy)]
-pub struct AttrQuery<'a> {
+pub struct AttrQuery<'attr> {
-    attrs: &'a Attrs,
+    attrs: &'attr Attrs,
     key: &'static str,
 }
 
-impl<'a> AttrQuery<'a> {
+impl<'attr> AttrQuery<'attr> {
-    pub fn tt_values(self) -> impl Iterator<Item = &'a Subtree> {
+    pub fn tt_values(self) -> impl Iterator<Item = &'attr Subtree> {
         self.attrs().filter_map(|attr| match attr.input.as_deref()? {
             AttrInput::TokenTree(it, _) => Some(it),
             _ => None,
         })
     }
 
-    pub fn string_value(self) -> Option<&'a SmolStr> {
+    pub fn string_value(self) -> Option<&'attr SmolStr> {
         self.attrs().find_map(|attr| match attr.input.as_deref()? {
             AttrInput::Literal(it) => Some(it),
             _ => None,
@@ -799,7 +803,7 @@ impl<'a> AttrQuery<'a> {
         self.attrs().next().is_some()
     }
 
-    pub fn attrs(self) -> impl Iterator<Item = &'a Attr> + Clone {
+    pub fn attrs(self) -> impl Iterator<Item = &'attr Attr> + Clone {
         let key = self.key;
         self.attrs
             .iter()

crates/hir_def/src/lang_item.rs

@@ -144,8 +144,8 @@ impl LangItems {
         let _p = profile::span("lang_item_query");
         let lang_items = db.crate_lang_items(start_crate);
         let start_crate_target = lang_items.items.get(&item);
-        if let Some(target) = start_crate_target {
+        if let Some(&target) = start_crate_target {
-            return Some(*target);
+            return Some(target);
         }
         db.crate_graph()[start_crate]
             .dependencies

crates/hir_expand/src/name.rs

@@ -309,26 +309,6 @@ pub mod known {
         wrapping_mul,
         wrapping_sub,
         // known methods of lang items
-        add,
-        mul,
-        sub,
-        div,
-        rem,
-        shl,
-        shr,
-        bitxor,
-        bitor,
-        bitand,
-        add_assign,
-        mul_assign,
-        sub_assign,
-        div_assign,
-        rem_assign,
-        shl_assign,
-        shr_assign,
-        bitxor_assign,
-        bitor_assign,
-        bitand_assign,
         eq,
         ne,
         ge,
@@ -336,12 +316,38 @@ pub mod known {
         le,
         lt,
         // lang items
-        not,
+        add_assign,
-        neg,
+        add,
+        bitand_assign,
+        bitand,
+        bitor_assign,
+        bitor,
+        bitxor_assign,
+        bitxor,
+        deref_mut,
+        deref,
+        div_assign,
+        div,
+        fn_mut,
+        fn_once,
         future_trait,
-        owned_box,
         index,
-        partial_ord
+        index_mut,
+        mul_assign,
+        mul,
+        neg,
+        not,
+        owned_box,
+        partial_ord,
+        r#fn,
+        rem_assign,
+        rem,
+        shl_assign,
+        shl,
+        shr_assign,
+        shr,
+        sub_assign,
+        sub,
     );
 
     // self/Self cannot be used as an identifier

crates/ide_completion/src/context.rs

@@ -3,7 +3,7 @@
 use std::iter;
 
 use base_db::SourceDatabaseExt;
-use hir::{Local, Name, ScopeDef, Semantics, SemanticsScope, Type, TypeInfo};
+use hir::{HasAttrs, Local, Name, ScopeDef, Semantics, SemanticsScope, Type, TypeInfo};
 use ide_db::{
     active_parameter::ActiveParameter,
     base_db::{FilePosition, SourceDatabase},
@@ -85,6 +85,7 @@ pub(crate) enum ParamKind {
     Function,
     Closure,
 }
+
 /// `CompletionContext` is created early during completion to figure out, where
 /// exactly is the cursor, syntax-wise.
 #[derive(Debug)]
@@ -120,6 +121,7 @@ pub(crate) struct CompletionContext<'a> {
     pub(super) lifetime_ctx: Option<LifetimeContext>,
     pub(super) pattern_ctx: Option<PatternContext>,
     pub(super) path_context: Option<PathCompletionContext>,
+
     pub(super) locals: Vec<(Name, Local)>,
 
     no_completion_required: bool,
@@ -308,6 +310,14 @@ impl<'a> CompletionContext<'a> {
         self.token.kind() == BANG && self.token.parent().map_or(false, |it| it.kind() == MACRO_CALL)
     }
 
+    /// Whether the given trait is an operator trait or not.
+    pub(crate) fn is_ops_trait(&self, trait_: hir::Trait) -> bool {
+        match trait_.attrs(self.db).lang() {
+            Some(lang) => OP_TRAIT_LANG_NAMES.contains(&lang.as_str()),
+            None => false,
+        }
+    }
+
     /// A version of [`SemanticsScope::process_all_names`] that filters out `#[doc(hidden)]` items.
     pub(crate) fn process_all_names(&self, f: &mut dyn FnMut(Name, ScopeDef)) {
         let _p = profile::span("CompletionContext::process_all_names");
@@ -388,6 +398,7 @@ impl<'a> CompletionContext<'a> {
                 locals.push((name, local));
             }
         });
+
         let mut ctx = CompletionContext {
             sema,
             scope,
@@ -889,6 +900,7 @@ fn pattern_context_for(pat: ast::Pat) -> PatternContext {
         });
     PatternContext { refutability, is_param, has_type_ascription }
 }
+
 fn find_node_with_range<N: AstNode>(syntax: &SyntaxNode, range: TextRange) -> Option<N> {
     syntax.covering_element(range).ancestors().find_map(N::cast)
 }
@@ -915,6 +927,37 @@ fn has_ref(token: &SyntaxToken) -> bool {
     token.kind() == T![&]
 }
 
+const OP_TRAIT_LANG_NAMES: &[&str] = &[
+    "add_assign",
+    "add",
+    "bitand_assign",
+    "bitand",
+    "bitor_assign",
+    "bitor",
+    "bitxor_assign",
+    "bitxor",
+    "deref_mut",
+    "deref",
+    "div_assign",
+    "div",
+    "fn_mut",
+    "fn_once",
+    "fn",
+    "index_mut",
+    "index",
+    "mul_assign",
+    "mul",
+    "neg",
+    "not",
+    "rem_assign",
+    "rem",
+    "shl_assign",
+    "shl",
+    "shr_assign",
+    "shr",
+    "sub",
+    "sub",
+];
 #[cfg(test)]
 mod tests {
     use expect_test::{expect, Expect};

crates/ide_completion/src/item.rs

@@ -139,6 +139,8 @@ pub struct CompletionRelevance {
     /// }
     /// ```
     pub is_local: bool,
+    /// Set for method completions of the `core::ops` family.
+    pub is_op_method: bool,
     /// This is set in cases like these:
     ///
     /// ```
@@ -175,6 +177,7 @@ pub enum CompletionRelevanceTypeMatch {
 }
 
 impl CompletionRelevance {
+    const BASE_LINE: u32 = 1;
     /// Provides a relevance score. Higher values are more relevant.
     ///
     /// The absolute value of the relevance score is not meaningful, for
@@ -185,7 +188,7 @@ 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 = Self::BASE_LINE;
 
         if self.exact_name_match {
             score += 1;
@@ -198,6 +201,9 @@ impl CompletionRelevance {
         if self.is_local {
             score += 1;
         }
+        if self.is_op_method {
+            score -= 1;
+        }
         if self.exact_postfix_snippet_match {
             score += 100;
         }
@@ -208,7 +214,7 @@ impl CompletionRelevance {
     /// some threshold such that we think it is especially likely
     /// to be relevant.
     pub fn is_relevant(&self) -> bool {
-        self.score() > 0
+        self.score() > (Self::BASE_LINE + 1)
     }
 }
 
@@ -558,6 +564,7 @@ mod tests {
         // 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 { is_op_method: true, ..CompletionRelevance::default() }],
             vec![CompletionRelevance::default()],
             vec![
                 CompletionRelevance { exact_name_match: true, ..CompletionRelevance::default() },
@@ -588,10 +595,8 @@ mod tests {
                 ..CompletionRelevance::default()
             }],
             vec![CompletionRelevance {
-                exact_name_match: false,
-                type_match: None,
-                is_local: false,
                 exact_postfix_snippet_match: true,
+                ..CompletionRelevance::default()
             }],
         ];
 

crates/ide_completion/src/render.rs

@@ -400,6 +400,7 @@ mod tests {
                 (relevance.exact_name_match, "name"),
                 (relevance.is_local, "local"),
                 (relevance.exact_postfix_snippet_match, "snippet"),
+                (relevance.is_op_method, "op_method"),
             ]
             .into_iter()
             .filter_map(|(cond, desc)| if cond { Some(desc) } else { None })
@@ -580,6 +581,7 @@ fn main() { let _: m::Spam = S$0 }
                                 Exact,
                             ),
                             is_local: false,
+                            is_op_method: false,
                             exact_postfix_snippet_match: false,
                         },
                         trigger_call_info: true,
@@ -600,6 +602,7 @@ fn main() { let _: m::Spam = S$0 }
                                 Exact,
                             ),
                             is_local: false,
+                            is_op_method: false,
                             exact_postfix_snippet_match: false,
                         },
                     },
@@ -685,6 +688,7 @@ fn foo() { A { the$0 } }
                                 CouldUnify,
                             ),
                             is_local: false,
+                            is_op_method: false,
                             exact_postfix_snippet_match: false,
                         },
                     },
@@ -1349,6 +1353,23 @@ fn main() {
         )
     }
 
+    #[test]
+    fn op_method_relevances() {
+        check_relevance(
+            r#"
+#[lang = "sub"]
+trait Sub {
+    fn sub(self, other: Self) -> Self { self }
+}
+impl Sub for u32 {}
+fn foo(a: u32) { a.$0 }
+"#,
+            expect![[r#"
+                me sub(…) (as Sub) [op_method]
+            "#]],
+        )
+    }
+
     #[test]
     fn struct_field_method_ref() {
         check_kinds(

crates/ide_completion/src/render/function.rs

@@ -70,6 +70,13 @@ fn render(
     item.set_relevance(CompletionRelevance {
         type_match: compute_type_match(completion, &ret_type),
         exact_name_match: compute_exact_name_match(completion, &call),
+        is_op_method: match func_type {
+            FuncType::Method(_) => func
+                .as_assoc_item(ctx.db())
+                .and_then(|trait_| trait_.containing_trait_or_trait_impl(ctx.db()))
+                .map_or(false, |trait_| completion.is_ops_trait(trait_)),
+            _ => false,
+        },
         ..CompletionRelevance::default()
     });