Unnamed repository; edit this file 'description' to name the repository.
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 <[email protected]>
| -rw-r--r-- | crates/hir/src/lib.rs | 6 | ||||
| -rw-r--r-- | crates/hir_def/src/attr.rs | 16 | ||||
| -rw-r--r-- | crates/hir_def/src/lang_item.rs | 4 | ||||
| -rw-r--r-- | crates/hir_expand/src/name.rs | 54 | ||||
| -rw-r--r-- | crates/ide_completion/src/context.rs | 45 | ||||
| -rw-r--r-- | crates/ide_completion/src/item.rs | 15 | ||||
| -rw-r--r-- | crates/ide_completion/src/render.rs | 21 | ||||
| -rw-r--r-- | crates/ide_completion/src/render/function.rs | 7 |
8 files changed, 130 insertions, 38 deletions
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index b0ff797275..2c91a11e15 100644 --- a/crates/hir/src/lib.rs +++ b/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 } } diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index a244d32149..b4ddfba0d0 100644 --- a/crates/hir_def/src/attr.rs +++ b/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> { - attrs: &'a Attrs, +pub struct AttrQuery<'attr> { + attrs: &'attr Attrs, key: &'static str, } -impl<'a> AttrQuery<'a> { - pub fn tt_values(self) -> impl Iterator<Item = &'a Subtree> { +impl<'attr> AttrQuery<'attr> { + 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() diff --git a/crates/hir_def/src/lang_item.rs b/crates/hir_def/src/lang_item.rs index 6797a77de5..8778501845 100644 --- a/crates/hir_def/src/lang_item.rs +++ b/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 { - return Some(*target); + if let Some(&target) = start_crate_target { + return Some(target); } db.crate_graph()[start_crate] .dependencies diff --git a/crates/hir_expand/src/name.rs b/crates/hir_expand/src/name.rs index 3636ab6213..4dcda0fcdc 100644 --- a/crates/hir_expand/src/name.rs +++ b/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, - neg, + add_assign, + 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 diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs index 26f6c17805..0baca08ca9 100644 --- a/crates/ide_completion/src/context.rs +++ b/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}; diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 4a6e034dc9..8ac4291078 100644 --- a/crates/ide_completion/src/item.rs +++ b/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() }], ]; diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index cd71ad1eea..15dacc8e46 100644 --- a/crates/ide_completion/src/render.rs +++ b/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, }, }, @@ -1350,6 +1354,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( r#" diff --git a/crates/ide_completion/src/render/function.rs b/crates/ide_completion/src/render/function.rs index bd46e1fefb..fc2cb933db 100644 --- a/crates/ide_completion/src/render/function.rs +++ b/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() }); |