From 89d410cef571f5fa7631b17e2fbe52a8f8f03990 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 28 Feb 2021 10:32:15 +0200 Subject: [PATCH] Do not propose already imported imports --- crates/hir/src/semantics.rs | 2 +- .../ide_assists/src/handlers/auto_import.rs | 4 +- .../src/completions/flyimport.rs | 23 +++------- crates/ide_db/src/helpers/import_assets.rs | 43 +++++++++++++++---- 4 files changed, 45 insertions(+), 27 deletions(-) diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index 945638cc565..69370ef3d86 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 e9993a7cc9f..18254758932 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::() { 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 af49fdd2626..d6adf70b138 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::>(); 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 { - 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 { +fn import_assets<'a>(ctx: &'a CompletionContext, fuzzy_name: String) -> Option> { 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 { 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, + sema: &'a Semantics, ) -> Option { + 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, + sema: &'a Semantics, ) -> Option { 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, fuzzy_name: String, sema: &Semantics, + scope: SemanticsScope<'a>, ) -> Option { 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 { 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, ) -> Vec { 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 { + 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();