From 26cd7451044e80706d30d730ff70cdd5a877b500 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 2 Sep 2024 11:49:11 +0200 Subject: [PATCH] Simplify CompletionRelevance --- .../src/completions/item_list/trait_impl.rs | 6 +- crates/ide-completion/src/item.rs | 79 +++++++++++-------- crates/ide-completion/src/render.rs | 73 +++++++---------- crates/ide-completion/src/render/function.rs | 17 ++-- 4 files changed, 85 insertions(+), 90 deletions(-) diff --git a/crates/ide-completion/src/completions/item_list/trait_impl.rs b/crates/ide-completion/src/completions/item_list/trait_impl.rs index e93bb8db716f..b5bc5533c79c 100644 --- a/crates/ide-completion/src/completions/item_list/trait_impl.rs +++ b/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/crates/ide-completion/src/item.rs b/crates/ide-completion/src/item.rs index a30a115da1f1..df30750b8fec 100644 --- a/crates/ide-completion/src/item.rs +++ b/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, - /// 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, + /// 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, - /// 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, } +#[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) {} @@ -243,14 +247,11 @@ impl 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; @@ -258,8 +259,17 @@ impl CompletionRelevance { if !is_private_editable { score += 1; } - // lower rank trait op methods - if !is_op_method { + + if let Some(trait_) = trait_ { + if trait_.notable_trait { + score += 1; + } + // lower rank trait op methods + if !trait_.is_op_method { + score += 10; + } + } else { + // lower rank trait op methods score += 10; } // lower rank for conflicting import names @@ -287,16 +297,6 @@ impl CompletionRelevance { 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 { @@ -701,8 +701,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/crates/ide-completion/src/render.rs b/crates/ide-completion/src/render.rs index ff5ec3a29f3e..2bec2eb87bc4 100644 --- a/crates/ide-completion/src/render.rs +++ b/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/crates/ide-completion/src/render/function.rs b/crates/ide-completion/src/render/function.rs index 74092b53f523..29820cb2036d 100644 --- a/crates/ide-completion/src/render/function.rs +++ b/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() });