Simplify completion import insertion

Author: Veykril

crates/ide/src/lib.rs

@@ -102,7 +102,7 @@ pub use ide_assists::{
     Assist, AssistConfig, AssistId, AssistKind, AssistResolveStrategy, SingleResolve,
 };
 pub use ide_completion::{
-    CompletionConfig, CompletionItem, CompletionItemKind, CompletionRelevance, ImportEdit, Snippet,
+    CompletionConfig, CompletionItem, CompletionItemKind, CompletionRelevance, Snippet,
     SnippetScope,
 };
 pub use ide_db::{

crates/ide_completion/src/completions/flyimport.rs

@@ -10,7 +10,6 @@ use syntax::{AstNode, SyntaxNode, T};
 use crate::{
     context::{CompletionContext, PathKind},
     render::{render_resolution_with_import, RenderContext},
-    ImportEdit,
 };
 
 use super::Completions;
@@ -136,10 +135,10 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext)
 
     let user_input_lowercased = potential_import_name.to_lowercase();
     let import_assets = import_assets(ctx, potential_import_name)?;
-    let import_scope = ImportScope::find_insert_use_container(
-        &position_for_import(ctx, Some(import_assets.import_candidate()))?,
-        &ctx.sema,
-    )?;
+    let position = position_for_import(ctx, Some(import_assets.import_candidate()))?;
+    if ImportScope::find_insert_use_container(&position, &ctx.sema).is_none() {
+        return None;
+    }
 
     let path_kind = match ctx.path_kind() {
         Some(kind) => Some(kind),
@@ -199,12 +198,7 @@ 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),
-                    ImportEdit { import, scope: import_scope.clone() },
-                )
-            }),
+            .filter_map(|import| render_resolution_with_import(RenderContext::new(ctx), import)),
     );
     Some(())
 }

crates/ide_completion/src/completions/postfix.rs

@@ -261,10 +261,12 @@ fn add_custom_postfix_completions(
     postfix_snippet: impl Fn(&str, &str, &str) -> Builder,
     receiver_text: &str,
 ) -> Option<()> {
-    let import_scope = ImportScope::find_insert_use_container(&ctx.token.parent()?, &ctx.sema)?;
+    if ImportScope::find_insert_use_container(&ctx.token.parent()?, &ctx.sema).is_none() {
+        return None;
+    }
     ctx.config.postfix_snippets().filter(|(_, snip)| snip.scope == SnippetScope::Expr).for_each(
         |(trigger, snippet)| {
-            let imports = match snippet.imports(ctx, &import_scope) {
+            let imports = match snippet.imports(ctx) {
                 Some(imports) => imports,
                 None => return,
             };

crates/ide_completion/src/completions/snippet.rs

@@ -104,10 +104,12 @@ fn add_custom_completions(
     cap: SnippetCap,
     scope: SnippetScope,
 ) -> Option<()> {
-    let import_scope = ImportScope::find_insert_use_container(&ctx.token.parent()?, &ctx.sema)?;
+    if ImportScope::find_insert_use_container(&ctx.token.parent()?, &ctx.sema).is_none() {
+        return None;
+    }
     ctx.config.prefix_snippets().filter(|(_, snip)| snip.scope == scope).for_each(
         |(trigger, snip)| {
-            let imports = match snip.imports(ctx, &import_scope) {
+            let imports = match snip.imports(ctx) {
                 Some(imports) => imports,
                 None => return,
             };

crates/ide_completion/src/item.rs

@@ -3,17 +3,10 @@
 use std::fmt;
 
 use hir::{Documentation, Mutability};
-use ide_db::{
-    helpers::mod_path_to_ast,
-    imports::{
-        import_assets::LocatedImport,
-        insert_use::{self, ImportScope, InsertUseConfig},
-    },
-    SnippetCap, SymbolKind,
-};
+use ide_db::{imports::import_assets::LocatedImport, SnippetCap, SymbolKind};
 use smallvec::SmallVec;
 use stdx::{impl_from, never};
-use syntax::{algo, SmolStr, TextRange};
+use syntax::{SmolStr, TextRange};
 use text_edit::TextEdit;
 
 /// `CompletionItem` describes a single completion variant in the editor pop-up.
@@ -73,7 +66,7 @@ pub struct CompletionItem {
     ref_match: Option<Mutability>,
 
     /// The import data to add to completion's edits.
-    import_to_add: SmallVec<[ImportEdit; 1]>,
+    import_to_add: SmallVec<[LocatedImport; 1]>,
 }
 
 // We use custom debug for CompletionItem to make snapshot tests more readable.
@@ -380,40 +373,17 @@ impl CompletionItem {
         self.ref_match.map(|mutability| (mutability, relevance))
     }
 
-    pub fn imports_to_add(&self) -> &[ImportEdit] {
+    pub fn imports_to_add(&self) -> &[LocatedImport] {
         &self.import_to_add
     }
 }
 
-/// An extra import to add after the completion is applied.
-#[derive(Debug, Clone)]
-pub struct ImportEdit {
-    pub import: LocatedImport,
-    pub scope: ImportScope,
-}
-
-impl ImportEdit {
-    /// Attempts to insert the import to the given scope, producing a text edit.
-    /// May return no edit in edge cases, such as scope already containing the import.
-    pub fn to_text_edit(&self, cfg: InsertUseConfig) -> Option<TextEdit> {
-        let _p = profile::span("ImportEdit::to_text_edit");
-
-        let new_ast = self.scope.clone_for_update();
-        insert_use::insert_use(&new_ast, mod_path_to_ast(&self.import.import_path), &cfg);
-        let mut import_insert = TextEdit::builder();
-        algo::diff(self.scope.as_syntax_node(), new_ast.as_syntax_node())
-            .into_text_edit(&mut import_insert);
-
-        Some(import_insert.finish())
-    }
-}
-
 /// A helper to make `CompletionItem`s.
 #[must_use]
 #[derive(Clone)]
 pub(crate) struct Builder {
     source_range: TextRange,
-    imports_to_add: SmallVec<[ImportEdit; 1]>,
+    imports_to_add: SmallVec<[LocatedImport; 1]>,
     trait_name: Option<SmolStr>,
     label: SmolStr,
     insert_text: Option<String>,
@@ -439,7 +409,7 @@ impl Builder {
 
         if let [import_edit] = &*self.imports_to_add {
             // snippets can have multiple imports, but normal completions only have up to one
-            if let Some(original_path) = import_edit.import.original_path.as_ref() {
+            if let Some(original_path) = import_edit.original_path.as_ref() {
                 lookup = lookup.or_else(|| Some(label.clone()));
                 label = SmolStr::from(format!("{} (use {})", label, original_path));
             }
@@ -533,7 +503,7 @@ impl Builder {
         self.trigger_call_info = Some(true);
         self
     }
-    pub(crate) fn add_import(&mut self, import_to_add: ImportEdit) -> &mut Builder {
+    pub(crate) fn add_import(&mut self, import_to_add: LocatedImport) -> &mut Builder {
         self.imports_to_add.push(import_to_add);
         self
     }

crates/ide_completion/src/lib.rs

@@ -30,7 +30,6 @@ pub use crate::{
     config::CompletionConfig,
     item::{
         CompletionItem, CompletionItemKind, CompletionRelevance, CompletionRelevancePostfixMatch,
-        ImportEdit,
     },
     snippet::{Snippet, SnippetScope},
 };
@@ -193,7 +192,6 @@ pub fn resolve_completion_edits(
     let new_ast = scope.clone_for_update();
     let mut import_insert = TextEdit::builder();
 
-    // FIXME: lift out and make some tests here, this is ImportEdit::to_text_edit but changed to work with multiple edits
     imports.into_iter().for_each(|(full_import_path, imported_name)| {
         let items_with_name = items_locator::items_with_name(
             &ctx.sema,

crates/ide_completion/src/render.rs

@@ -11,12 +11,14 @@ pub(crate) mod union_literal;
 pub(crate) mod literal;
 
 use hir::{AsAssocItem, HasAttrs, HirDisplay, ScopeDef};
-use ide_db::{helpers::item_name, RootDatabase, SnippetCap, SymbolKind};
+use ide_db::{
+    helpers::item_name, imports::import_assets::LocatedImport, RootDatabase, SnippetCap, SymbolKind,
+};
 use syntax::{SmolStr, SyntaxKind, TextRange};
 
 use crate::{
     context::{PathCompletionCtx, PathKind},
-    item::{CompletionRelevanceTypeMatch, ImportEdit},
+    item::CompletionRelevanceTypeMatch,
     render::{function::render_fn, literal::render_variant_lit, macro_::render_macro},
     CompletionContext, CompletionItem, CompletionItemKind, CompletionRelevance,
 };
@@ -25,7 +27,7 @@ use crate::{
 pub(crate) struct RenderContext<'a> {
     completion: &'a CompletionContext<'a>,
     is_private_editable: bool,
-    import_to_add: Option<ImportEdit>,
+    import_to_add: Option<LocatedImport>,
 }
 
 impl<'a> RenderContext<'a> {
@@ -38,7 +40,7 @@ impl<'a> RenderContext<'a> {
         self
     }
 
-    pub(crate) fn import_to_add(mut self, import_to_add: Option<ImportEdit>) -> Self {
+    pub(crate) fn import_to_add(mut self, import_to_add: Option<LocatedImport>) -> Self {
         self.import_to_add = import_to_add;
         self
     }
@@ -156,22 +158,22 @@ pub(crate) fn render_resolution_simple(
 
 pub(crate) fn render_resolution_with_import(
     ctx: RenderContext<'_>,
-    import_edit: ImportEdit,
+    import_edit: LocatedImport,
 ) -> Option<CompletionItem> {
-    let resolution = ScopeDef::from(import_edit.import.original_item);
+    let resolution = ScopeDef::from(import_edit.original_item);
     let local_name = match resolution {
         ScopeDef::ModuleDef(hir::ModuleDef::Function(f)) => f.name(ctx.completion.db),
         ScopeDef::ModuleDef(hir::ModuleDef::Const(c)) => c.name(ctx.completion.db)?,
         ScopeDef::ModuleDef(hir::ModuleDef::TypeAlias(t)) => t.name(ctx.completion.db),
-        _ => item_name(ctx.db(), import_edit.import.original_item)?,
+        _ => item_name(ctx.db(), import_edit.original_item)?,
     };
     Some(render_resolution_(ctx, local_name, Some(import_edit), resolution))
 }
 
 fn render_resolution_(
     ctx: RenderContext<'_>,
     local_name: hir::Name,
-    import_to_add: Option<ImportEdit>,
+    import_to_add: Option<LocatedImport>,
     resolution: ScopeDef,
 ) -> CompletionItem {
     let _p = profile::span("render_resolution");
@@ -200,7 +202,7 @@ fn render_resolution_(
 fn render_resolution_simple_(
     ctx: RenderContext<'_>,
     local_name: hir::Name,
-    import_to_add: Option<ImportEdit>,
+    import_to_add: Option<LocatedImport>,
     resolution: ScopeDef,
 ) -> CompletionItem {
     let _p = profile::span("render_resolution");

crates/ide_completion/src/snippet.rs

@@ -102,11 +102,11 @@ use std::ops::Deref;
 // }
 // ----
 
-use ide_db::imports::{import_assets::LocatedImport, insert_use::ImportScope};
+use ide_db::imports::import_assets::LocatedImport;
 use itertools::Itertools;
 use syntax::{ast, AstNode, GreenNode, SyntaxNode};
 
-use crate::{context::CompletionContext, ImportEdit};
+use crate::context::CompletionContext;
 
 /// A snippet scope describing where a snippet may apply to.
 /// These may differ slightly in meaning depending on the snippet trigger.
@@ -156,12 +156,8 @@ impl Snippet {
     }
 
     /// Returns [`None`] if the required items do not resolve.
-    pub(crate) fn imports(
-        &self,
-        ctx: &CompletionContext,
-        import_scope: &ImportScope,
-    ) -> Option<Vec<ImportEdit>> {
-        import_edits(ctx, import_scope, &self.requires)
+    pub(crate) fn imports(&self, ctx: &CompletionContext) -> Option<Vec<LocatedImport>> {
+        import_edits(ctx, &self.requires)
     }
 
     pub fn snippet(&self) -> String {
@@ -173,11 +169,7 @@ impl Snippet {
     }
 }
 
-fn import_edits(
-    ctx: &CompletionContext,
-    import_scope: &ImportScope,
-    requires: &[GreenNode],
-) -> Option<Vec<ImportEdit>> {
+fn import_edits(ctx: &CompletionContext, requires: &[GreenNode]) -> Option<Vec<LocatedImport>> {
     let resolve = |import: &GreenNode| {
         let path = ast::Path::cast(SyntaxNode::new_root(import.clone()))?;
         let item = match ctx.scope.speculative_resolve(&path)? {
@@ -186,10 +178,7 @@ fn import_edits(
         };
         let path =
             ctx.module.find_use_path_prefixed(ctx.db, item, ctx.config.insert_use.prefix_kind)?;
-        Some((path.len() > 1).then(|| ImportEdit {
-            import: LocatedImport::new(path.clone(), item, item, None),
-            scope: import_scope.clone(),
-        }))
+        Some((path.len() > 1).then(|| LocatedImport::new(path.clone(), item, item, None)))
     };
     let mut res = Vec::with_capacity(requires.len());
     for import in requires {

crates/ide_completion/src/tests.rs

@@ -35,7 +35,7 @@ use stdx::{format_to, trim_indent};
 use syntax::{AstNode, NodeOrToken, SyntaxElement};
 use test_utils::assert_eq_text;
 
-use crate::{CompletionConfig, CompletionItem, CompletionItemKind};
+use crate::{resolve_completion_edits, CompletionConfig, CompletionItem, CompletionItemKind};
 
 /// Lots of basic item definitions
 const BASE_ITEMS_FIXTURE: &str = r#"
@@ -178,15 +178,24 @@ pub(crate) fn check_edit_with_config(
     let mut actual = db.file_text(position.file_id).to_string();
 
     let mut combined_edit = completion.text_edit().to_owned();
-    completion
-        .imports_to_add()
-        .iter()
-        .filter_map(|edit| edit.to_text_edit(config.insert_use))
-        .for_each(|text_edit| {
-            combined_edit.union(text_edit).expect(
-                "Failed to apply completion resolve changes: change ranges overlap, but should not",
-            )
-        });
+
+    resolve_completion_edits(
+        &db,
+        &config,
+        position,
+        completion.imports_to_add().iter().filter_map(|import_edit| {
+            let import_path = &import_edit.import_path;
+            let import_name = import_path.segments().last()?;
+            Some((import_path.to_string(), import_name.to_string()))
+        }),
+    )
+    .into_iter()
+    .flatten()
+    .for_each(|text_edit| {
+        combined_edit.union(text_edit).expect(
+            "Failed to apply completion resolve changes: change ranges overlap, but should not",
+        )
+    });
 
     combined_edit.apply(&mut actual);
     assert_eq_text!(&ra_fixture_after, &actual)

crates/rust-analyzer/src/to_proto.rs

@@ -283,7 +283,7 @@ fn completion_item(
             let imports: Vec<_> = imports
                 .iter()
                 .filter_map(|import_edit| {
-                    let import_path = &import_edit.import.import_path;
+                    let import_path = &import_edit.import_path;
                     let import_name = import_path.segments().last()?;
                     Some(lsp_ext::CompletionImport {
                         full_import_path: import_path.to_string(),