Migrate merge imports to editor

crates/ide-assists/src/handlers/merge_imports.rs

@@ -1,10 +1,12 @@
-use either::Either;
 use ide_db::imports::{
     insert_use::{ImportGranularity, InsertUseConfig},
     merge_imports::{MergeBehavior, try_merge_imports, try_merge_trees},
 };
 use syntax::{
-    AstNode, SyntaxElement, SyntaxNode, algo::neighbor, ast, match_ast, syntax_editor::Removable,
+    AstNode, SyntaxElement,
+    algo::neighbor,
+    ast, match_ast,
+    syntax_editor::{Removable, SyntaxEditor},
 };
 
 use crate::{
@@ -13,8 +15,6 @@ use crate::{
     utils::next_prev,
 };
 
-use Edit::*;
-
 // Assist: merge_imports
 //
 // Merges neighbor imports with a common prefix.
@@ -28,16 +28,19 @@ use Edit::*;
 // use std::{fmt::Formatter, io};
 // ```
 pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_, '_>) -> Option<()> {
-    let (target, edits) = if ctx.has_empty_selection() {
+    let (target, editor) = if ctx.has_empty_selection() {
         // Merge a neighbor
         cov_mark::hit!(merge_with_use_item_neighbors);
         let tree = ctx.find_node_at_offset::<ast::UseTree>()?.top_use_tree();
         let target = tree.syntax().text_range();
 
         let use_item = tree.syntax().parent().and_then(ast::Use::cast)?;
-        let mut neighbor = next_prev().find_map(|dir| neighbor(&use_item, dir)).into_iter();
-        let edits = use_item.try_merge_from(&mut neighbor, &ctx.config.insert_use);
-        (target, edits?)
+        let neighbor = next_prev().find_map(|dir| neighbor(&use_item, dir))?;
+        let parent_node = use_item.syntax().parent()?;
+        let (editor, _) =
+            SyntaxEditor::new(parent_node.ancestors().last().unwrap_or(parent_node.clone()));
+        merge_uses(use_item, vec![neighbor], &ctx.config.insert_use, &editor)?;
+        (target, editor)
     } else {
         // Merge selected
         let selection_range = ctx.selection_trimmed();
@@ -50,104 +53,81 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_, '_>) -> O
         });
 
         let first_selected = selected_nodes.next()?;
-        let edits = match_ast! {
+        let (editor, _) =
+            SyntaxEditor::new(parent_node.ancestors().last().unwrap_or(parent_node.clone()));
+        match_ast! {
             match first_selected {
                 ast::Use(use_item) => {
                     cov_mark::hit!(merge_with_selected_use_item_neighbors);
-                    use_item.try_merge_from(&mut selected_nodes.filter_map(ast::Use::cast), &ctx.config.insert_use)
+                    merge_uses(
+                        use_item,
+                        selected_nodes.filter_map(ast::Use::cast).collect(),
+                        &ctx.config.insert_use,
+                        &editor,
+                    )?;
                 },
                 ast::UseTree(use_tree) => {
                     cov_mark::hit!(merge_with_selected_use_tree_neighbors);
-                    use_tree.try_merge_from(&mut selected_nodes.filter_map(ast::UseTree::cast), &ctx.config.insert_use)
+                    merge_use_trees(
+                        use_tree,
+                        selected_nodes.filter_map(ast::UseTree::cast).collect(),
+                        &editor,
+                    )?;
                 },
                 _ => return None,
             }
-        };
-        (selection_range, edits?)
-    };
-
-    let parent_node = match ctx.covering_element() {
-        SyntaxElement::Node(n) => n,
-        SyntaxElement::Token(t) => t.parent()?,
+        }
+        (selection_range, editor)
     };
 
     acc.add(AssistId::refactor_rewrite("merge_imports"), "Merge imports", target, |builder| {
-        let editor = builder.make_editor(&parent_node);
-
-        for edit in edits {
-            match edit {
-                Remove(it) => {
-                    let node = it.as_ref();
-                    if let Some(left) = node.left() {
-                        left.remove(&editor);
-                    } else if let Some(right) = node.right() {
-                        right.remove(&editor);
-                    }
-                }
-                Replace(old, new) => {
-                    editor.replace(old, &new);
-                }
-            }
-        }
         builder.add_file_edits(ctx.vfs_file_id(), editor);
     })
 }
 
-trait Merge: AstNode + Clone {
-    fn try_merge_from(
-        self,
-        items: &mut dyn Iterator<Item = Self>,
-        cfg: &InsertUseConfig,
-    ) -> Option<Vec<Edit>> {
-        let mut edits = Vec::new();
-        let mut merged = self.clone();
-        for item in items {
-            merged = merged.try_merge(&item, cfg)?;
-            edits.push(Edit::Remove(item.into_either()));
-        }
-        if !edits.is_empty() {
-            edits.push(Edit::replace(self, merged));
-            Some(edits)
-        } else {
-            None
-        }
+fn merge_uses(
+    first: ast::Use,
+    rest: Vec<ast::Use>,
+    cfg: &InsertUseConfig,
+    editor: &SyntaxEditor,
+) -> Option<()> {
+    if rest.is_empty() {
+        return None;
     }
-    fn try_merge(&self, other: &Self, cfg: &InsertUseConfig) -> Option<Self>;
-    fn into_either(self) -> Either<ast::Use, ast::UseTree>;
-}
 
-impl Merge for ast::Use {
-    fn try_merge(&self, other: &Self, cfg: &InsertUseConfig) -> Option<Self> {
-        let mb = match cfg.granularity {
-            ImportGranularity::One => MergeBehavior::One,
-            _ => MergeBehavior::Crate,
-        };
-        try_merge_imports(self, other, mb)
+    let mb = match cfg.granularity {
+        ImportGranularity::One => MergeBehavior::One,
+        _ => MergeBehavior::Crate,
+    };
+    let mut merged = first.clone();
+    for item in &rest {
+        merged = try_merge_imports(editor, &merged, item, mb)?;
     }
-    fn into_either(self) -> Either<ast::Use, ast::UseTree> {
-        Either::Left(self)
+    for item in rest {
+        item.remove(editor);
     }
+    editor.replace(first.syntax(), merged.syntax());
+    Some(())
 }
 
-impl Merge for ast::UseTree {
-    fn try_merge(&self, other: &Self, _: &InsertUseConfig) -> Option<Self> {
-        try_merge_trees(self, other, MergeBehavior::Crate)
-    }
-    fn into_either(self) -> Either<ast::Use, ast::UseTree> {
-        Either::Right(self)
+fn merge_use_trees(
+    first: ast::UseTree,
+    rest: Vec<ast::UseTree>,
+    editor: &SyntaxEditor,
+) -> Option<()> {
+    if rest.is_empty() {
+        return None;
     }
-}
-
-#[derive(Debug)]
-enum Edit {
-    Remove(Either<ast::Use, ast::UseTree>),
-    Replace(SyntaxNode, SyntaxNode),
-}
 
-impl Edit {
-    fn replace(old: impl AstNode, new: impl AstNode) -> Self {
-        Edit::Replace(old.syntax().clone(), new.syntax().clone())
+    let mut merged = first.clone();
+    for item in &rest {
+        merged = try_merge_trees(editor, &merged, item, MergeBehavior::Crate)?;
+    }
+    for item in rest {
+        item.remove(editor);
     }
+    editor.replace(first.syntax(), merged.syntax());
+    Some(())
 }
 
 #[cfg(test)]

crates/ide-assists/src/handlers/normalize_import.rs

@@ -1,5 +1,5 @@
 use ide_db::imports::merge_imports::try_normalize_import;
-use syntax::{AstNode, ast};
+use syntax::{AstNode, ast, syntax_editor::SyntaxEditor};
 
 use crate::{
     AssistId,
@@ -25,11 +25,13 @@ pub(crate) fn normalize_import(acc: &mut Assists, ctx: &AssistContext<'_, '_>) -
     };
 
     let target = use_item.syntax().text_range();
+    let (editor, _) = SyntaxEditor::new(use_item.syntax().ancestors().last()?);
     let normalized_use_item =
-        try_normalize_import(&use_item, ctx.config.insert_use.granularity.into())?;
+        try_normalize_import(&editor, &use_item, ctx.config.insert_use.granularity.into())?;
+    editor.replace(use_item.syntax(), normalized_use_item.syntax());
 
     acc.add(AssistId::refactor_rewrite("normalize_import"), "Normalize import", target, |builder| {
-        builder.replace_ast(use_item, normalized_use_item);
+        builder.add_file_edits(ctx.vfs_file_id(), editor);
     })
 }
 

crates/ide-db/src/imports/insert_use.rs

@@ -17,7 +17,7 @@ use crate::{
     RootDatabase,
     imports::merge_imports::{
         MergeBehavior, NormalizationStyle, common_prefix, eq_attrs, eq_visibility,
-        try_merge_imports, use_tree_cmp,
+        try_merge_imports, use_tree_cmp, wrap_in_tree_list,
     },
 };
 
@@ -251,7 +251,7 @@ fn insert_use_with_alias_option_with_editor(
     let mut use_tree = make.use_tree(path, None, alias, false);
     if mb == Some(MergeBehavior::One)
         && use_tree.path().is_some()
-        && let Some(wrapped) = use_tree.wrap_in_tree_list_with_editor()
+        && let Some(wrapped) = wrap_in_tree_list(&use_tree, make)
     {
         use_tree = wrapped;
     }
@@ -263,7 +263,7 @@ fn insert_use_with_alias_option_with_editor(
         for existing_use in
             scope.as_syntax_node().children().filter_map(ast::Use::cast).filter(filter)
         {
-            if let Some(merged) = try_merge_imports(&existing_use, &use_item, mb) {
+            if let Some(merged) = try_merge_imports(syntax_editor, &existing_use, &use_item, mb) {
                 syntax_editor.replace(existing_use.syntax(), merged.syntax());
                 return;
             }

crates/ide-db/src/imports/insert_use/tests.rs

@@ -1430,7 +1430,8 @@ fn check_merge_only_fail(ra_fixture0: &str, ra_fixture1: &str, mb: MergeBehavior
         .find_map(ast::Use::cast)
         .unwrap();
 
-    let result = try_merge_imports(&use0, &use1, mb);
+    let (editor, _) = SyntaxEditor::new(use0.syntax().ancestors().last().unwrap());
+    let result = try_merge_imports(&editor, &use0, &use1, mb);
     assert_eq!(result.map(|u| u.to_string()), None);
 }
 
@@ -1495,7 +1496,8 @@ fn check_merge(ra_fixture0: &str, ra_fixture1: &str, last: &str, mb: MergeBehavi
         .find_map(ast::Use::cast)
         .unwrap();
 
-    let result = try_merge_imports(&use0, &use1, mb);
+    let (editor, _) = SyntaxEditor::new(use0.syntax().ancestors().last().unwrap());
+    let result = try_merge_imports(&editor, &use0, &use1, mb);
     assert_eq!(result.map(|u| u.to_string().trim().to_owned()), Some(last.trim().to_owned()));
 }
 
@@ -1525,7 +1527,8 @@ fn merge_gated_imports_with_different_values() {
         .find_map(ast::Use::cast)
         .unwrap();
 
-    let result = try_merge_imports(&use0, &use1, MergeBehavior::Crate);
+    let (editor, _) = SyntaxEditor::new(use0.syntax().ancestors().last().unwrap());
+    let result = try_merge_imports(&editor, &use0, &use1, MergeBehavior::Crate);
     assert_eq!(result, None);
 }