Auto merge of rust-lang#14979 - DropDemBits:structure-snippets-migrat…

…e-1, r=Veykril

internal: Migrate some assists to use the structured snippet API

Migrates the following assists:

- `add_missing_impl_members`
- `extract_type_alias`

As an additional requirement, these assists are also migrated to use the mutable AST API, since otherwise there would be overlapping `Indel` spans

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

@@ -4,10 +4,7 @@ use syntax::ast::{self, make, AstNode};
 
 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/extract_type_alias.rs

@@ -1,6 +1,9 @@
 use either::Either;
 use ide_db::syntax_helpers::node_ext::walk_ty;
-use syntax::ast::{self, edit::IndentLevel, make, AstNode, HasGenericParams, HasName};
+use syntax::{
+    ast::{self, edit::IndentLevel, make, AstNode, HasGenericParams, HasName},
+    ted,
+};
 
 use crate::{AssistContext, AssistId, AssistKind, Assists};
 
@@ -34,14 +37,16 @@ pub(crate) fn extract_type_alias(acc: &mut Assists, ctx: &AssistContext<'_>) ->
         || item.syntax(),
         |impl_| impl_.as_ref().either(AstNode::syntax, AstNode::syntax),
     );
-    let insert_pos = node.text_range().start();
     let target = ty.syntax().text_range();
 
     acc.add(
         AssistId("extract_type_alias", AssistKind::RefactorExtract),
         "Extract type as type alias",
         target,
-        |builder| {
+        |edit| {
+            let node = edit.make_syntax_mut(node.clone());
+            let target_ty = edit.make_mut(ty.clone());
+
             let mut known_generics = match item.generic_param_list() {
                 Some(it) => it.generic_params().collect(),
                 None => Vec::new(),
@@ -56,27 +61,29 @@ pub(crate) fn extract_type_alias(acc: &mut Assists, ctx: &AssistContext<'_>) ->
             let generic_params =
                 generics.map(|it| make::generic_param_list(it.into_iter().cloned()));
 
+            // Replace original type with the alias
             let ty_args = generic_params
                 .as_ref()
                 .map_or(String::new(), |it| it.to_generic_args().to_string());
-            let replacement = format!("Type{ty_args}");
-            builder.replace(target, replacement);
-
-            let indent = IndentLevel::from_node(node);
-            let generic_params = generic_params.map_or(String::new(), |it| it.to_string());
-            match ctx.config.snippet_cap {
-                Some(cap) => {
-                    builder.insert_snippet(
-                        cap,
-                        insert_pos,
-                        format!("type $0Type{generic_params} = {ty};\n\n{indent}"),
-                    );
-                }
-                None => {
-                    builder.insert(
-                        insert_pos,
-                        format!("type Type{generic_params} = {ty};\n\n{indent}"),
-                    );
+            // FIXME: replace with a `ast::make` constructor
+            let new_ty = make::ty(&format!("Type{ty_args}")).clone_for_update();
+            ted::replace(target_ty.syntax(), new_ty.syntax());
+
+            // Insert new alias
+            let indent = IndentLevel::from_node(&node);
+            let ty_alias = make::ty_alias("Type", generic_params, None, None, Some((ty, None)))
+                .clone_for_update();
+            ted::insert_all(
+                ted::Position::before(node),
+                vec![
+                    ty_alias.syntax().clone().into(),
+                    make::tokens::whitespace(&format!("\n\n{indent}")).into(),
+                ],
+            );
+
+            if let Some(cap) = ctx.config.snippet_cap {
+                if let Some(name) = ty_alias.name() {
+                    edit.add_tabstop_before(cap, name);
                 }
             }
         },

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)]

crates/syntax/src/ast/make.rs

@@ -166,7 +166,7 @@ pub fn ty_alias(
     assignment: Option<(ast::Type, Option<ast::WhereClause>)>,
 ) -> ast::TypeAlias {
     let mut s = String::new();
-    s.push_str(&format!("type {} ", ident));
+    s.push_str(&format!("type {}", ident));
 
     if let Some(list) = generic_param_list {
         s.push_str(&list.to_string());
@@ -182,9 +182,9 @@ pub fn ty_alias(
 
     if let Some(exp) = assignment {
         if let Some(cl) = exp.1 {
-            s.push_str(&format!("= {} {}", &exp.0.to_string(), &cl.to_string()));
+            s.push_str(&format!(" = {} {}", &exp.0.to_string(), &cl.to_string()));
         } else {
-            s.push_str(&format!("= {}", &exp.0.to_string()));
+            s.push_str(&format!(" = {}", &exp.0.to_string()));
         }
     }