7 files changed, 185 insertions, 86 deletions

diff --git a/crates/ide_assists/src/handlers/add_missing_impl_members.rs b/crates/ide_assists/src/handlers/add_missing_impl_members.rs
index 0148635f9..8225ae22c 100644
--- a/crates/ide_assists/src/handlers/add_missing_impl_members.rs
+++ b/crates/ide_assists/src/handlers/add_missing_impl_members.rs
@@ -64,7 +64,6 @@ pub(crate) fn add_missing_impl_members(acc: &mut Assists, ctx: &AssistContext) -
 // impl Trait for () {
 //     type X = ();
 //     fn foo(&self) {}$0
-//
 // }
 // ```
 // ->
@@ -195,6 +194,7 @@ impl Foo for S {
     fn baz(&self) {
         todo!()
     }
+
 }"#,
         );
     }
@@ -231,6 +231,7 @@ impl Foo for S {
     fn foo(&self) {
         ${0:todo!()}
     }
+
 }"#,
         );
     }
diff --git a/crates/ide_assists/src/handlers/auto_import.rs b/crates/ide_assists/src/handlers/auto_import.rs
index a454a2af3..506cc292c 100644
--- a/crates/ide_assists/src/handlers/auto_import.rs
+++ b/crates/ide_assists/src/handlers/auto_import.rs
@@ -969,4 +969,30 @@ mod bar {
 "#,
         );
     }
+
+    #[test]
+    fn uses_abs_path_with_extern_crate_clash() {
+        check_assist(
+            auto_import,
+            r#"
+//- /main.rs crate:main deps:foo
+mod foo {}
+
+const _: () = {
+    Foo$0
+};
+//- /foo.rs crate:foo
+pub struct Foo
+"#,
+            r#"
+use ::foo::Foo;
+
+mod foo {}
+
+const _: () = {
+    Foo
+};
+"#,
+        );
+    }
 }
diff --git a/crates/ide_assists/src/handlers/generate_new.rs b/crates/ide_assists/src/handlers/generate_new.rs
index 8ce5930b7..959a1f86c 100644
--- a/crates/ide_assists/src/handlers/generate_new.rs
+++ b/crates/ide_assists/src/handlers/generate_new.rs
@@ -1,4 +1,3 @@
-use ast::Adt;
 use itertools::Itertools;
 use stdx::format_to;
 use syntax::ast::{self, AstNode, NameOwner, StructKind, VisibilityOwner};
@@ -37,7 +36,7 @@ pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
     };
 
     // Return early if we've found an existing new fn
-    let impl_def = find_struct_impl(&ctx, &Adt::Struct(strukt.clone()), "new")?;
+    let impl_def = find_struct_impl(&ctx, &ast::Adt::Struct(strukt.clone()), "new")?;
 
     let target = strukt.syntax().text_range();
     acc.add(AssistId("generate_new", AssistKind::Generate), "Generate `new`", target, |builder| {
@@ -60,7 +59,7 @@ pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
         let start_offset = impl_def
             .and_then(|impl_def| find_impl_block_start(impl_def, &mut buf))
             .unwrap_or_else(|| {
-                buf = generate_impl_text(&Adt::Struct(strukt.clone()), &buf);
+                buf = generate_impl_text(&ast::Adt::Struct(strukt.clone()), &buf);
                 strukt.syntax().text_range().end()
             });
 
@@ -81,101 +80,132 @@ mod tests {
     use super::*;
 
     #[test]
-    #[rustfmt::skip]
     fn test_generate_new() {
-        // Check output of generation
         check_assist(
             generate_new,
-"struct Foo {$0}",
-"struct Foo {}
+            r#"
+struct Foo {$0}
+"#,
+            r#"
+struct Foo {}
 
 impl Foo {
     fn $0new() -> Self { Self {  } }
-}",
+}
+"#,
         );
         check_assist(
             generate_new,
-"struct Foo<T: Clone> {$0}",
-"struct Foo<T: Clone> {}
+            r#"
+struct Foo<T: Clone> {$0}
+"#,
+            r#"
+struct Foo<T: Clone> {}
 
 impl<T: Clone> Foo<T> {
     fn $0new() -> Self { Self {  } }
-}",
+}
+"#,
         );
         check_assist(
             generate_new,
-"struct Foo<'a, T: Foo<'a>> {$0}",
-"struct Foo<'a, T: Foo<'a>> {}
+            r#"
+struct Foo<'a, T: Foo<'a>> {$0}
+"#,
+            r#"
+struct Foo<'a, T: Foo<'a>> {}
 
 impl<'a, T: Foo<'a>> Foo<'a, T> {
     fn $0new() -> Self { Self {  } }
-}",
+}
+"#,
         );
         check_assist(
             generate_new,
-"struct Foo { baz: String $0}",
-"struct Foo { baz: String }
+            r#"
+struct Foo { baz: String $0}
+"#,
+            r#"
+struct Foo { baz: String }
 
 impl Foo {
     fn $0new(baz: String) -> Self { Self { baz } }
-}",
+}
+"#,
         );
         check_assist(
             generate_new,
-"struct Foo { baz: String, qux: Vec<i32> $0}",
-"struct Foo { baz: String, qux: Vec<i32> }
+            r#"
+struct Foo { baz: String, qux: Vec<i32> $0}
+"#,
+            r#"
+struct Foo { baz: String, qux: Vec<i32> }
 
 impl Foo {
     fn $0new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }
-}",
+}
+"#,
         );
+    }
 
-        // Check that visibility modifiers don't get brought in for fields
+    #[test]
+    fn check_that_visibility_modifiers_dont_get_brought_in() {
         check_assist(
             generate_new,
-"struct Foo { pub baz: String, pub qux: Vec<i32> $0}",
-"struct Foo { pub baz: String, pub qux: Vec<i32> }
+            r#"
+struct Foo { pub baz: String, pub qux: Vec<i32> $0}
+"#,
+            r#"
+struct Foo { pub baz: String, pub qux: Vec<i32> }
 
 impl Foo {
     fn $0new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }
-}",
+}
+"#,
         );
+    }
 
-        // Check that it reuses existing impls
+    #[test]
+    fn check_it_reuses_existing_impls() {
         check_assist(
             generate_new,
-"struct Foo {$0}
+            r#"
+struct Foo {$0}
 
 impl Foo {}
-",
-"struct Foo {}
+"#,
+            r#"
+struct Foo {}
 
 impl Foo {
     fn $0new() -> Self { Self {  } }
 }
-",
+"#,
         );
         check_assist(
             generate_new,
-"struct Foo {$0}
+            r#"
+struct Foo {$0}
 
 impl Foo {
     fn qux(&self) {}
 }
-",
-"struct Foo {}
+"#,
+            r#"
+struct Foo {}
 
 impl Foo {
     fn $0new() -> Self { Self {  } }
 
     fn qux(&self) {}
 }
-",
+"#,
         );
 
         check_assist(
             generate_new,
-"struct Foo {$0}
+            r#"
+struct Foo {$0}
 
 impl Foo {
     fn qux(&self) {}
@@ -183,8 +213,9 @@ impl Foo {
         5
     }
 }
-",
-"struct Foo {}
+"#,
+            r#"
+struct Foo {}
 
 impl Foo {
     fn $0new() -> Self { Self {  } }
@@ -194,27 +225,37 @@ impl Foo {
         5
     }
 }
-",
+"#,
         );
+    }
 
-        // Check visibility of new fn based on struct
+    #[test]
+    fn check_visibility_of_new_fn_based_on_struct() {
         check_assist(
             generate_new,
-"pub struct Foo {$0}",
-"pub struct Foo {}
+            r#"
+pub struct Foo {$0}
+"#,
+            r#"
+pub struct Foo {}
 
 impl Foo {
     pub fn $0new() -> Self { Self {  } }
-}",
+}
+"#,
         );
         check_assist(
             generate_new,
-"pub(crate) struct Foo {$0}",
-"pub(crate) struct Foo {}
+            r#"
+pub(crate) struct Foo {$0}
+"#,
+            r#"
+pub(crate) struct Foo {}
 
 impl Foo {
     pub(crate) fn $0new() -> Self { Self {  } }
-}",
+}
+"#,
         );
     }
 
@@ -222,26 +263,28 @@ impl Foo {
     fn generate_new_not_applicable_if_fn_exists() {
         check_assist_not_applicable(
             generate_new,
-            "
+            r#"
 struct Foo {$0}
 
 impl Foo {
     fn new() -> Self {
         Self
     }
-}",
+}
+"#,
         );
 
         check_assist_not_applicable(
             generate_new,
-            "
+            r#"
 struct Foo {$0}
 
 impl Foo {
     fn New() -> Self {
         Self
     }
-}",
+}
+"#,
         );
     }
 
@@ -249,12 +292,12 @@ impl Foo {
     fn generate_new_target() {
         check_assist_target(
             generate_new,
-            "
+            r#"
 struct SomeThingIrrelevant;
 /// Has a lifetime parameter
 struct Foo<'a, T: Foo<'a>> {$0}
 struct EvenMoreIrrelevant;
-",
+"#,
             "/// Has a lifetime parameter
 struct Foo<'a, T: Foo<'a>> {}",
         );
@@ -264,7 +307,7 @@ struct Foo<'a, T: Foo<'a>> {}",
     fn test_unrelated_new() {
         check_assist(
             generate_new,
-            r##"
+            r#"
 pub struct AstId<N: AstNode> {
     file_id: HirFileId,
     file_ast_id: FileAstId<N>,
@@ -285,8 +328,9 @@ impl<T> Source<T> {
     pub fn map<F: FnOnce(T) -> U, U>(self, f: F) -> Source<U> {
         Source { file_id: self.file_id, ast: f(self.ast) }
     }
-}"##,
-            r##"
+}
+"#,
+            r#"
 pub struct AstId<N: AstNode> {
     file_id: HirFileId,
     file_ast_id: FileAstId<N>,
@@ -309,7 +353,8 @@ impl<T> Source<T> {
     pub fn map<F: FnOnce(T) -> U, U>(self, f: F) -> Source<U> {
         Source { file_id: self.file_id, ast: f(self.ast) }
     }
-}"##,
+}
+"#,
         );
     }
 }
diff --git a/crates/ide_assists/src/handlers/move_module_to_file.rs b/crates/ide_assists/src/handlers/move_module_to_file.rs
index 6e685b4b2..93f702c55 100644
--- a/crates/ide_assists/src/handlers/move_module_to_file.rs
+++ b/crates/ide_assists/src/handlers/move_module_to_file.rs
@@ -1,4 +1,4 @@
-use ast::{edit::IndentLevel, VisibilityOwner};
+use ast::edit::IndentLevel;
 use ide_db::base_db::AnchoredPathBuf;
 use stdx::format_to;
 use syntax::{
@@ -60,12 +60,18 @@ pub(crate) fn move_module_to_file(acc: &mut Assists, ctx: &AssistContext) -> Opt
             };
 
             let mut buf = String::new();
-            if let Some(v) = module_ast.visibility() {
-                format_to!(buf, "{} ", v);
-            }
             format_to!(buf, "mod {};", module_name);
 
-            builder.replace(module_ast.syntax().text_range(), buf);
+            let replacement_start = if let Some(mod_token) = module_ast.mod_token() {
+                mod_token.text_range().start()
+            } else {
+                module_ast.syntax().text_range().start()
+            };
+
+            builder.replace(
+                TextRange::new(replacement_start, module_ast.syntax().text_range().end()),
+                buf,
+            );
 
             let dst = AnchoredPathBuf { anchor: ctx.frange.file_id, path };
             builder.create_file(dst, contents);
@@ -184,4 +190,26 @@ pub(crate) mod tests;
         cov_mark::check!(available_before_curly);
         check_assist_not_applicable(move_module_to_file, r#"mod m { $0 }"#);
     }
+
+    #[test]
+    fn keep_outer_comments_and_attributes() {
+        check_assist(
+            move_module_to_file,
+            r#"
+/// doc comment
+#[attribute]
+mod $0tests {
+    #[test] fn t() {}
+}
+"#,
+            r#"
+//- /main.rs
+/// doc comment
+#[attribute]
+mod tests;
+//- /tests.rs
+#[test] fn t() {}
+"#,
+        );
+    }
 }
diff --git a/crates/ide_assists/src/handlers/replace_unwrap_with_match.rs b/crates/ide_assists/src/handlers/replace_unwrap_with_match.rs
index 0fec961b4..a3bfa221c 100644
--- a/crates/ide_assists/src/handlers/replace_unwrap_with_match.rs
+++ b/crates/ide_assists/src/handlers/replace_unwrap_with_match.rs
@@ -17,7 +17,7 @@ use ide_db::ty_filter::TryEnum;
 
 // Assist: replace_unwrap_with_match
 //
-// Replaces `unwrap` a `match` expression. Works for Result and Option.
+// Replaces `unwrap` with a `match` expression. Works for Result and Option.
 //
 // ```
 // enum Result<T, E> { Ok(T), Err(E) }
diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs
index 49c1a9776..4406406a2 100644
--- a/crates/ide_assists/src/tests/generated.rs
+++ b/crates/ide_assists/src/tests/generated.rs
@@ -50,7 +50,6 @@ trait Trait {
 impl Trait for () {
     type X = ();
     fn foo(&self) {}$0
-
 }
 "#####,
         r#####"
diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs
index 0dcf20c61..fc7caee04 100644
--- a/crates/ide_assists/src/utils.rs
+++ b/crates/ide_assists/src/utils.rs
@@ -17,7 +17,7 @@ use syntax::{
     ast::AttrsOwner,
     ast::NameOwner,
     ast::{self, edit, make, ArgListOwner, GenericParamsOwner},
-    AstNode, Direction, SmolStr,
+    ted, AstNode, Direction, SmolStr,
     SyntaxKind::*,
     SyntaxNode, TextSize, T,
 };
@@ -128,43 +128,43 @@ pub fn add_trait_assoc_items_to_impl(
     sema: &hir::Semantics<ide_db::RootDatabase>,
     items: Vec<ast::AssocItem>,
     trait_: hir::Trait,
-    impl_def: ast::Impl,
+    impl_: ast::Impl,
     target_scope: hir::SemanticsScope,
 ) -> (ast::Impl, ast::AssocItem) {
-    let impl_item_list = impl_def.assoc_item_list().unwrap_or_else(make::assoc_item_list);
-
-    let n_existing_items = impl_item_list.assoc_items().count();
     let source_scope = sema.scope_for_def(trait_);
     let ast_transform = QualifyPaths::new(&target_scope, &source_scope)
-        .or(SubstituteTypeParams::for_trait_impl(&source_scope, trait_, impl_def.clone()));
+        .or(SubstituteTypeParams::for_trait_impl(&source_scope, trait_, impl_.clone()));
 
     let items = items
         .into_iter()
         .map(|it| it.clone_for_update())
         .inspect(|it| ast_transform::apply(&*ast_transform, it))
-        .map(|it| match it {
-            ast::AssocItem::Fn(def) => ast::AssocItem::Fn(add_body(def)),
-            ast::AssocItem::TypeAlias(def) => ast::AssocItem::TypeAlias(def.remove_bounds()),
-            _ => it,
-        })
-        .map(|it| edit::remove_attrs_and_docs(&it));
-
-    let new_impl_item_list = impl_item_list.append_items(items);
-    let new_impl_def = impl_def.with_assoc_item_list(new_impl_item_list);
-    let first_new_item =
-        new_impl_def.assoc_item_list().unwrap().assoc_items().nth(n_existing_items).unwrap();
-    return (new_impl_def, first_new_item);
-
-    fn add_body(fn_def: ast::Fn) -> ast::Fn {
-        match fn_def.body() {
-            Some(_) => fn_def,
-            None => {
+        .map(|it| edit::remove_attrs_and_docs(&it).clone_subtree().clone_for_update());
+
+    let res = impl_.clone_for_update();
+
+    let assoc_item_list = res.get_or_create_assoc_item_list();
+    let mut first_item = None;
+    for item in items {
+        first_item.get_or_insert_with(|| item.clone());
+        match &item {
+            ast::AssocItem::Fn(fn_) if fn_.body().is_none() => {
                 let body = make::block_expr(None, Some(make::ext::expr_todo()))
                     .indent(edit::IndentLevel(1));
-                fn_def.with_body(body)
+                ted::replace(fn_.get_or_create_body().syntax(), body.clone_for_update().syntax())
             }
+            ast::AssocItem::TypeAlias(type_alias) => {
+                if let Some(type_bound_list) = type_alias.type_bound_list() {
+                    type_bound_list.remove()
+                }
+            }
+            _ => {}
         }
+
+        assoc_item_list.add_item(item)
     }
+
+    (res, first_item.unwrap())
 }
 
 #[derive(Clone, Copy, Debug)]