Do not propose already imported imports

4 files changed, 45 insertions, 27 deletions

diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs
index 945638cc5..69370ef3d 100644
--- a/crates/hir/src/semantics.rs
+++ b/crates/hir/src/semantics.rs
@@ -774,7 +774,7 @@ fn find_root(node: &SyntaxNode) -> SyntaxNode {
 ///
 /// Note that if you are wondering "what does this specific existing name mean?",
 /// you'd better use the `resolve_` family of methods.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct SemanticsScope<'a> {
     pub db: &'a dyn HirDatabase,
     file_id: HirFileId,
diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs
index e9993a7cc..182547589 100644
--- a/crates/ide_assists/src/handlers/auto_import.rs
+++ b/crates/ide_assists/src/handlers/auto_import.rs
@@ -111,7 +111,9 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
     Some(())
 }
 
-pub(super) fn find_importable_node(ctx: &AssistContext) -> Option<(ImportAssets, SyntaxNode)> {
+pub(super) fn find_importable_node<'a>(
+    ctx: &'a AssistContext,
+) -> Option<(ImportAssets<'a>, SyntaxNode)> {
     if let Some(path_under_caret) = ctx.find_node_at_offset_with_descend::<ast::Path>() {
         ImportAssets::for_exact_path(&path_under_caret, &ctx.sema)
             .zip(Some(path_under_caret.syntax().clone()))
diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs
index af49fdd26..d6adf70b1 100644
--- a/crates/ide_completion/src/completions/flyimport.rs
+++ b/crates/ide_completion/src/completions/flyimport.rs
@@ -48,12 +48,11 @@
 //! Note that having this flag set to `true` does not guarantee that the feature is enabled: your client needs to have the corredponding
 //! capability enabled.
 
-use hir::{AsAssocItem, ModPath, ScopeDef};
+use hir::{AsAssocItem, ItemInNs, ModPath, ScopeDef};
 use ide_db::helpers::{
     import_assets::{ImportAssets, ImportCandidate},
     insert_use::ImportScope,
 };
-use rustc_hash::FxHashSet;
 use syntax::{AstNode, SyntaxNode, T};
 
 use crate::{
@@ -92,19 +91,17 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext)
         &ctx.sema,
     )?;
 
-    let scope_definitions = scope_definitions(ctx);
     let mut all_mod_paths = import_assets
         .search_for_imports(&ctx.sema, ctx.config.insert_use.prefix_kind)
         .into_iter()
         .map(|import| {
             let proposed_def = match import.item_to_display() {
-                hir::ItemInNs::Types(id) => ScopeDef::ModuleDef(id.into()),
-                hir::ItemInNs::Values(id) => ScopeDef::ModuleDef(id.into()),
-                hir::ItemInNs::Macros(id) => ScopeDef::MacroDef(id.into()),
+                ItemInNs::Types(id) => ScopeDef::ModuleDef(id.into()),
+                ItemInNs::Values(id) => ScopeDef::ModuleDef(id.into()),
+                ItemInNs::Macros(id) => ScopeDef::MacroDef(id.into()),
             };
             (import, proposed_def)
         })
-        .filter(|(_, proposed_def)| !scope_definitions.contains(proposed_def))
         .collect::<Vec<_>>();
     all_mod_paths.sort_by_cached_key(|(import, _)| {
         compute_fuzzy_completion_order_key(import.display_path(), &user_input_lowercased)
@@ -125,14 +122,6 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext)
     Some(())
 }
 
-fn scope_definitions(ctx: &CompletionContext) -> FxHashSet<ScopeDef> {
-    let mut scope_definitions = FxHashSet::default();
-    ctx.scope.process_all_names(&mut |_, scope_def| {
-        scope_definitions.insert(scope_def);
-    });
-    scope_definitions
-}
-
 pub(crate) fn position_for_import<'a>(
     ctx: &'a CompletionContext,
     import_candidate: Option<&ImportCandidate>,
@@ -150,13 +139,14 @@ pub(crate) fn position_for_import<'a>(
     })
 }
 
-fn import_assets(ctx: &CompletionContext, fuzzy_name: String) -> Option<ImportAssets> {
+fn import_assets<'a>(ctx: &'a CompletionContext, fuzzy_name: String) -> Option<ImportAssets<'a>> {
     let current_module = ctx.scope.module()?;
     if let Some(dot_receiver) = &ctx.dot_receiver {
         ImportAssets::for_fuzzy_method_call(
             current_module,
             ctx.sema.type_of_expr(dot_receiver)?,
             fuzzy_name,
+            ctx.scope.clone(),
         )
     } else {
         let fuzzy_name_length = fuzzy_name.len();
@@ -165,6 +155,7 @@ fn import_assets(ctx: &CompletionContext, fuzzy_name: String) -> Option<ImportAs
             ctx.path_qual.clone(),
             fuzzy_name,
             &ctx.sema,
+            ctx.scope.clone(),
         )?;
 
         if matches!(assets_for_path.import_candidate(), ImportCandidate::Path(_))
diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs
index 2e7a183d1..b25786928 100644
--- a/crates/ide_db/src/helpers/import_assets.rs
+++ b/crates/ide_db/src/helpers/import_assets.rs
@@ -2,7 +2,7 @@
 use either::Either;
 use hir::{
     AsAssocItem, AssocItem, AssocItemContainer, Crate, ItemInNs, MacroDef, ModPath, Module,
-    ModuleDef, PathResolution, PrefixKind, Semantics, Type,
+    ModuleDef, PathResolution, PrefixKind, ScopeDef, Semantics, SemanticsScope, Type,
 };
 use rustc_hash::FxHashSet;
 use syntax::{ast, AstNode};
@@ -62,33 +62,38 @@ impl NameToImport {
 }
 
 #[derive(Debug)]
-pub struct ImportAssets {
+pub struct ImportAssets<'a> {
     import_candidate: ImportCandidate,
     module_with_candidate: Module,
+    scope: SemanticsScope<'a>,
 }
 
-impl ImportAssets {
+impl<'a> ImportAssets<'a> {
     pub fn for_method_call(
         method_call: &ast::MethodCallExpr,
-        sema: &Semantics<RootDatabase>,
+        sema: &'a Semantics<RootDatabase>,
     ) -> Option<Self> {
+        let scope = sema.scope(method_call.syntax());
         Some(Self {
             import_candidate: ImportCandidate::for_method_call(sema, method_call)?,
-            module_with_candidate: sema.scope(method_call.syntax()).module()?,
+            module_with_candidate: scope.module()?,
+            scope,
         })
     }
 
     pub fn for_exact_path(
         fully_qualified_path: &ast::Path,
-        sema: &Semantics<RootDatabase>,
+        sema: &'a Semantics<RootDatabase>,
     ) -> Option<Self> {
         let syntax_under_caret = fully_qualified_path.syntax();
         if syntax_under_caret.ancestors().find_map(ast::Use::cast).is_some() {
             return None;
         }
+        let scope = sema.scope(syntax_under_caret);
         Some(Self {
             import_candidate: ImportCandidate::for_regular_path(sema, fully_qualified_path)?,
-            module_with_candidate: sema.scope(syntax_under_caret).module()?,
+            module_with_candidate: scope.module()?,
+            scope,
         })
     }
 
@@ -97,10 +102,12 @@ impl ImportAssets {
         qualifier: Option<ast::Path>,
         fuzzy_name: String,
         sema: &Semantics<RootDatabase>,
+        scope: SemanticsScope<'a>,
     ) -> Option<Self> {
         Some(Self {
             import_candidate: ImportCandidate::for_fuzzy_path(qualifier, fuzzy_name, sema)?,
             module_with_candidate,
+            scope,
         })
     }
 
@@ -108,6 +115,7 @@ impl ImportAssets {
         module_with_method_call: Module,
         receiver_ty: Type,
         fuzzy_method_name: String,
+        scope: SemanticsScope<'a>,
     ) -> Option<Self> {
         Some(Self {
             import_candidate: ImportCandidate::TraitMethod(TraitImportCandidate {
@@ -115,6 +123,7 @@ impl ImportAssets {
                 name: NameToImport::Fuzzy(fuzzy_method_name),
             }),
             module_with_candidate: module_with_method_call,
+            scope,
         })
     }
 }
@@ -155,7 +164,7 @@ impl LocatedImport {
     }
 }
 
-impl ImportAssets {
+impl<'a> ImportAssets<'a> {
     pub fn import_candidate(&self) -> &ImportCandidate {
         &self.import_candidate
     }
@@ -189,6 +198,7 @@ impl ImportAssets {
         prefixed: Option<PrefixKind>,
     ) -> Vec<LocatedImport> {
         let current_crate = self.module_with_candidate.krate();
+        let scope_definitions = self.scope_definitions();
 
         let imports_for_candidate_name = match self.name_to_import() {
             NameToImport::Exact(exact_name) => {
@@ -219,9 +229,25 @@ impl ImportAssets {
         self.applicable_defs(sema.db, prefixed, imports_for_candidate_name)
             .into_iter()
             .filter(|import| import.import_path().len() > 1)
+            .filter(|import| {
+                let proposed_def = match import.item_to_import() {
+                    ItemInNs::Types(id) => ScopeDef::ModuleDef(id.into()),
+                    ItemInNs::Values(id) => ScopeDef::ModuleDef(id.into()),
+                    ItemInNs::Macros(id) => ScopeDef::MacroDef(id.into()),
+                };
+                !scope_definitions.contains(&proposed_def)
+            })
             .collect()
     }
 
+    fn scope_definitions(&self) -> FxHashSet<ScopeDef> {
+        let mut scope_definitions = FxHashSet::default();
+        self.scope.process_all_names(&mut |_, scope_def| {
+            scope_definitions.insert(scope_def);
+        });
+        scope_definitions
+    }
+
     fn applicable_defs(
         &self,
         db: &RootDatabase,
@@ -297,7 +323,6 @@ fn path_applicable_imports(
         Qualifier::FirstSegmentUnresolved(first_segment, qualifier) => (first_segment, qualifier),
     };
 
-    // TODO kb zz.syntax().ast_node() <- two options are now proposed despite the trait being imported
     let unresolved_qualifier_string = unresolved_qualifier.to_string();
     let unresolved_first_segment_string = unresolved_first_segment.to_string();