Merge #11961

11961: feature: deprioritize already-imported names in `use` items r=jonas-schievink a=jonas-schievink

![screenshot-2022-04-11-18:48:46](https://user-images.githubusercontent.com/1786438/162790376-6b133925-7cf9-46c5-b0e2-d8c3cba61d47.png)

Fixes #11640

bors r+

Co-authored-by: Jonas Schievink <[email protected]>

Author: bors[bot]

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<hir::Name>,
     ) {
-        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<hir::ModPath>,
         local_name: Option<hir::Name>,
     ) {
-        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(

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(())
 }

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());
                         }
                     }
                 }

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");
 

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,22 +144,22 @@ pub(crate) fn render_resolution(
     ctx: RenderContext<'_>,
     local_name: hir::Name,
     resolution: ScopeDef,
-) -> CompletionItem {
+) -> Builder {
     render_resolution_(ctx, local_name, None, resolution)
 }
 
 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<CompletionItem> {
+) -> Option<Builder> {
     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<LocatedImport>,
     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<LocatedImport>,
     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<hir::Documentation> {
@@ -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,

crates/ide_completion/src/render/function.rs

@@ -22,7 +22,7 @@ pub(crate) fn render_fn(
     ctx: RenderContext<'_>,
     local_name: Option<hir::Name>,
     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<hir::Name>,
     local_name: Option<hir::Name>,
     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<hir::Name>,
     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>(