From 63f87ff04724f2c8f1770daab2e6cea024ccb923 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 11 Apr 2022 18:48:27 +0200 Subject: [PATCH] Deprioritize already-imported names --- crates/ide_completion/src/completions.rs | 64 ++++++++++++------- .../src/completions/flyimport.rs | 3 +- crates/ide_completion/src/completions/use_.rs | 30 ++++++++- crates/ide_completion/src/item.rs | 20 ++++++ crates/ide_completion/src/render.rs | 17 +++-- crates/ide_completion/src/render/function.rs | 8 +-- crates/ide_completion/src/render/literal.rs | 10 +-- crates/ide_completion/src/render/macro_.rs | 16 ++--- 8 files changed, 119 insertions(+), 49 deletions(-) diff --git a/crates/ide_completion/src/completions.rs b/crates/ide_completion/src/completions.rs index 7dbc511395..b8a904a3a0 100644 --- a/crates/ide_completion/src/completions.rs +++ b/crates/ide_completion/src/completions.rs @@ -126,7 +126,7 @@ impl Completions { cov_mark::hit!(qualified_path_doc_hidden); return; } - self.add(render_resolution(RenderContext::new(ctx), local_name, resolution)); + self.add(render_resolution(RenderContext::new(ctx), local_name, resolution).build()); } pub(crate) fn add_resolution_simple( @@ -138,7 +138,7 @@ impl Completions { if ctx.is_scope_def_hidden(resolution) { return; } - self.add(render_resolution_simple(RenderContext::new(ctx), local_name, resolution)); + self.add(render_resolution_simple(RenderContext::new(ctx), local_name, resolution).build()); } pub(crate) fn add_macro( @@ -152,11 +152,14 @@ impl Completions { Visible::Editable => true, Visible::No => return, }; - self.add(render_macro( - RenderContext::new(ctx).private_editable(is_private_editable), - local_name, - mac, - )); + self.add( + render_macro( + RenderContext::new(ctx).private_editable(is_private_editable), + local_name, + mac, + ) + .build(), + ); } pub(crate) fn add_function( @@ -170,11 +173,14 @@ impl Completions { Visible::Editable => true, Visible::No => return, }; - self.add(render_fn( - RenderContext::new(ctx).private_editable(is_private_editable), - local_name, - func, - )); + self.add( + render_fn( + RenderContext::new(ctx).private_editable(is_private_editable), + local_name, + func, + ) + .build(), + ); } pub(crate) fn add_method( @@ -189,12 +195,15 @@ impl Completions { Visible::Editable => true, Visible::No => return, }; - self.add(render_method( - RenderContext::new(ctx).private_editable(is_private_editable), - receiver, - local_name, - func, - )); + self.add( + render_method( + RenderContext::new(ctx).private_editable(is_private_editable), + receiver, + local_name, + func, + ) + .build(), + ); } pub(crate) fn add_const(&mut self, ctx: &CompletionContext, konst: hir::Const) { @@ -235,7 +244,11 @@ impl Completions { variant: hir::Variant, path: hir::ModPath, ) { - self.add_opt(render_variant_lit(RenderContext::new(ctx), None, variant, Some(path))); + if let Some(builder) = + render_variant_lit(RenderContext::new(ctx), None, variant, Some(path)) + { + self.add(builder.build()); + } } pub(crate) fn add_enum_variant( @@ -244,7 +257,11 @@ impl Completions { variant: hir::Variant, local_name: Option, ) { - self.add_opt(render_variant_lit(RenderContext::new(ctx), local_name, variant, None)); + if let Some(builder) = + render_variant_lit(RenderContext::new(ctx), local_name, variant, None) + { + self.add(builder.build()); + } } pub(crate) fn add_field( @@ -275,8 +292,11 @@ impl Completions { path: Option, local_name: Option, ) { - let item = render_struct_literal(RenderContext::new(ctx), strukt, path, local_name); - self.add_opt(item); + if let Some(builder) = + render_struct_literal(RenderContext::new(ctx), strukt, path, local_name) + { + self.add(builder.build()); + } } pub(crate) fn add_union_literal( diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs index fb857aeccd..76d708ae51 100644 --- a/crates/ide_completion/src/completions/flyimport.rs +++ b/crates/ide_completion/src/completions/flyimport.rs @@ -205,7 +205,8 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext) &user_input_lowercased, ) }) - .filter_map(|import| render_resolution_with_import(RenderContext::new(ctx), import)), + .filter_map(|import| render_resolution_with_import(RenderContext::new(ctx), import)) + .map(|builder| builder.build()), ); Some(()) } diff --git a/crates/ide_completion/src/completions/use_.rs b/crates/ide_completion/src/completions/use_.rs index d7c279f9d9..3f757f9816 100644 --- a/crates/ide_completion/src/completions/use_.rs +++ b/crates/ide_completion/src/completions/use_.rs @@ -1,11 +1,13 @@ //! Completion for use trees use hir::ScopeDef; +use rustc_hash::FxHashSet; use syntax::{ast, AstNode}; use crate::{ context::{CompletionContext, PathCompletionCtx, PathKind, PathQualifierCtx}, - Completions, + item::Builder, + CompletionRelevance, Completions, }; pub(crate) fn complete_use_tree(acc: &mut Completions, ctx: &CompletionContext) { @@ -39,6 +41,22 @@ pub(crate) fn complete_use_tree(acc: &mut Completions, ctx: &CompletionContext) None => return, }; + let mut already_imported_names = FxHashSet::default(); + if let Some(list) = ctx.token.ancestors().find_map(ast::UseTreeList::cast) { + let use_tree = list.parent_use_tree(); + if use_tree.path().as_ref() == Some(path) { + for tree in list.use_trees() { + if tree.is_simple_path() { + if let Some(name) = + tree.path().and_then(|path| path.as_single_name_ref()) + { + already_imported_names.insert(name.to_string()); + } + } + } + } + } + match resolution { hir::PathResolution::Def(hir::ModuleDef::Module(module)) => { let module_scope = module.scope(ctx.db, Some(ctx.module)); @@ -50,6 +68,9 @@ pub(crate) fn complete_use_tree(acc: &mut Completions, ctx: &CompletionContext) ) }; for (name, def) in module_scope { + let is_name_already_imported = + already_imported_names.contains(name.as_text().unwrap().as_str()); + let add_resolution = match def { ScopeDef::Unknown if unknown_is_current(&name) => { // for `use self::foo$0`, don't suggest `foo` as a completion @@ -61,7 +82,12 @@ pub(crate) fn complete_use_tree(acc: &mut Completions, ctx: &CompletionContext) }; if add_resolution { - acc.add_resolution(ctx, name, def); + let mut builder = Builder::from_resolution(ctx, name, def); + builder.set_relevance(CompletionRelevance { + is_name_already_imported, + ..Default::default() + }); + acc.add(builder.build()); } } } diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs index 2fa8f77264..849ce0b409 100644 --- a/crates/ide_completion/src/item.rs +++ b/crates/ide_completion/src/item.rs @@ -9,6 +9,11 @@ use stdx::{impl_from, never}; use syntax::{SmolStr, TextRange}; use text_edit::TextEdit; +use crate::{ + context::CompletionContext, + render::{render_resolution, RenderContext}, +}; + /// `CompletionItem` describes a single completion variant in the editor pop-up. /// It is basically a POD with various properties. To construct a /// `CompletionItem`, use `new` method and the `Builder` struct. @@ -134,6 +139,8 @@ 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 when an import is suggested whose name is already imported. + pub is_name_already_imported: 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. @@ -200,6 +207,7 @@ impl CompletionRelevance { type_match, is_local, is_item_from_trait, + is_name_already_imported, is_op_method, is_private_editable, postfix_match, @@ -214,6 +222,10 @@ impl CompletionRelevance { if !is_op_method { score += 10; } + // lower rank for conflicting import names + if !is_name_already_imported { + score += 1; + } if exact_name_match { score += 10; } @@ -413,6 +425,14 @@ pub(crate) struct Builder { } impl Builder { + pub(crate) fn from_resolution( + ctx: &CompletionContext, + local_name: hir::Name, + resolution: hir::ScopeDef, + ) -> Self { + render_resolution(RenderContext::new(ctx), local_name, resolution) + } + pub(crate) fn build(self) -> CompletionItem { let _p = profile::span("item::Builder::build"); diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs index 5b0257f6b4..810ef63ec3 100644 --- a/crates/ide_completion/src/render.rs +++ b/crates/ide_completion/src/render.rs @@ -18,7 +18,7 @@ use syntax::{SmolStr, SyntaxKind, TextRange}; use crate::{ context::{PathCompletionCtx, PathKind}, - item::CompletionRelevanceTypeMatch, + item::{Builder, CompletionRelevanceTypeMatch}, render::{function::render_fn, literal::render_variant_lit, macro_::render_macro}, CompletionContext, CompletionItem, CompletionItemKind, CompletionRelevance, }; @@ -144,7 +144,7 @@ pub(crate) fn render_resolution( ctx: RenderContext<'_>, local_name: hir::Name, resolution: ScopeDef, -) -> CompletionItem { +) -> Builder { render_resolution_(ctx, local_name, None, resolution) } @@ -152,14 +152,14 @@ pub(crate) fn render_resolution_simple( ctx: RenderContext<'_>, local_name: hir::Name, resolution: ScopeDef, -) -> CompletionItem { +) -> Builder { render_resolution_simple_(ctx, local_name, None, resolution) } pub(crate) fn render_resolution_with_import( ctx: RenderContext<'_>, import_edit: LocatedImport, -) -> Option { +) -> Option { let resolution = ScopeDef::from(import_edit.original_item); let local_name = match resolution { ScopeDef::ModuleDef(hir::ModuleDef::Function(f)) => f.name(ctx.completion.db), @@ -182,7 +182,7 @@ fn render_resolution_( local_name: hir::Name, import_to_add: Option, resolution: ScopeDef, -) -> CompletionItem { +) -> Builder { let _p = profile::span("render_resolution"); use hir::ModuleDef::*; @@ -211,7 +211,7 @@ fn render_resolution_simple_( local_name: hir::Name, import_to_add: Option, resolution: ScopeDef, -) -> CompletionItem { +) -> Builder { let _p = profile::span("render_resolution"); use hir::ModuleDef::*; @@ -292,7 +292,7 @@ fn render_resolution_simple_( if let Some(import_to_add) = ctx.import_to_add { item.add_import(import_to_add); } - item.build() + item } fn scope_def_docs(db: &RootDatabase, resolution: ScopeDef) -> Option { @@ -625,6 +625,7 @@ fn main() { let _: m::Spam = S$0 } ), is_local: false, is_item_from_trait: false, + is_name_already_imported: false, is_op_method: false, is_private_editable: false, postfix_match: None, @@ -648,6 +649,7 @@ fn main() { let _: m::Spam = S$0 } ), is_local: false, is_item_from_trait: false, + is_name_already_imported: false, is_op_method: false, is_private_editable: false, postfix_match: None, @@ -737,6 +739,7 @@ fn foo() { A { the$0 } } ), is_local: false, is_item_from_trait: false, + is_name_already_imported: false, is_op_method: false, is_private_editable: false, postfix_match: None, diff --git a/crates/ide_completion/src/render/function.rs b/crates/ide_completion/src/render/function.rs index 211aa432c7..38520e1832 100644 --- a/crates/ide_completion/src/render/function.rs +++ b/crates/ide_completion/src/render/function.rs @@ -22,7 +22,7 @@ pub(crate) fn render_fn( ctx: RenderContext<'_>, local_name: Option, func: hir::Function, -) -> CompletionItem { +) -> Builder { let _p = profile::span("render_fn"); render(ctx, local_name, func, FuncKind::Function) } @@ -32,7 +32,7 @@ pub(crate) fn render_method( receiver: Option, local_name: Option, func: hir::Function, -) -> CompletionItem { +) -> Builder { let _p = profile::span("render_method"); render(ctx, local_name, func, FuncKind::Method(receiver)) } @@ -42,7 +42,7 @@ fn render( local_name: Option, func: hir::Function, func_kind: FuncKind, -) -> CompletionItem { +) -> Builder { let db = completion.db; let name = local_name.unwrap_or_else(|| func.name(db)); @@ -107,7 +107,7 @@ fn render( } } } - item.build() + item } pub(super) fn add_call_parens<'b>( diff --git a/crates/ide_completion/src/render/literal.rs b/crates/ide_completion/src/render/literal.rs index e65631712e..f1773137fe 100644 --- a/crates/ide_completion/src/render/literal.rs +++ b/crates/ide_completion/src/render/literal.rs @@ -5,7 +5,7 @@ use ide_db::SymbolKind; use crate::{ context::{CompletionContext, PathCompletionCtx}, - item::CompletionItem, + item::{Builder, CompletionItem}, render::{ compute_ref_match, compute_type_match, variant::{ @@ -22,7 +22,7 @@ pub(crate) fn render_variant_lit( local_name: Option, variant: hir::Variant, path: Option, -) -> Option { +) -> Option { let _p = profile::span("render_enum_variant"); let db = ctx.db(); @@ -35,7 +35,7 @@ pub(crate) fn render_struct_literal( strukt: hir::Struct, path: Option, local_name: Option, -) -> Option { +) -> Option { let _p = profile::span("render_struct_literal"); let db = ctx.db(); @@ -48,7 +48,7 @@ fn render( thing: Variant, name: hir::Name, path: Option, -) -> Option { +) -> Option { let db = completion.db; let kind = thing.kind(db); let has_call_parens = @@ -112,7 +112,7 @@ fn render( if let Some(import_to_add) = ctx.import_to_add { item.add_import(import_to_add); } - Some(item.build()) + Some(item) } #[derive(Clone, Copy)] diff --git a/crates/ide_completion/src/render/macro_.rs b/crates/ide_completion/src/render/macro_.rs index 9f848febeb..ba18e5d216 100644 --- a/crates/ide_completion/src/render/macro_.rs +++ b/crates/ide_completion/src/render/macro_.rs @@ -4,13 +4,13 @@ use hir::{Documentation, HirDisplay}; use ide_db::SymbolKind; use syntax::SmolStr; -use crate::{context::PathKind, item::CompletionItem, render::RenderContext}; +use crate::{ + context::PathKind, + item::{Builder, CompletionItem}, + render::RenderContext, +}; -pub(crate) fn render_macro( - ctx: RenderContext<'_>, - name: hir::Name, - macro_: hir::Macro, -) -> CompletionItem { +pub(crate) fn render_macro(ctx: RenderContext<'_>, name: hir::Name, macro_: hir::Macro) -> Builder { let _p = profile::span("render_macro"); render(ctx, name, macro_) } @@ -19,7 +19,7 @@ fn render( ctx @ RenderContext { completion, .. }: RenderContext<'_>, name: hir::Name, macro_: hir::Macro, -) -> CompletionItem { +) -> Builder { let source_range = if completion.is_immediately_after_macro_bang() { cov_mark::hit!(completes_macro_call_if_cursor_at_bang_token); completion.token.parent().map_or_else(|| ctx.source_range(), |it| it.text_range()) @@ -66,7 +66,7 @@ fn render( item.add_import(import_to_add); } - item.build() + item } fn label(