10 files changed, 436 insertions, 103 deletions

diff --git a/crates/ra_assists/Cargo.toml b/crates/ra_assists/Cargo.toml
index d314dc8e6..a87f4052a 100644
--- a/crates/ra_assists/Cargo.toml
+++ b/crates/ra_assists/Cargo.toml
@@ -11,6 +11,8 @@ doctest = false
 format-buf = "1.0.0"
 join_to_string = "0.1.3"
 rustc-hash = "1.1.0"
+itertools = "0.9.0"
+either = "1.5.3"
 
 ra_syntax = { path = "../ra_syntax" }
 ra_text_edit = { path = "../ra_text_edit" }
diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs
index 62182cf03..c3e653299 100644
--- a/crates/ra_assists/src/assist_ctx.rs
+++ b/crates/ra_assists/src/assist_ctx.rs
@@ -11,6 +11,7 @@ use ra_syntax::{
 use ra_text_edit::TextEditBuilder;
 
 use crate::{AssistAction, AssistId, AssistLabel, GroupLabel, ResolvedAssist};
+use algo::SyntaxRewriter;
 
 #[derive(Clone, Debug)]
 pub(crate) struct Assist(pub(crate) Vec<AssistInfo>);
@@ -234,6 +235,11 @@ impl ActionBuilder {
     pub(crate) fn replace_ast<N: AstNode>(&mut self, old: N, new: N) {
         algo::diff(old.syntax(), new.syntax()).into_text_edit(&mut self.edit)
     }
+    pub(crate) fn rewrite(&mut self, rewriter: SyntaxRewriter) {
+        let node = rewriter.rewrite_root().unwrap();
+        let new = rewriter.rewrite(&node);
+        algo::diff(&node, &new).into_text_edit(&mut self.edit)
+    }
 
     fn build(self) -> AssistAction {
         AssistAction {
diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs
index 45558c448..52b4c82db 100644
--- a/crates/ra_assists/src/ast_transform.rs
+++ b/crates/ra_assists/src/ast_transform.rs
@@ -3,7 +3,10 @@ use rustc_hash::FxHashMap;
 
 use hir::{PathResolution, SemanticsScope};
 use ra_ide_db::RootDatabase;
-use ra_syntax::ast::{self, AstNode};
+use ra_syntax::{
+    algo::SyntaxRewriter,
+    ast::{self, AstNode},
+};
 
 pub trait AstTransform<'a> {
     fn get_substitution(&self, node: &ra_syntax::SyntaxNode) -> Option<ra_syntax::SyntaxNode>;
@@ -153,15 +156,14 @@ impl<'a> QualifyPaths<'a> {
 }
 
 pub fn apply<'a, N: AstNode>(transformer: &dyn AstTransform<'a>, node: N) -> N {
-    let syntax = node.syntax();
-    let result = ra_syntax::algo::replace_descendants(syntax, |element| match element {
+    SyntaxRewriter::from_fn(|element| match element {
         ra_syntax::SyntaxElement::Node(n) => {
             let replacement = transformer.get_substitution(&n)?;
             Some(replacement.into())
         }
         _ => None,
-    });
-    N::cast(result).unwrap()
+    })
+    .rewrite_ast(&node)
 }
 
 impl<'a> AstTransform<'a> for QualifyPaths<'a> {
diff --git a/crates/ra_assists/src/doc_tests/generated.rs b/crates/ra_assists/src/doc_tests/generated.rs
index aef6793e8..62dcb3808 100644
--- a/crates/ra_assists/src/doc_tests/generated.rs
+++ b/crates/ra_assists/src/doc_tests/generated.rs
@@ -275,8 +275,8 @@ enum Action { Move { distance: u32 }, Stop }
 
 fn handle(action: Action) {
     match action {
-        Action::Move { distance } => (),
-        Action::Stop => (),
+        Action::Move { distance } => {}
+        Action::Stop => {}
     }
 }
 "#####,
diff --git a/crates/ra_assists/src/handlers/add_missing_impl_members.rs b/crates/ra_assists/src/handlers/add_missing_impl_members.rs
index e5920b6f6..722f207e2 100644
--- a/crates/ra_assists/src/handlers/add_missing_impl_members.rs
+++ b/crates/ra_assists/src/handlers/add_missing_impl_members.rs
@@ -151,7 +151,7 @@ fn add_missing_impl_members_inner(
                 ast::ImplItem::FnDef(def) => ast::ImplItem::FnDef(add_body(def)),
                 _ => it,
             })
-            .map(|it| edit::strip_attrs_and_docs(&it));
+            .map(|it| edit::remove_attrs_and_docs(&it));
         let new_impl_item_list = impl_item_list.append_items(items);
         let cursor_position = {
             let first_new_item = new_impl_item_list.impl_items().nth(n_existing_items).unwrap();
diff --git a/crates/ra_assists/src/handlers/auto_import.rs b/crates/ra_assists/src/handlers/auto_import.rs
index bb280f633..99682e023 100644
--- a/crates/ra_assists/src/handlers/auto_import.rs
+++ b/crates/ra_assists/src/handlers/auto_import.rs
@@ -17,6 +17,7 @@ use crate::{
     utils::insert_use_statement,
     AssistId,
 };
+use either::Either;
 
 // Assist: auto_import
 //
@@ -58,6 +59,7 @@ pub(crate) fn auto_import(ctx: AssistCtx) -> Option<Assist> {
     group.finish()
 }
 
+#[derive(Debug)]
 struct AutoImportAssets {
     import_candidate: ImportCandidate,
     module_with_name_to_import: Module,
@@ -127,14 +129,14 @@ impl AutoImportAssets {
         ImportsLocator::new(db)
             .find_imports(&self.get_search_query())
             .into_iter()
-            .filter_map(|module_def| match &self.import_candidate {
+            .filter_map(|candidate| match &self.import_candidate {
                 ImportCandidate::TraitAssocItem(assoc_item_type, _) => {
-                    let located_assoc_item = match module_def {
-                        ModuleDef::Function(located_function) => located_function
+                    let located_assoc_item = match candidate {
+                        Either::Left(ModuleDef::Function(located_function)) => located_function
                             .as_assoc_item(db)
                             .map(|assoc| assoc.container(db))
                             .and_then(Self::assoc_to_trait),
-                        ModuleDef::Const(located_const) => located_const
+                        Either::Left(ModuleDef::Const(located_const)) => located_const
                             .as_assoc_item(db)
                             .map(|assoc| assoc.container(db))
                             .and_then(Self::assoc_to_trait),
@@ -153,10 +155,11 @@ impl AutoImportAssets {
                             |_, assoc| Self::assoc_to_trait(assoc.container(db)),
                         )
                         .map(ModuleDef::from)
+                        .map(Either::Left)
                 }
                 ImportCandidate::TraitMethod(function_callee, _) => {
                     let located_assoc_item =
-                        if let ModuleDef::Function(located_function) = module_def {
+                        if let Either::Left(ModuleDef::Function(located_function)) = candidate {
                             located_function
                                 .as_assoc_item(db)
                                 .map(|assoc| assoc.container(db))
@@ -179,10 +182,18 @@ impl AutoImportAssets {
                             },
                         )
                         .map(ModuleDef::from)
+                        .map(Either::Left)
+                }
+                _ => Some(candidate),
+            })
+            .filter_map(|candidate| match candidate {
+                Either::Left(module_def) => {
+                    self.module_with_name_to_import.find_use_path(db, module_def)
+                }
+                Either::Right(macro_def) => {
+                    self.module_with_name_to_import.find_use_path(db, macro_def)
                 }
-                _ => Some(module_def),
             })
-            .filter_map(|module_def| self.module_with_name_to_import.find_use_path(db, module_def))
             .filter(|use_path| !use_path.segments.is_empty())
             .take(20)
             .collect::<BTreeSet<_>>()
@@ -440,6 +451,30 @@ mod tests {
     }
 
     #[test]
+    fn macro_import() {
+        check_assist(
+            auto_import,
+            r"
+                    //- /lib.rs crate:crate_with_macro
+                    #[macro_export]
+                    macro_rules! foo {
+                        () => ()
+                    }
+
+                    //- /main.rs crate:main deps:crate_with_macro
+                    fn main() {
+                        foo<|>
+                    }",
+            r"use crate_with_macro::foo;
+
+fn main() {
+    foo<|>
+}
+",
+        );
+    }
+
+    #[test]
     fn auto_import_target() {
         check_assist_target(
             auto_import,
diff --git a/crates/ra_assists/src/handlers/fill_match_arms.rs b/crates/ra_assists/src/handlers/fill_match_arms.rs
index fbd6a3ec3..add82e5b1 100644
--- a/crates/ra_assists/src/handlers/fill_match_arms.rs
+++ b/crates/ra_assists/src/handlers/fill_match_arms.rs
@@ -2,7 +2,8 @@
 
 use std::iter;
 
-use hir::{Adt, HasSource, Semantics};
+use hir::{Adt, HasSource, ModuleDef, Semantics};
+use itertools::Itertools;
 use ra_ide_db::RootDatabase;
 
 use crate::{Assist, AssistCtx, AssistId};
@@ -29,8 +30,8 @@ use ast::{MatchArm, Pat};
 //
 // fn handle(action: Action) {
 //     match action {
-//         Action::Move { distance } => (),
-//         Action::Stop => (),
+//         Action::Move { distance } => {}
+//         Action::Stop => {}
 //     }
 // }
 // ```
@@ -39,13 +40,6 @@ pub(crate) fn fill_match_arms(ctx: AssistCtx) -> Option<Assist> {
     let match_arm_list = match_expr.match_arm_list()?;
 
     let expr = match_expr.expr()?;
-    let enum_def = resolve_enum_def(&ctx.sema, &expr)?;
-    let module = ctx.sema.scope(expr.syntax()).module()?;
-
-    let variants = enum_def.variants(ctx.db);
-    if variants.is_empty() {
-        return None;
-    }
 
     let mut arms: Vec<MatchArm> = match_arm_list.arms().collect();
     if arms.len() == 1 {
@@ -54,13 +48,49 @@ pub(crate) fn fill_match_arms(ctx: AssistCtx) -> Option<Assist> {
         }
     }
 
-    let db = ctx.db;
-    let missing_arms: Vec<MatchArm> = variants
-        .into_iter()
-        .filter_map(|variant| build_pat(db, module, variant))
-        .filter(|variant_pat| is_variant_missing(&mut arms, variant_pat))
-        .map(|pat| make::match_arm(iter::once(pat), make::expr_unit()))
-        .collect();
+    let module = ctx.sema.scope(expr.syntax()).module()?;
+
+    let missing_arms: Vec<MatchArm> = if let Some(enum_def) = resolve_enum_def(&ctx.sema, &expr) {
+        let variants = enum_def.variants(ctx.db);
+
+        variants
+            .into_iter()
+            .filter_map(|variant| build_pat(ctx.db, module, variant))
+            .filter(|variant_pat| is_variant_missing(&mut arms, variant_pat))
+            .map(|pat| make::match_arm(iter::once(pat), make::expr_empty_block()))
+            .collect()
+    } else if let Some(enum_defs) = resolve_tuple_of_enum_def(&ctx.sema, &expr) {
+        // Partial fill not currently supported for tuple of enums.
+        if !arms.is_empty() {
+            return None;
+        }
+
+        // We do not currently support filling match arms for a tuple
+        // containing a single enum.
+        if enum_defs.len() < 2 {
+            return None;
+        }
+
+        // When calculating the match arms for a tuple of enums, we want
+        // to create a match arm for each possible combination of enum
+        // values. The `multi_cartesian_product` method transforms
+        // Vec<Vec<EnumVariant>> into Vec<(EnumVariant, .., EnumVariant)>
+        // where each tuple represents a proposed match arm.
+        enum_defs
+            .into_iter()
+            .map(|enum_def| enum_def.variants(ctx.db))
+            .multi_cartesian_product()
+            .map(|variants| {
+                let patterns =
+                    variants.into_iter().filter_map(|variant| build_pat(ctx.db, module, variant));
+                ast::Pat::from(make::tuple_pat(patterns))
+            })
+            .filter(|variant_pat| is_variant_missing(&mut arms, variant_pat))
+            .map(|pat| make::match_arm(iter::once(pat), make::expr_empty_block()))
+            .collect()
+    } else {
+        return None;
+    };
 
     if missing_arms.is_empty() {
         return None;
@@ -104,8 +134,27 @@ fn resolve_enum_def(sema: &Semantics<RootDatabase>, expr: &ast::Expr) -> Option<
     })
 }
 
+fn resolve_tuple_of_enum_def(
+    sema: &Semantics<RootDatabase>,
+    expr: &ast::Expr,
+) -> Option<Vec<hir::Enum>> {
+    sema.type_of_expr(&expr)?
+        .tuple_fields(sema.db)
+        .iter()
+        .map(|ty| {
+            ty.autoderef(sema.db).find_map(|ty| match ty.as_adt() {
+                Some(Adt::Enum(e)) => Some(e),
+                // For now we only handle expansion for a tuple of enums. Here
+                // we map non-enum items to None and rely on `collect` to
+                // convert Vec<Option<hir::Enum>> into Option<Vec<hir::Enum>>.
+                _ => None,
+            })
+        })
+        .collect()
+}
+
 fn build_pat(db: &RootDatabase, module: hir::Module, var: hir::EnumVariant) -> Option<ast::Pat> {
-    let path = crate::ast_transform::path_to_ast(module.find_use_path(db, var.into())?);
+    let path = crate::ast_transform::path_to_ast(module.find_use_path(db, ModuleDef::from(var))?);
 
     // FIXME: use HIR for this; it doesn't currently expose struct vs. tuple vs. unit variants though
     let pat: ast::Pat = match var.source(db).value.kind() {
@@ -143,8 +192,23 @@ mod tests {
             fn main() {
                 match A::As<|> {
                     A::As,
-                    A::Bs{x,y:Some(_)} => (),
-                    A::Cs(_, Some(_)) => (),
+                    A::Bs{x,y:Some(_)} => {}
+                    A::Cs(_, Some(_)) => {}
+                }
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn tuple_of_non_enum() {
+        // for now this case is not handled, although it potentially could be
+        // in the future
+        check_assist_not_applicable(
+            fill_match_arms,
+            r#"
+            fn main() {
+                match (0, false)<|> {
                 }
             }
             "#,
@@ -163,8 +227,8 @@ mod tests {
             }
             fn main() {
                 match A::As<|> {
-                    A::Bs{x,y:Some(_)} => (),
-                    A::Cs(_, Some(_)) => (),
+                    A::Bs{x,y:Some(_)} => {}
+                    A::Cs(_, Some(_)) => {}
                 }
             }
             "#,
@@ -176,9 +240,9 @@ mod tests {
             }
             fn main() {
                 match <|>A::As {
-                    A::Bs{x,y:Some(_)} => (),
-                    A::Cs(_, Some(_)) => (),
-                    A::As => (),
+                    A::Bs{x,y:Some(_)} => {}
+                    A::Cs(_, Some(_)) => {}
+                    A::As => {}
                 }
             }
             "#,
@@ -197,7 +261,7 @@ mod tests {
             }
             fn main() {
                 match A::As<|> {
-                    A::Cs(_) | A::Bs => (),
+                    A::Cs(_) | A::Bs => {}
                 }
             }
             "#,
@@ -209,8 +273,8 @@ mod tests {
             }
             fn main() {
                 match <|>A::As {
-                    A::Cs(_) | A::Bs => (),
-                    A::As => (),
+                    A::Cs(_) | A::Bs => {}
+                    A::As => {}
                 }
             }
             "#,
@@ -235,8 +299,8 @@ mod tests {
             }
             fn main() {
                 match A::As<|> {
-                    A::Bs if 0 < 1 => (),
-                    A::Ds(_value) => (),
+                    A::Bs if 0 < 1 => {}
+                    A::Ds(_value) => { let x = 1; }
                     A::Es(B::Xs) => (),
                 }
             }
@@ -255,11 +319,11 @@ mod tests {
             }
             fn main() {
                 match <|>A::As {
-                    A::Bs if 0 < 1 => (),
-                    A::Ds(_value) => (),
+                    A::Bs if 0 < 1 => {}
+                    A::Ds(_value) => { let x = 1; }
                     A::Es(B::Xs) => (),
-                    A::As => (),
-                    A::Cs => (),
+                    A::As => {}
+                    A::Cs => {}
                 }
             }
             "#,
@@ -296,11 +360,174 @@ mod tests {
             fn main() {
                 let a = A::As;
                 match <|>a {
-                    A::As => (),
-                    A::Bs => (),
-                    A::Cs(_) => (),
-                    A::Ds(_, _) => (),
-                    A::Es { x, y } => (),
+                    A::As => {}
+                    A::Bs => {}
+                    A::Cs(_) => {}
+                    A::Ds(_, _) => {}
+                    A::Es { x, y } => {}
+                }
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn fill_match_arms_tuple_of_enum() {
+        check_assist(
+            fill_match_arms,
+            r#"
+            enum A {
+                One,
+                Two,
+            }
+            enum B {
+                One,
+                Two,
+            }
+
+            fn main() {
+                let a = A::One;
+                let b = B::One;
+                match (a<|>, b) {}
+            }
+            "#,
+            r#"
+            enum A {
+                One,
+                Two,
+            }
+            enum B {
+                One,
+                Two,
+            }
+
+            fn main() {
+                let a = A::One;
+                let b = B::One;
+                match <|>(a, b) {
+                    (A::One, B::One) => {}
+                    (A::One, B::Two) => {}
+                    (A::Two, B::One) => {}
+                    (A::Two, B::Two) => {}
+                }
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn fill_match_arms_tuple_of_enum_ref() {
+        check_assist(
+            fill_match_arms,
+            r#"
+            enum A {
+                One,
+                Two,
+            }
+            enum B {
+                One,
+                Two,
+            }
+
+            fn main() {
+                let a = A::One;
+                let b = B::One;
+                match (&a<|>, &b) {}
+            }
+            "#,
+            r#"
+            enum A {
+                One,
+                Two,
+            }
+            enum B {
+                One,
+                Two,
+            }
+
+            fn main() {
+                let a = A::One;
+                let b = B::One;
+                match <|>(&a, &b) {
+                    (A::One, B::One) => {}
+                    (A::One, B::Two) => {}
+                    (A::Two, B::One) => {}
+                    (A::Two, B::Two) => {}
+                }
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn fill_match_arms_tuple_of_enum_partial() {
+        check_assist_not_applicable(
+            fill_match_arms,
+            r#"
+            enum A {
+                One,
+                Two,
+            }
+            enum B {
+                One,
+                Two,
+            }
+
+            fn main() {
+                let a = A::One;
+                let b = B::One;
+                match (a<|>, b) {
+                    (A::Two, B::One) => {}
+                }
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn fill_match_arms_tuple_of_enum_not_applicable() {
+        check_assist_not_applicable(
+            fill_match_arms,
+            r#"
+            enum A {
+                One,
+                Two,
+            }
+            enum B {
+                One,
+                Two,
+            }
+
+            fn main() {
+                let a = A::One;
+                let b = B::One;
+                match (a<|>, b) {
+                    (A::Two, B::One) => {}
+                    (A::One, B::One) => {}
+                    (A::One, B::Two) => {}
+                    (A::Two, B::Two) => {}
+                }
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn fill_match_arms_single_element_tuple_of_enum() {
+        // For now we don't hande the case of a single element tuple, but
+        // we could handle this in the future if `make::tuple_pat` allowed
+        // creating a tuple with a single pattern.
+        check_assist_not_applicable(
+            fill_match_arms,
+            r#"
+            enum A {
+                One,
+                Two,
+            }
+
+            fn main() {
+                let a = A::One;
+                match (a<|>, ) {
                 }
             }
             "#,
@@ -328,7 +555,7 @@ mod tests {
 
             fn foo(a: &A) {
                 match <|>a {
-                    A::As => (),
+                    A::As => {}
                 }
             }
             "#,
@@ -353,7 +580,7 @@ mod tests {
 
             fn foo(a: &mut A) {
                 match <|>a {
-                    A::Es { x, y } => (),
+                    A::Es { x, y } => {}
                 }
             }
             "#,
@@ -384,7 +611,7 @@ mod tests {
 
             fn main() {
                 match E::X {
-                    <|>_ => {},
+                    <|>_ => {}
                 }
             }
             "#,
@@ -393,8 +620,8 @@ mod tests {
 
             fn main() {
                 match <|>E::X {
-                    E::X => (),
-                    E::Y => (),
+                    E::X => {}
+                    E::Y => {}
                 }
             }
             "#,
@@ -421,8 +648,8 @@ mod tests {
 
             fn main() {
                 match <|>X {
-                    X => (),
-                    foo::E::Y => (),
+                    X => {}
+                    foo::E::Y => {}
                 }
             }
             "#,
diff --git a/crates/ra_assists/src/handlers/invert_if.rs b/crates/ra_assists/src/handlers/invert_if.rs
index 3a2665d17..4c5716868 100644
--- a/crates/ra_assists/src/handlers/invert_if.rs
+++ b/crates/ra_assists/src/handlers/invert_if.rs
@@ -33,6 +33,11 @@ pub(crate) fn invert_if(ctx: AssistCtx) -> Option<Assist> {
         return None;
     }
 
+    // This assist should not apply for if-let.
+    if expr.condition()?.pat().is_some() {
+        return None;
+    }
+
     let cond = expr.condition()?.expr()?;
     let then_node = expr.then_branch()?.syntax().clone();
 
@@ -90,4 +95,12 @@ mod tests {
     fn invert_if_doesnt_apply_with_cursor_not_on_if() {
         check_assist_not_applicable(invert_if, "fn f() { if !<|>cond { 3 * 2 } else { 1 } }")
     }
+
+    #[test]
+    fn invert_if_doesnt_apply_with_if_let() {
+        check_assist_not_applicable(
+            invert_if,
+            "fn f() { i<|>f let Some(_) = Some(1) { 1 } else { 0 } }",
+        )
+    }
 }
diff --git a/crates/ra_assists/src/handlers/merge_imports.rs b/crates/ra_assists/src/handlers/merge_imports.rs
index 89bc975bd..9c57d1e30 100644
--- a/crates/ra_assists/src/handlers/merge_imports.rs
+++ b/crates/ra_assists/src/handlers/merge_imports.rs
@@ -1,9 +1,9 @@
 use std::iter::successors;
 
 use ra_syntax::{
-    algo::neighbor,
+    algo::{neighbor, SyntaxRewriter},
     ast::{self, edit::AstNodeEdit, make},
-    AstNode, AstToken, Direction, InsertPosition, SyntaxElement, TextRange, T,
+    AstNode, Direction, InsertPosition, SyntaxElement, T,
 };
 
 use crate::{Assist, AssistCtx, AssistId};
@@ -22,9 +22,10 @@ use crate::{Assist, AssistCtx, AssistId};
 // ```
 pub(crate) fn merge_imports(ctx: AssistCtx) -> Option<Assist> {
     let tree: ast::UseTree = ctx.find_node_at_offset()?;
-    let (new_tree, to_delete) = if let Some(use_item) =
-        tree.syntax().parent().and_then(ast::UseItem::cast)
-    {
+    let mut rewriter = SyntaxRewriter::default();
+    let mut offset = ctx.frange.range.start();
+
+    if let Some(use_item) = tree.syntax().parent().and_then(ast::UseItem::cast) {
         let (merged, to_delete) = next_prev()
             .filter_map(|dir| neighbor(&use_item, dir))
             .filter_map(|it| Some((it.clone(), it.use_tree()?)))
@@ -32,42 +33,28 @@ pub(crate) fn merge_imports(ctx: AssistCtx) -> Option<Assist> {
                 Some((try_merge_trees(&tree, &use_tree)?, use_item.clone()))
             })?;
 
-        let mut range = to_delete.syntax().text_range();
-        let next_ws = to_delete
-            .syntax()
-            .next_sibling_or_token()
-            .and_then(|it| it.into_token())
-            .and_then(ast::Whitespace::cast);
-        if let Some(ws) = next_ws {
-            range = range.extend_to(&ws.syntax().text_range())
+        rewriter.replace_ast(&tree, &merged);
+        rewriter += to_delete.remove();
+
+        if to_delete.syntax().text_range().end() < offset {
+            offset -= to_delete.syntax().text_range().len();
         }
-        (merged, range)
     } else {
         let (merged, to_delete) = next_prev()
             .filter_map(|dir| neighbor(&tree, dir))
             .find_map(|use_tree| Some((try_merge_trees(&tree, &use_tree)?, use_tree.clone())))?;
 
-        let mut range = to_delete.syntax().text_range();
-        if let Some((dir, nb)) = next_prev().find_map(|dir| Some((dir, neighbor(&to_delete, dir)?)))
-        {
-            let nb_range = nb.syntax().text_range();
-            if dir == Direction::Prev {
-                range = TextRange::from_to(nb_range.end(), range.end());
-            } else {
-                range = TextRange::from_to(range.start(), nb_range.start());
-            }
+        rewriter.replace_ast(&tree, &merged);
+        rewriter += to_delete.remove();
+
+        if to_delete.syntax().text_range().end() < offset {
+            offset -= to_delete.syntax().text_range().len();
         }
-        (merged, range)
     };
 
-    let mut offset = ctx.frange.range.start();
     ctx.add_assist(AssistId("merge_imports"), "Merge imports", |edit| {
-        edit.replace_ast(tree, new_tree);
-        edit.delete(to_delete);
-
-        if to_delete.end() <= offset {
-            offset -= to_delete.len();
-        }
+        edit.rewrite(rewriter);
+        // FIXME: we only need because our diff is imprecise
         edit.set_cursor(offset);
     })
 }
@@ -156,7 +143,7 @@ use std::fmt::Debug;
 use std::fmt<|>::Display;
 ",
             r"
-use std::fmt<|>::{Display, Debug};
+use std::fmt:<|>:{Display, Debug};
 ",
         );
     }
@@ -178,7 +165,57 @@ use std::{fmt<|>::{Debug, Display}};
 use std::{fmt::Debug, fmt<|>::Display};
 ",
             r"
-use std::{fmt<|>::{Display, Debug}};
+use std::{fmt::<|>{Display, Debug}};
+",
+        );
+    }
+
+    #[test]
+    fn removes_just_enough_whitespace() {
+        check_assist(
+            merge_imports,
+            r"
+use foo<|>::bar;
+use foo::baz;
+
+/// Doc comment
+",
+            r"
+use foo<|>::{bar, baz};
+
+/// Doc comment
+",
+        );
+    }
+
+    #[test]
+    fn works_with_trailing_comma() {
+        check_assist(
+            merge_imports,
+            r"
+use {
+    foo<|>::bar,
+    foo::baz,
+};
+",
+            r"
+use {
+    foo<|>::{bar, baz},
+};
+",
+        );
+        check_assist(
+            merge_imports,
+            r"
+use {
+    foo::baz,
+    foo<|>::bar,
+};
+",
+            r"
+use {
+    foo::{bar<|>, baz},
+};
 ",
         );
     }
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index b8704ea7d..bcc9b3f10 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -165,7 +165,6 @@ mod helpers {
 
     use ra_db::{fixture::WithFixture, FileId, FileRange, SourceDatabaseExt};
     use ra_ide_db::{symbol_index::SymbolsDatabase, RootDatabase};
-    use ra_syntax::TextRange;
     use test_utils::{add_cursor, assert_eq_text, extract_range_or_offset, RangeOrOffset};
 
     use crate::{AssistCtx, AssistHandler};
@@ -175,8 +174,7 @@ mod helpers {
         let (mut db, file_id) = RootDatabase::with_single_file(text);
         // FIXME: ideally, this should be done by the above `RootDatabase::with_single_file`,
         // but it looks like this might need specialization? :(
-        let local_roots = vec![db.file_source_root(file_id)];
-        db.set_local_roots(Arc::new(local_roots));
+        db.set_local_roots(Arc::new(vec![db.file_source_root(file_id)]));
         (db, file_id)
     }
 
@@ -206,11 +204,24 @@ mod helpers {
     }
 
     fn check(assist: AssistHandler, before: &str, expected: ExpectedResult) {
-        let (range_or_offset, before) = extract_range_or_offset(before);
-        let range: TextRange = range_or_offset.into();
+        let (text_without_caret, file_with_caret_id, range_or_offset, db) =
+            if before.contains("//-") {
+                let (mut db, position) = RootDatabase::with_position(before);
+                db.set_local_roots(Arc::new(vec![db.file_source_root(position.file_id)]));
+                (
+                    db.file_text(position.file_id).as_ref().to_owned(),
+                    position.file_id,
+                    RangeOrOffset::Offset(position.offset),
+                    db,
+                )
+            } else {
+                let (range_or_offset, text_without_caret) = extract_range_or_offset(before);
+                let (db, file_id) = with_single_file(&text_without_caret);
+                (text_without_caret, file_id, range_or_offset, db)
+            };
+
+        let frange = FileRange { file_id: file_with_caret_id, range: range_or_offset.into() };
 
-        let (db, file_id) = with_single_file(&before);
-        let frange = FileRange { file_id, range };
         let sema = Semantics::new(&db);
         let assist_ctx = AssistCtx::new(&sema, frange, true);
 
@@ -218,7 +229,7 @@ mod helpers {
             (Some(assist), ExpectedResult::After(after)) => {
                 let action = assist.0[0].action.clone().unwrap();
 
-                let mut actual = action.edit.apply(&before);
+                let mut actual = action.edit.apply(&text_without_caret);
                 match action.cursor_position {
                     None => {
                         if let RangeOrOffset::Offset(before_cursor_pos) = range_or_offset {
@@ -237,7 +248,7 @@ mod helpers {
             (Some(assist), ExpectedResult::Target(target)) => {
                 let action = assist.0[0].action.clone().unwrap();
                 let range = action.target.expect("expected target on action");
-                assert_eq_text!(&before[range], target);
+                assert_eq_text!(&text_without_caret[range], target);
             }
             (Some(_), ExpectedResult::NotApplicable) => panic!("assist should not be applicable!"),
             (None, ExpectedResult::After(_)) | (None, ExpectedResult::Target(_)) => {