Migrate add_missing_impl_members to mutable ast

`replace_derive_with_manual_impl` was slightly since it used
`add_trait_assoc_items_to_impl`
(which was also used by `add_missing_impl_members`)

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

@@ -4,10 +4,7 @@
 
 use crate::{
     assist_context::{AssistContext, Assists},
-    utils::{
-        add_trait_assoc_items_to_impl, filter_assoc_items, gen_trait_fn_body, render_snippet,
-        Cursor, DefaultMethods,
-    },
+    utils::{add_trait_assoc_items_to_impl, filter_assoc_items, gen_trait_fn_body, DefaultMethods},
     AssistId, AssistKind,
 };
 
@@ -130,7 +127,8 @@ fn add_missing_impl_members_inner(
     }
 
     let target = impl_def.syntax().text_range();
-    acc.add(AssistId(assist_id, AssistKind::QuickFix), label, target, |builder| {
+    acc.add(AssistId(assist_id, AssistKind::QuickFix), label, target, |edit| {
+        let new_impl_def = edit.make_mut(impl_def.clone());
         let missing_items = missing_items
             .into_iter()
             .map(|it| {
@@ -142,38 +140,34 @@ fn add_missing_impl_members_inner(
                 it.clone_for_update()
             })
             .collect();
-        let (new_impl_def, first_new_item) = add_trait_assoc_items_to_impl(
+        let first_new_item = add_trait_assoc_items_to_impl(
             &ctx.sema,
             missing_items,
             trait_,
-            impl_def.clone(),
+            &new_impl_def,
             target_scope,
         );
-        match ctx.config.snippet_cap {
-            None => builder.replace(target, new_impl_def.to_string()),
-            Some(cap) => {
-                let mut cursor = Cursor::Before(first_new_item.syntax());
-                let placeholder;
-                if let DefaultMethods::No = mode {
-                    if let ast::AssocItem::Fn(func) = &first_new_item {
-                        if try_gen_trait_body(ctx, func, trait_ref, &impl_def).is_none() {
-                            if let Some(m) =
-                                func.syntax().descendants().find_map(ast::MacroCall::cast)
-                            {
-                                if m.syntax().text() == "todo!()" {
-                                    placeholder = m;
-                                    cursor = Cursor::Replace(placeholder.syntax());
-                                }
+
+        if let Some(cap) = ctx.config.snippet_cap {
+            let mut placeholder = None;
+            if let DefaultMethods::No = mode {
+                if let ast::AssocItem::Fn(func) = &first_new_item {
+                    if try_gen_trait_body(ctx, func, trait_ref, &impl_def).is_none() {
+                        if let Some(m) = func.syntax().descendants().find_map(ast::MacroCall::cast)
+                        {
+                            if m.syntax().text() == "todo!()" {
+                                placeholder = Some(m);
                             }
                         }
                     }
                 }
-                builder.replace_snippet(
-                    cap,
-                    target,
-                    render_snippet(cap, new_impl_def.syntax(), cursor),
-                )
             }
+
+            if let Some(macro_call) = placeholder {
+                edit.add_placeholder_snippet(cap, macro_call);
+            } else {
+                edit.add_tabstop_before(cap, first_new_item);
+            };
         };
     })
 }

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

@@ -182,7 +182,11 @@ fn impl_def_from_trait(
     let impl_def = {
         use syntax::ast::Impl;
         let text = generate_trait_impl_text(adt, trait_path.to_string().as_str(), "");
-        let parse = syntax::SourceFile::parse(&text);
+        // FIXME: `generate_trait_impl_text` currently generates two newlines
+        // at the front, but these leading newlines should really instead be
+        // inserted at the same time the impl is inserted
+        assert_eq!(&text[..2], "\n\n", "`generate_trait_impl_text` output changed");
+        let parse = syntax::SourceFile::parse(&text[2..]);
         let node = match parse.tree().syntax().descendants().find_map(Impl::cast) {
             Some(it) => it,
             None => {
@@ -193,7 +197,7 @@ fn impl_def_from_trait(
                 )
             }
         };
-        let node = node.clone_subtree();
+        let node = node.clone_for_update();
         assert_eq!(node.syntax().text_range().start(), 0.into());
         node
     };
@@ -209,8 +213,8 @@ fn impl_def_from_trait(
             it.clone_for_update()
         })
         .collect();
-    let (impl_def, first_assoc_item) =
-        add_trait_assoc_items_to_impl(sema, trait_items, trait_, impl_def, target_scope);
+    let first_assoc_item =
+        add_trait_assoc_items_to_impl(sema, trait_items, trait_, &impl_def, target_scope);
 
     // Generate a default `impl` function body for the derived trait.
     if let ast::AssocItem::Fn(ref func) = first_assoc_item {

crates/ide-assists/src/utils.rs

@@ -132,9 +132,9 @@ pub fn add_trait_assoc_items_to_impl(
     sema: &Semantics<'_, RootDatabase>,
     items: Vec<ast::AssocItem>,
     trait_: hir::Trait,
-    impl_: ast::Impl,
+    impl_: &ast::Impl,
     target_scope: hir::SemanticsScope<'_>,
-) -> (ast::Impl, ast::AssocItem) {
+) -> ast::AssocItem {
     let source_scope = sema.scope_for_def(trait_);
 
     let transform = PathTransform::trait_impl(&target_scope, &source_scope, trait_, impl_.clone());
@@ -147,9 +147,7 @@ pub fn add_trait_assoc_items_to_impl(
         assoc_item
     });
 
-    let res = impl_.clone_for_update();
-
-    let assoc_item_list = res.get_or_create_assoc_item_list();
+    let assoc_item_list = impl_.get_or_create_assoc_item_list();
     let mut first_item = None;
     for item in items {
         first_item.get_or_insert_with(|| item.clone());
@@ -172,7 +170,7 @@ pub fn add_trait_assoc_items_to_impl(
         assoc_item_list.add_item(item)
     }
 
-    (res, first_item.unwrap())
+    first_item.unwrap()
 }
 
 #[derive(Clone, Copy, Debug)]