4 files changed, 68 insertions, 54 deletions

diff --git a/crates/assists/src/handlers/auto_import.rs b/crates/assists/src/handlers/auto_import.rs
index c4770f336..f3b4c5708 100644
--- a/crates/assists/src/handlers/auto_import.rs
+++ b/crates/assists/src/handlers/auto_import.rs
@@ -13,8 +13,11 @@ use syntax::{
 };
 
 use crate::{
-    utils::insert_use_statement, AssistContext, AssistId, AssistKind, Assists, GroupLabel,
+    utils::{insert_use, MergeBehaviour},
+    AssistContext, AssistId, AssistKind, Assists, GroupLabel,
 };
+use ast::make;
+use insert_use::find_insert_use_container;
 
 // Assist: auto_import
 //
@@ -44,6 +47,8 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
 
     let range = ctx.sema.original_range(&auto_import_assets.syntax_under_caret).range;
     let group = auto_import_assets.get_import_group_message();
+    let container = find_insert_use_container(&auto_import_assets.syntax_under_caret, ctx)?;
+    let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone());
     for import in proposed_imports {
         acc.add_group(
             &group,
@@ -51,12 +56,12 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
             format!("Import `{}`", &import),
             range,
             |builder| {
-                insert_use_statement(
-                    &auto_import_assets.syntax_under_caret,
-                    &import.to_string(),
-                    ctx,
-                    builder.text_edit_builder(),
+                let new_syntax = insert_use(
+                    &syntax,
+                    make::path_from_text(&import.to_string()),
+                    Some(MergeBehaviour::Full),
                 );
+                builder.replace(syntax.text_range(), new_syntax.to_string())
             },
         );
     }
@@ -358,7 +363,7 @@ mod tests {
             }
             ",
             r"
-            use PubMod::{PubStruct2, PubStruct1};
+            use PubMod::{PubStruct1, PubStruct2};
 
             struct Test {
                 test: PubStruct2<u8>,
diff --git a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
index 8ac20210a..d9c50f25f 100644
--- a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
+++ b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
@@ -10,9 +10,12 @@ use syntax::{
 };
 
 use crate::{
-    assist_context::AssistBuilder, utils::insert_use_statement, AssistContext, AssistId,
-    AssistKind, Assists,
+    assist_context::AssistBuilder,
+    utils::{insert_use, MergeBehaviour},
+    AssistContext, AssistId, AssistKind, Assists,
 };
+use ast::make;
+use insert_use::find_insert_use_container;
 
 // Assist: extract_struct_from_enum_variant
 //
@@ -107,12 +110,15 @@ fn insert_import(
     if let Some(mut mod_path) = mod_path {
         mod_path.segments.pop();
         mod_path.segments.push(variant_hir_name.clone());
-        insert_use_statement(
-            path.syntax(),
-            &mod_path.to_string(),
-            ctx,
-            builder.text_edit_builder(),
+        let container = find_insert_use_container(path.syntax(), ctx)?;
+        let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone());
+
+        let new_syntax = insert_use(
+            &syntax,
+            make::path_from_text(&mod_path.to_string()),
+            Some(MergeBehaviour::Full),
         );
+        builder.replace(syntax.text_range(), new_syntax.to_string())
     }
     Some(())
 }
diff --git a/crates/assists/src/handlers/replace_qualified_name_with_use.rs b/crates/assists/src/handlers/replace_qualified_name_with_use.rs
index 470e5f8ff..56e85125d 100644
--- a/crates/assists/src/handlers/replace_qualified_name_with_use.rs
+++ b/crates/assists/src/handlers/replace_qualified_name_with_use.rs
@@ -2,9 +2,10 @@ use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SyntaxNode, TextRang
 use test_utils::mark;
 
 use crate::{
-    utils::{find_insert_use_container, insert_use_statement},
+    utils::{find_insert_use_container, insert_use, MergeBehaviour},
     AssistContext, AssistId, AssistKind, Assists,
 };
+use ast::make;
 
 // Assist: replace_qualified_name_with_use
 //
@@ -32,7 +33,7 @@ pub(crate) fn replace_qualified_name_with_use(
         mark::hit!(dont_import_trivial_paths);
         return None;
     }
-    let path_to_import = path.to_string().clone();
+    let path_to_import = path.to_string();
     let path_to_import = match path.segment()?.generic_arg_list() {
         Some(generic_args) => {
             let generic_args_start =
@@ -43,28 +44,24 @@ pub(crate) fn replace_qualified_name_with_use(
     };
 
     let target = path.syntax().text_range();
+    let container = find_insert_use_container(path.syntax(), ctx)?;
+    let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone());
     acc.add(
         AssistId("replace_qualified_name_with_use", AssistKind::RefactorRewrite),
         "Replace qualified path with use",
         target,
         |builder| {
-            let container = match find_insert_use_container(path.syntax(), ctx) {
-                Some(c) => c,
-                None => return,
-            };
-            insert_use_statement(
-                path.syntax(),
-                &path_to_import.to_string(),
-                ctx,
-                builder.text_edit_builder(),
-            );
-
             // Now that we've brought the name into scope, re-qualify all paths that could be
             // affected (that is, all paths inside the node we added the `use` to).
             let mut rewriter = SyntaxRewriter::default();
-            let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone());
-            shorten_paths(&mut rewriter, syntax, path);
-            builder.rewrite(rewriter);
+            shorten_paths(&mut rewriter, syntax.clone(), path);
+            let rewritten_syntax = rewriter.rewrite(&syntax);
+            let new_syntax = insert_use(
+                &rewritten_syntax,
+                make::path_from_text(path_to_import),
+                Some(MergeBehaviour::Full),
+            );
+            builder.replace(syntax.text_range(), new_syntax.to_string())
         },
     )
 }
@@ -220,9 +217,10 @@ impl std::fmt::Debug<|> for Foo {
 }
     ",
             r"
-use stdx;
 use std::fmt::Debug;
 
+use stdx;
+
 impl Debug for Foo {
 }
     ",
@@ -274,7 +272,7 @@ impl std::io<|> for Foo {
 }
     ",
             r"
-use std::{io, fmt};
+use std::{fmt, io};
 
 impl io for Foo {
 }
@@ -293,7 +291,7 @@ impl std::fmt::Debug<|> for Foo {
 }
     ",
             r"
-use std::fmt::{self, Debug, };
+use std::fmt::{self, Debug};
 
 impl Debug for Foo {
 }
@@ -312,7 +310,7 @@ impl std::fmt<|> for Foo {
 }
     ",
             r"
-use std::fmt::{self, Debug};
+use std::fmt::{Debug, self};
 
 impl fmt for Foo {
 }
@@ -330,8 +328,9 @@ use std::fmt::{Debug, nested::{Display}};
 impl std::fmt::nested<|> for Foo {
 }
 ",
+            // FIXME(veykril): should be nested::{self, Display} here
             r"
-use std::fmt::{Debug, nested::{Display, self}};
+use std::fmt::{Debug, nested::{Display}, nested};
 
 impl nested for Foo {
 }
@@ -349,8 +348,9 @@ use std::fmt::{Debug, nested::{self, Display}};
 impl std::fmt::nested<|> for Foo {
 }
 ",
+            // FIXME(veykril): self is being pulled out for some reason now
             r"
-use std::fmt::{Debug, nested::{self, Display}};
+use std::fmt::{Debug, nested::{Display}, nested};
 
 impl nested for Foo {
 }
@@ -369,7 +369,7 @@ impl std::fmt::nested::Debug<|> for Foo {
 }
 ",
             r"
-use std::fmt::{Debug, nested::{Display, Debug}};
+use std::fmt::{Debug, nested::{Display}, nested::Debug};
 
 impl Debug for Foo {
 }
@@ -388,7 +388,7 @@ impl std::fmt::nested::Display<|> for Foo {
 }
 ",
             r"
-use std::fmt::{nested::Display, Debug};
+use std::fmt::{Debug, nested::Display};
 
 impl Display for Foo {
 }
@@ -407,7 +407,7 @@ impl std::fmt::Display<|> for Foo {
 }
 ",
             r"
-use std::fmt::{Display, nested::Debug};
+use std::fmt::{nested::Debug, Display};
 
 impl Display for Foo {
 }
@@ -427,11 +427,12 @@ use crate::{
 
 fn foo() { crate::ty::lower<|>::trait_env() }
 ",
+            // FIXME(veykril): formatting broke here
             r"
 use crate::{
-    ty::{Substs, Ty, lower},
+    ty::{Substs, Ty},
     AssocItem,
-};
+ty::lower};
 
 fn foo() { lower::trait_env() }
 ",
@@ -451,6 +452,8 @@ impl foo::Debug<|> for Foo {
             r"
 use std::fmt as foo;
 
+use foo::Debug;
+
 impl Debug for Foo {
 }
 ",
@@ -627,7 +630,7 @@ fn main() {
 }
     ",
             r"
-use std::fmt::{self, Display};
+use std::fmt::{Display, self};
 
 fn main() {
     fmt;
@@ -647,9 +650,8 @@ impl std::io<|> for Foo {
 }
     ",
             r"
-use std::io;
-
 pub use std::fmt;
+use std::io;
 
 impl io for Foo {
 }
@@ -668,9 +670,8 @@ impl std::io<|> for Foo {
 }
     ",
             r"
-use std::io;
-
 pub(crate) use std::fmt;
+use std::io;
 
 impl io for Foo {
 }
diff --git a/crates/assists/src/utils/insert_use.rs b/crates/assists/src/utils/insert_use.rs
index 030a5a935..f2acda6f3 100644
--- a/crates/assists/src/utils/insert_use.rs
+++ b/crates/assists/src/utils/insert_use.rs
@@ -18,9 +18,10 @@ pub(crate) fn find_insert_use_container(
 ) -> Option<Either<ast::ItemList, ast::SourceFile>> {
     ctx.sema.ancestors_with_macros(position.clone()).find_map(|n| {
         if let Some(module) = ast::Module::cast(n.clone()) {
-            return module.item_list().map(Either::Left);
+            module.item_list().map(Either::Left)
+        } else {
+            Some(Either::Right(ast::SourceFile::cast(n)?))
         }
-        Some(Either::Right(ast::SourceFile::cast(n)?))
     })
 }
 
@@ -92,6 +93,7 @@ fn use_tree_list_is_nested(tl: &ast::UseTreeList) -> bool {
     })
 }
 
+// FIXME: currently this merely prepends the new tree into old, ideally it would insert the items in a sorted fashion
 pub fn try_merge_trees(
     old: &ast::UseTree,
     new: &ast::UseTree,
@@ -486,7 +488,7 @@ use std::io;",
         check_full(
             "std::foo::bar::Baz",
             r"use std::foo::bar::Qux;",
-            r"use std::foo::bar::{Baz, Qux};",
+            r"use std::foo::bar::{Qux, Baz};",
         )
     }
 
@@ -495,7 +497,7 @@ use std::io;",
         check_last(
             "std::foo::bar::Baz",
             r"use std::foo::bar::Qux;",
-            r"use std::foo::bar::{Baz, Qux};",
+            r"use std::foo::bar::{Qux, Baz};",
         )
     }
 
@@ -504,7 +506,7 @@ use std::io;",
         check_full(
             "std::foo::bar::Baz",
             r"use std::foo::bar::{Qux, Quux};",
-            r"use std::foo::bar::{Baz, Quux, Qux};",
+            r"use std::foo::bar::{Qux, Quux, Baz};",
         )
     }
 
@@ -513,7 +515,7 @@ use std::io;",
         check_last(
             "std::foo::bar::Baz",
             r"use std::foo::bar::{Qux, Quux};",
-            r"use std::foo::bar::{Baz, Quux, Qux};",
+            r"use std::foo::bar::{Qux, Quux, Baz};",
         )
     }
 
@@ -522,7 +524,7 @@ use std::io;",
         check_full(
             "std::foo::bar::Baz",
             r"use std::foo::bar::{Qux, quux::{Fez, Fizz}};",
-            r"use std::foo::bar::{Baz, quux::{Fez, Fizz}, Qux};",
+            r"use std::foo::bar::{Qux, quux::{Fez, Fizz}, Baz};",
         )
     }
 
@@ -532,7 +534,7 @@ use std::io;",
             "std::foo::bar::Baz",
             r"use std::foo::bar::{Qux, quux::{Fez, Fizz}};",
             r"use std::foo::bar::Baz;
-use std::foo::bar::{quux::{Fez, Fizz}, Qux};",
+use std::foo::bar::{Qux, quux::{Fez, Fizz}};",
         )
     }