From ca0633c8088628592359ab3aa9a336eb2137ff0c Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 6 Jan 2022 13:31:36 +0100 Subject: [PATCH 1/3] feat: Deprioritize ops methods in completion --- crates/hir/src/lib.rs | 6 +++ crates/hir_def/src/lang_item.rs | 4 +- crates/hir_expand/src/name.rs | 54 ++++++++++--------- crates/ide_completion/src/context.rs | 56 +++++++++++++++++++- crates/ide_completion/src/item.rs | 9 ++-- crates/ide_completion/src/render/function.rs | 7 +++ 6 files changed, 106 insertions(+), 30 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 { + 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/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..965f9b356a 100644 --- a/crates/ide_completion/src/context.rs +++ b/crates/ide_completion/src/context.rs @@ -3,13 +3,14 @@ use std::iter; use base_db::SourceDatabaseExt; -use hir::{Local, Name, ScopeDef, Semantics, SemanticsScope, Type, TypeInfo}; +use hir::{known, Local, Name, ScopeDef, Semantics, SemanticsScope, Type, TypeInfo}; use ide_db::{ active_parameter::ActiveParameter, base_db::{FilePosition, SourceDatabase}, helpers::FamousDefs, RootDatabase, }; +use rustc_hash::FxHashSet; use syntax::{ algo::find_node_at_offset, ast::{self, HasName, NameOrNameRef}, @@ -85,6 +86,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,7 +122,10 @@ pub(crate) struct CompletionContext<'a> { pub(super) lifetime_ctx: Option, pub(super) pattern_ctx: Option, pub(super) path_context: Option, + pub(super) locals: Vec<(Name, Local)>, + /// Operator traits defined in the project + pub(super) ops_traits: FxHashSet, no_completion_required: bool, } @@ -308,6 +313,11 @@ 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 { + self.ops_traits.contains(&trait_) + } + /// 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,17 @@ impl<'a> CompletionContext<'a> { locals.push((name, local)); } }); + let mut ops_traits = + FxHashSet::with_capacity_and_hasher(OP_TRAIT_LANG_NAMES.len(), Default::default()); + if let Some(krate) = krate { + let _p = profile::span("CompletionContext::new ops"); + for trait_ in + OP_TRAIT_LANG_NAMES.iter().filter_map(|item| hir::Trait::lang(db, krate, item)) + { + ops_traits.insert(trait_); + } + } + let mut ctx = CompletionContext { sema, scope, @@ -413,6 +434,7 @@ impl<'a> CompletionContext<'a> { locals, incomplete_let: false, no_completion_required: false, + ops_traits, }; ctx.expand_and_fill( original_file.syntax().clone(), @@ -889,6 +911,7 @@ fn pattern_context_for(pat: ast::Pat) -> PatternContext { }); PatternContext { refutability, is_param, has_type_ascription } } + fn find_node_with_range(syntax: &SyntaxNode, range: TextRange) -> Option { syntax.covering_element(range).ancestors().find_map(N::cast) } @@ -915,6 +938,37 @@ fn has_ref(token: &SyntaxToken) -> bool { token.kind() == T![&] } +const OP_TRAIT_LANG_NAMES: &[hir::Name] = &[ + known::add_assign, + known::add, + known::bitand_assign, + known::bitand, + known::bitor_assign, + known::bitor, + known::bitxor_assign, + known::bitxor, + known::deref_mut, + known::deref, + known::div_assign, + known::div, + known::fn_mut, + known::fn_once, + known::r#fn, + known::index_mut, + known::index, + known::mul_assign, + known::mul, + known::neg, + known::not, + known::rem_assign, + known::rem, + known::shl_assign, + known::shl, + known::shr_assign, + known::shr, + known::sub, + known::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..f2cf577285 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: /// /// ``` @@ -198,6 +200,9 @@ impl CompletionRelevance { if self.is_local { score += 1; } + if self.is_op_method { + score -= 1; + } if self.exact_postfix_snippet_match { score += 100; } @@ -588,10 +593,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/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() }); From 94b369faa3bec8bfcbb833bae34682fad6f89bc4 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 6 Jan 2022 13:36:43 +0100 Subject: [PATCH 2/3] Update tests --- crates/ide_completion/src/render.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index cd71ad1eea..648dce4306 100644 --- a/crates/ide_completion/src/render.rs +++ b/crates/ide_completion/src/render.rs @@ -580,6 +580,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 +601,7 @@ fn main() { let _: m::Spam = S$0 } Exact, ), is_local: false, + is_op_method: false, exact_postfix_snippet_match: false, }, }, @@ -685,6 +687,7 @@ fn foo() { A { the$0 } } CouldUnify, ), is_local: false, + is_op_method: false, exact_postfix_snippet_match: false, }, }, From 4901ea3eef1aa01a231f76767fa5ee19e7f76cd9 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 11 Jan 2022 10:07:16 +0100 Subject: [PATCH 3/3] Lookup lang attribute on assoc item trait directly --- crates/hir_def/src/attr.rs | 16 +++--- crates/ide_completion/src/context.rs | 81 ++++++++++++---------------- crates/ide_completion/src/item.rs | 6 ++- crates/ide_completion/src/render.rs | 18 +++++++ 4 files changed, 67 insertions(+), 54 deletions(-) diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index 30642f6cc6..c60ef87a93 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 { let docs = self.by_key("doc").attrs().flat_map(|attr| match attr.input.as_deref()? { AttrInput::Literal(s) => Some(s), @@ -754,20 +758,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 { +impl<'attr> AttrQuery<'attr> { + pub fn tt_values(self) -> impl Iterator { 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, @@ -778,7 +782,7 @@ impl<'a> AttrQuery<'a> { self.attrs().next().is_some() } - pub fn attrs(self) -> impl Iterator + Clone { + pub fn attrs(self) -> impl Iterator + Clone { let key = self.key; self.attrs .iter() diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs index 965f9b356a..0baca08ca9 100644 --- a/crates/ide_completion/src/context.rs +++ b/crates/ide_completion/src/context.rs @@ -3,14 +3,13 @@ use std::iter; use base_db::SourceDatabaseExt; -use hir::{known, 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}, helpers::FamousDefs, RootDatabase, }; -use rustc_hash::FxHashSet; use syntax::{ algo::find_node_at_offset, ast::{self, HasName, NameOrNameRef}, @@ -124,8 +123,6 @@ pub(crate) struct CompletionContext<'a> { pub(super) path_context: Option, pub(super) locals: Vec<(Name, Local)>, - /// Operator traits defined in the project - pub(super) ops_traits: FxHashSet, no_completion_required: bool, } @@ -315,7 +312,10 @@ impl<'a> CompletionContext<'a> { /// Whether the given trait is an operator trait or not. pub(crate) fn is_ops_trait(&self, trait_: hir::Trait) -> bool { - self.ops_traits.contains(&trait_) + 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. @@ -398,16 +398,6 @@ impl<'a> CompletionContext<'a> { locals.push((name, local)); } }); - let mut ops_traits = - FxHashSet::with_capacity_and_hasher(OP_TRAIT_LANG_NAMES.len(), Default::default()); - if let Some(krate) = krate { - let _p = profile::span("CompletionContext::new ops"); - for trait_ in - OP_TRAIT_LANG_NAMES.iter().filter_map(|item| hir::Trait::lang(db, krate, item)) - { - ops_traits.insert(trait_); - } - } let mut ctx = CompletionContext { sema, @@ -434,7 +424,6 @@ impl<'a> CompletionContext<'a> { locals, incomplete_let: false, no_completion_required: false, - ops_traits, }; ctx.expand_and_fill( original_file.syntax().clone(), @@ -938,36 +927,36 @@ fn has_ref(token: &SyntaxToken) -> bool { token.kind() == T![&] } -const OP_TRAIT_LANG_NAMES: &[hir::Name] = &[ - known::add_assign, - known::add, - known::bitand_assign, - known::bitand, - known::bitor_assign, - known::bitor, - known::bitxor_assign, - known::bitxor, - known::deref_mut, - known::deref, - known::div_assign, - known::div, - known::fn_mut, - known::fn_once, - known::r#fn, - known::index_mut, - known::index, - known::mul_assign, - known::mul, - known::neg, - known::not, - known::rem_assign, - known::rem, - known::shl_assign, - known::shl, - known::shr_assign, - known::shr, - known::sub, - known::sub, +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 { diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index f2cf577285..8ac4291078 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs @@ -177,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 @@ -187,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; @@ -213,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) } } @@ -563,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() }, diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index 648dce4306..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 }) @@ -1352,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(