Auto merge of #16467 - DropDemBits:structured-snippet-migrate-6, r=DropDemBits

internal: Migrate assists to the structured snippet API, part 6/7

Continuing from #16082

Migrates the following assists:

- `extract_function`
- `generate_getter_or_setter`
- `generate_impl`
- `generate_new`
- `replace_derive_with_manual_impl`

Would've been the final PR in the structured snippet migration series, but I didn't notice that `generate_trait_from_impl` started to use `{insert,replace}_snippet` when I first started the migration 😅. This appears to be a pretty self-contained change, so I'll leave that for a separate future PR.

This also removes the last usages of `render_snippet`, which was a follow up goal of #11638. 🎉

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

@@ -1,4 +1,4 @@
-use std::iter;
+use std::{iter, ops::RangeInclusive};
 
 use ast::make;
 use either::Either;
@@ -12,27 +12,25 @@ use ide_db::{
     helpers::mod_path_to_ast,
     imports::insert_use::{insert_use, ImportScope},
     search::{FileReference, ReferenceCategory, SearchScope},
+    source_change::SourceChangeBuilder,
     syntax_helpers::node_ext::{
         for_each_tail_expr, preorder_expr, walk_expr, walk_pat, walk_patterns_in_expr,
     },
     FxIndexSet, RootDatabase,
 };
-use itertools::Itertools;
-use stdx::format_to;
 use syntax::{
     ast::{
-        self,
-        edit::{AstNodeEdit, IndentLevel},
-        AstNode, HasGenericParams,
+        self, edit::IndentLevel, edit_in_place::Indent, AstNode, AstToken, HasGenericParams,
+        HasName,
     },
-    match_ast, ted, AstToken, SyntaxElement,
+    match_ast, ted, SyntaxElement,
     SyntaxKind::{self, COMMENT},
     SyntaxNode, SyntaxToken, TextRange, TextSize, TokenAtOffset, WalkEvent, T,
 };
 
 use crate::{
     assist_context::{AssistContext, Assists, TreeMutator},
-    utils::generate_impl_text,
+    utils::generate_impl,
     AssistId,
 };
 
@@ -134,17 +132,65 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op
             let new_indent = IndentLevel::from_node(&insert_after);
             let old_indent = fun.body.indent_level();
 
-            builder.replace(target_range, make_call(ctx, &fun, old_indent));
+            let insert_after = builder.make_syntax_mut(insert_after);
+
+            let call_expr = make_call(ctx, &fun, old_indent);
+
+            // Map the element range to replace into the mutable version
+            let elements = match &fun.body {
+                FunctionBody::Expr(expr) => {
+                    // expr itself becomes the replacement target
+                    let expr = &builder.make_mut(expr.clone());
+                    let node = SyntaxElement::Node(expr.syntax().clone());
+
+                    node.clone()..=node
+                }
+                FunctionBody::Span { parent, elements, .. } => {
+                    // Map the element range into the mutable versions
+                    let parent = builder.make_mut(parent.clone());
+
+                    let start = parent
+                        .syntax()
+                        .children_with_tokens()
+                        .nth(elements.start().index())
+                        .expect("should be able to find mutable start element");
+
+                    let end = parent
+                        .syntax()
+                        .children_with_tokens()
+                        .nth(elements.end().index())
+                        .expect("should be able to find mutable end element");
+
+                    start..=end
+                }
+            };
 
             let has_impl_wrapper =
                 insert_after.ancestors().any(|a| a.kind() == SyntaxKind::IMPL && a != insert_after);
 
+            let fn_def = format_function(ctx, module, &fun, old_indent).clone_for_update();
+
+            if let Some(cap) = ctx.config.snippet_cap {
+                if let Some(name) = fn_def.name() {
+                    builder.add_tabstop_before(cap, name);
+                }
+            }
+
             let fn_def = match fun.self_param_adt(ctx) {
                 Some(adt) if anchor == Anchor::Method && !has_impl_wrapper => {
-                    let fn_def = format_function(ctx, module, &fun, old_indent, new_indent + 1);
-                    generate_impl_text(&adt, &fn_def).replace("{\n\n", "{")
+                    fn_def.indent(1.into());
+
+                    let impl_ = generate_impl(&adt);
+                    impl_.indent(new_indent);
+                    impl_.get_or_create_assoc_item_list().add_item(fn_def.into());
+
+                    impl_.syntax().clone()
+                }
+                _ => {
+                    fn_def.indent(new_indent);
+
+                    fn_def.syntax().clone()
                 }
-                _ => format_function(ctx, module, &fun, old_indent, new_indent),
             };
 
             // There are external control flows
@@ -177,12 +223,15 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op
                 }
             }
 
-            let insert_offset = insert_after.text_range().end();
+            // Replace the call site with the call to the new function
+            fixup_call_site(builder, &fun.body);
+            ted::replace_all(elements, vec![call_expr.into()]);
 
-            match ctx.config.snippet_cap {
-                Some(cap) => builder.insert_snippet(cap, insert_offset, fn_def),
-                None => builder.insert(insert_offset, fn_def),
-            };
+            // Insert the newly extracted function (or impl)
+            ted::insert_all_raw(
+                ted::Position::after(insert_after),
+                vec![make::tokens::whitespace(&format!("\n\n{new_indent}")).into(), fn_def.into()],
+            );
         },
     )
 }
@@ -225,10 +274,10 @@ fn extraction_target(node: &SyntaxNode, selection_range: TextRange) -> Option<Fu
     if let Some(stmt) = ast::Stmt::cast(node.clone()) {
         return match stmt {
             ast::Stmt::Item(_) => None,
-            ast::Stmt::ExprStmt(_) | ast::Stmt::LetStmt(_) => Some(FunctionBody::from_range(
+            ast::Stmt::ExprStmt(_) | ast::Stmt::LetStmt(_) => FunctionBody::from_range(
                 node.parent().and_then(ast::StmtList::cast)?,
                 node.text_range(),
-            )),
+            ),
         };
     }
 
@@ -241,7 +290,7 @@ fn extraction_target(node: &SyntaxNode, selection_range: TextRange) -> Option<Fu
         }
 
         // Extract the full statements.
-        return Some(FunctionBody::from_range(stmt_list, selection_range));
+        return FunctionBody::from_range(stmt_list, selection_range);
     }
 
     let expr = ast::Expr::cast(node.clone())?;
@@ -371,7 +420,7 @@ impl RetType {
 #[derive(Debug)]
 enum FunctionBody {
     Expr(ast::Expr),
-    Span { parent: ast::StmtList, text_range: TextRange },
+    Span { parent: ast::StmtList, elements: RangeInclusive<SyntaxElement>, text_range: TextRange },
 }
 
 #[derive(Debug)]
@@ -569,26 +618,38 @@ impl FunctionBody {
         }
     }
 
-    fn from_range(parent: ast::StmtList, selected: TextRange) -> FunctionBody {
+    fn from_range(parent: ast::StmtList, selected: TextRange) -> Option<FunctionBody> {
         let full_body = parent.syntax().children_with_tokens();
 
-        let mut text_range = full_body
+        // Get all of the elements intersecting with the selection
+        let mut stmts_in_selection = full_body
             .filter(|it| ast::Stmt::can_cast(it.kind()) || it.kind() == COMMENT)
-            .map(|element| element.text_range())
-            .filter(|&range| selected.intersect(range).filter(|it| !it.is_empty()).is_some())
-            .reduce(|acc, stmt| acc.cover(stmt));
+            .filter(|it| selected.intersect(it.text_range()).filter(|it| !it.is_empty()).is_some());
 
-        if let Some(tail_range) = parent
-            .tail_expr()
-            .map(|it| it.syntax().text_range())
-            .filter(|&it| selected.intersect(it).is_some())
+        let first_element = stmts_in_selection.next();
+
+        // If the tail expr is part of the selection too, make that the last element
+        // Otherwise use the last stmt
+        let last_element = if let Some(tail_expr) =
+            parent.tail_expr().filter(|it| selected.intersect(it.syntax().text_range()).is_some())
         {
-            text_range = Some(match text_range {
-                Some(text_range) => text_range.cover(tail_range),
-                None => tail_range,
-            });
+            Some(tail_expr.syntax().clone().into())
+        } else {
+            stmts_in_selection.last()
+        };
+
+        let elements = match (first_element, last_element) {
+            (None, _) => {
+                cov_mark::hit!(extract_function_empty_selection_is_not_applicable);
+                return None;
             }
-        Self::Span { parent, text_range: text_range.unwrap_or(selected) }
+            (Some(first), None) => first.clone()..=first,
+            (Some(first), Some(last)) => first..=last,
+        };
+
+        let text_range = elements.start().text_range().cover(elements.end().text_range());
+
+        Some(Self::Span { parent, elements, text_range })
     }
 
     fn indent_level(&self) -> IndentLevel {
@@ -601,7 +662,7 @@ impl FunctionBody {
     fn tail_expr(&self) -> Option<ast::Expr> {
         match &self {
             FunctionBody::Expr(expr) => Some(expr.clone()),
-            FunctionBody::Span { parent, text_range } => {
+            FunctionBody::Span { parent, text_range, .. } => {
                 let tail_expr = parent.tail_expr()?;
                 text_range.contains_range(tail_expr.syntax().text_range()).then_some(tail_expr)
             }
@@ -611,7 +672,7 @@ impl FunctionBody {
     fn walk_expr(&self, cb: &mut dyn FnMut(ast::Expr)) {
         match self {
             FunctionBody::Expr(expr) => walk_expr(expr, cb),
-            FunctionBody::Span { parent, text_range } => {
+            FunctionBody::Span { parent, text_range, .. } => {
                 parent
                     .statements()
                     .filter(|stmt| text_range.contains_range(stmt.syntax().text_range()))
@@ -634,7 +695,7 @@ impl FunctionBody {
     fn preorder_expr(&self, cb: &mut dyn FnMut(WalkEvent<ast::Expr>) -> bool) {
         match self {
             FunctionBody::Expr(expr) => preorder_expr(expr, cb),
-            FunctionBody::Span { parent, text_range } => {
+            FunctionBody::Span { parent, text_range, .. } => {
                 parent
                     .statements()
                     .filter(|stmt| text_range.contains_range(stmt.syntax().text_range()))
@@ -657,7 +718,7 @@ impl FunctionBody {
     fn walk_pat(&self, cb: &mut dyn FnMut(ast::Pat)) {
         match self {
             FunctionBody::Expr(expr) => walk_patterns_in_expr(expr, cb),
-            FunctionBody::Span { parent, text_range } => {
+            FunctionBody::Span { parent, text_range, .. } => {
                 parent
                     .statements()
                     .filter(|stmt| text_range.contains_range(stmt.syntax().text_range()))
@@ -1151,7 +1212,7 @@ impl HasTokenAtOffset for FunctionBody {
     fn token_at_offset(&self, offset: TextSize) -> TokenAtOffset<SyntaxToken> {
         match self {
             FunctionBody::Expr(expr) => expr.syntax().token_at_offset(offset),
-            FunctionBody::Span { parent, text_range } => {
+            FunctionBody::Span { parent, text_range, .. } => {
                 match parent.syntax().token_at_offset(offset) {
                     TokenAtOffset::None => TokenAtOffset::None,
                     TokenAtOffset::Single(t) => {
@@ -1316,7 +1377,19 @@ fn impl_type_name(impl_node: &ast::Impl) -> Option<String> {
     Some(impl_node.self_ty()?.to_string())
 }
 
-fn make_call(ctx: &AssistContext<'_>, fun: &Function, indent: IndentLevel) -> String {
+/// Fixes up the call site before the target expressions are replaced with the call expression
+fn fixup_call_site(builder: &mut SourceChangeBuilder, body: &FunctionBody) {
+    let parent_match_arm = body.parent().and_then(ast::MatchArm::cast);
+
+    if let Some(parent_match_arm) = parent_match_arm {
+        if parent_match_arm.comma_token().is_none() {
+            let parent_match_arm = builder.make_mut(parent_match_arm);
+            ted::append_child_raw(parent_match_arm.syntax(), make::token(T![,]));
+        }
+    }
+}
+
+fn make_call(ctx: &AssistContext<'_>, fun: &Function, indent: IndentLevel) -> SyntaxNode {
     let ret_ty = fun.return_type(ctx);
 
     let args = make::arg_list(fun.params.iter().map(|param| param.to_arg(ctx)));
@@ -1334,44 +1407,45 @@ fn make_call(ctx: &AssistContext<'_>, fun: &Function, indent: IndentLevel) -> St
     if fun.control_flow.is_async {
         call_expr = make::expr_await(call_expr);
     }
-    let expr = handler.make_call_expr(call_expr).indent(indent);
 
-    let mut_modifier = |var: &OutlivedLocal| if var.mut_usage_outside_body { "mut " } else { "" };
+    let expr = handler.make_call_expr(call_expr).clone_for_update();
+    expr.indent(indent);
 
-    let mut buf = String::new();
-    match fun.outliving_locals.as_slice() {
-        [] => {}
+    let outliving_bindings = match fun.outliving_locals.as_slice() {
+        [] => None,
         [var] => {
-            let modifier = mut_modifier(var);
             let name = var.local.name(ctx.db());
-            format_to!(buf, "let {modifier}{} = ", name.display(ctx.db()))
+            let name = make::name(&name.display(ctx.db()).to_string());
+            Some(ast::Pat::IdentPat(make::ident_pat(false, var.mut_usage_outside_body, name)))
         }
         vars => {
-            buf.push_str("let (");
-            let bindings = vars.iter().format_with(", ", |local, f| {
-                let modifier = mut_modifier(local);
-                let name = local.local.name(ctx.db());
-                f(&format_args!("{modifier}{}", name.display(ctx.db())))?;
-                Ok(())
+            let binding_pats = vars.iter().map(|var| {
+                let name = var.local.name(ctx.db());
+                let name = make::name(&name.display(ctx.db()).to_string());
+                make::ident_pat(false, var.mut_usage_outside_body, name).into()
             });
-            format_to!(buf, "{bindings}");
-            buf.push_str(") = ");
-        }
+            Some(ast::Pat::TuplePat(make::tuple_pat(binding_pats)))
         }
+    };
 
-    format_to!(buf, "{expr}");
     let parent_match_arm = fun.body.parent().and_then(ast::MatchArm::cast);
-    let insert_comma = parent_match_arm.as_ref().is_some_and(|it| it.comma_token().is_none());
 
-    if insert_comma {
-        buf.push(',');
-    } else if parent_match_arm.is_none()
+    if let Some(bindings) = outliving_bindings {
+        // with bindings that outlive it
+        make::let_stmt(bindings, None, Some(expr)).syntax().clone_for_update()
+    } else if parent_match_arm.as_ref().is_some() {
+        // as a tail expr for a match arm
+        expr.syntax().clone()
+    } else if parent_match_arm.as_ref().is_none()
         && fun.ret_ty.is_unit()
         && (!fun.outliving_locals.is_empty() || !expr.is_block_like())
     {
-        buf.push(';');
+        // as an expr stmt
+        make::expr_stmt(expr).syntax().clone_for_update()
+    } else {
+        // as a tail expr, or a block
+        expr.syntax().clone()
     }
-    buf
 }
 
 enum FlowHandler {
@@ -1500,42 +1574,25 @@ fn format_function(
     module: hir::Module,
     fun: &Function,
     old_indent: IndentLevel,
-    new_indent: IndentLevel,
-) -> String {
-    let mut fn_def = String::new();
-
-    let fun_name = &fun.name;
+) -> ast::Fn {
+    let fun_name = make::name(&fun.name.text());
     let params = fun.make_param_list(ctx, module);
     let ret_ty = fun.make_ret_ty(ctx, module);
-    let body = make_body(ctx, old_indent, new_indent, fun);
-    let const_kw = if fun.mods.is_const { "const " } else { "" };
-    let async_kw = if fun.control_flow.is_async { "async " } else { "" };
-    let unsafe_kw = if fun.control_flow.is_unsafe { "unsafe " } else { "" };
+    let body = make_body(ctx, old_indent, fun);
     let (generic_params, where_clause) = make_generic_params_and_where_clause(ctx, fun);
 
-    format_to!(fn_def, "\n\n{new_indent}{const_kw}{async_kw}{unsafe_kw}");
-    match ctx.config.snippet_cap {
-        Some(_) => format_to!(fn_def, "fn $0{fun_name}"),
-        None => format_to!(fn_def, "fn {fun_name}"),
-    }
-
-    if let Some(generic_params) = generic_params {
-        format_to!(fn_def, "{generic_params}");
-    }
-
-    format_to!(fn_def, "{params}");
-
-    if let Some(ret_ty) = ret_ty {
-        format_to!(fn_def, " {ret_ty}");
-    }
-
-    if let Some(where_clause) = where_clause {
-        format_to!(fn_def, " {where_clause}");
-    }
-
-    format_to!(fn_def, " {body}");
-
-    fn_def
+    make::fn_(
+        None,
+        fun_name,
+        generic_params,
+        where_clause,
+        params,
+        body,
+        ret_ty,
+        fun.control_flow.is_async,
+        fun.mods.is_const,
+        fun.control_flow.is_unsafe,
+    )
 }
 
 fn make_generic_params_and_where_clause(
@@ -1716,12 +1773,7 @@ impl FunType {
     }
 }
 
-fn make_body(
-    ctx: &AssistContext<'_>,
-    old_indent: IndentLevel,
-    new_indent: IndentLevel,
-    fun: &Function,
-) -> ast::BlockExpr {
+fn make_body(ctx: &AssistContext<'_>, old_indent: IndentLevel, fun: &Function) -> ast::BlockExpr {
     let ret_ty = fun.return_type(ctx);
     let handler = FlowHandler::from_ret_ty(fun, &ret_ty);
 
@@ -1732,7 +1784,7 @@ fn make_body(
             match expr {
                 ast::Expr::BlockExpr(block) => {
                     // If the extracted expression is itself a block, there is no need to wrap it inside another block.
-                    let block = block.dedent(old_indent);
+                    block.dedent(old_indent);
                     let elements = block.stmt_list().map_or_else(
                         || Either::Left(iter::empty()),
                         |stmt_list| {
@@ -1752,13 +1804,13 @@ fn make_body(
                     make::hacky_block_expr(elements, block.tail_expr())
                 }
                 _ => {
-                    let expr = expr.dedent(old_indent).indent(IndentLevel(1));
+                    expr.reindent_to(1.into());
 
                     make::block_expr(Vec::new(), Some(expr))
                 }
             }
         }
-        FunctionBody::Span { parent, text_range } => {
+        FunctionBody::Span { parent, text_range, .. } => {
             let mut elements: Vec<_> = parent
                 .syntax()
                 .children_with_tokens()
@@ -1801,8 +1853,8 @@ fn make_body(
                 .map(|node_or_token| match &node_or_token {
                     syntax::NodeOrToken::Node(node) => match ast::Stmt::cast(node.clone()) {
                         Some(stmt) => {
-                            let indented = stmt.dedent(old_indent).indent(body_indent);
-                            let ast_node = indented.syntax().clone_subtree();
+                            stmt.reindent_to(body_indent);
+                            let ast_node = stmt.syntax().clone_subtree();
                             syntax::NodeOrToken::Node(ast_node)
                         }
                         _ => node_or_token,
@@ -1810,13 +1862,15 @@ fn make_body(
                     _ => node_or_token,
                 })
                 .collect::<Vec<SyntaxElement>>();
-            let tail_expr = tail_expr.map(|expr| expr.dedent(old_indent).indent(body_indent));
+            if let Some(tail_expr) = &mut tail_expr {
+                tail_expr.reindent_to(body_indent);
+            }
 
             make::hacky_block_expr(elements, tail_expr)
         }
     };
 
-    let block = match &handler {
+    match &handler {
         FlowHandler::None => block,
         FlowHandler::Try { kind } => {
             let block = with_default_tail_expr(block, make::expr_unit());
@@ -1851,9 +1905,7 @@ fn make_body(
             let args = make::arg_list(iter::once(tail_expr));
             make::expr_call(ok, args)
         }),
-    };
-
-    block.indent(new_indent)
+    }
 }
 
 fn map_tail_expr(block: ast::BlockExpr, f: impl FnOnce(ast::Expr) -> ast::Expr) -> ast::BlockExpr {
@@ -2551,6 +2603,20 @@ fn $0fun_name(n: u32) -> u32 {
         check_assist_not_applicable(extract_function, r"fn main() { 1 + /* $0comment$0 */ 1; }");
     }
 
+    #[test]
+    fn empty_selection_is_not_applicable() {
+        cov_mark::check!(extract_function_empty_selection_is_not_applicable);
+        check_assist_not_applicable(
+            extract_function,
+            r#"
+fn main() {
+    $0
+
+    $0
+}"#,
+        );
+    }
+
     #[test]
     fn part_of_expr_stmt() {
         check_assist(

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

@@ -161,13 +161,13 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<'
                     Some(impl_def) => edit.make_mut(impl_def),
                     None => {
                         let name = &strukt_name.to_string();
-                        let params = strukt.generic_param_list();
-                        let ty_params = params;
+                        let ty_params = strukt.generic_param_list();
+                        let ty_args = ty_params.as_ref().map(|it| it.to_generic_args());
                         let where_clause = strukt.where_clause();
 
                         let impl_def = make::impl_(
                             ty_params,
-                            None,
+                            ty_args,
                             make::ty_path(make::ext::ident_path(name)),
                             where_clause,
                             None,

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

@@ -956,7 +956,8 @@ where
 impl<T> AnotherTrait for S<T>
 where
     T: AnotherTrait,
-{}"#,
+{
+}"#,
         );
     }
 
@@ -1446,7 +1447,8 @@ where
 impl<T> AnotherTrait for S<T>
 where
     T: AnotherTrait,
-{}"#,
+{
+}"#,
         );
     }
 

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

@@ -1033,7 +1033,7 @@ fn fn_arg_type(
         if ty.is_reference() || ty.is_mutable_reference() {
             let famous_defs = &FamousDefs(&ctx.sema, ctx.sema.scope(fn_arg.syntax())?.krate());
             convert_reference_type(ty.strip_references(), ctx.db(), famous_defs)
-                .map(|conversion| conversion.convert_type(ctx.db()))
+                .map(|conversion| conversion.convert_type(ctx.db()).to_string())
                 .or_else(|| ty.display_source_code(ctx.db(), target_module.into(), true).ok())
         } else {
             ty.display_source_code(ctx.db(), target_module.into(), true).ok()

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

@@ -1,12 +1,12 @@
 use ide_db::{famous_defs::FamousDefs, source_change::SourceChangeBuilder};
 use stdx::{format_to, to_lower_snake_case};
 use syntax::{
-    ast::{self, AstNode, HasName, HasVisibility},
-    TextRange,
+    ast::{self, edit_in_place::Indent, make, AstNode, HasName, HasVisibility},
+    ted, TextRange,
 };
 
 use crate::{
-    utils::{convert_reference_type, find_impl_block_end, find_struct_impl, generate_impl_text},
+    utils::{convert_reference_type, find_struct_impl, generate_impl},
     AssistContext, AssistId, AssistKind, Assists, GroupLabel,
 };
 
@@ -214,14 +214,14 @@ fn generate_getter_from_info(
     ctx: &AssistContext<'_>,
     info: &AssistInfo,
     record_field_info: &RecordFieldInfo,
-) -> String {
-    let mut buf = String::with_capacity(512);
-
-    let vis = info.strukt.visibility().map_or(String::new(), |v| format!("{v} "));
+) -> ast::Fn {
     let (ty, body) = if matches!(info.assist_type, AssistType::MutGet) {
         (
-            format!("&mut {}", record_field_info.field_ty),
-            format!("&mut self.{}", record_field_info.field_name),
+            make::ty_ref(record_field_info.field_ty.clone(), true),
+            make::expr_ref(
+                make::expr_field(make::ext::expr_self(), &record_field_info.field_name.text()),
+                true,
+            ),
         )
     } else {
         (|| {
@@ -240,41 +240,52 @@ fn generate_getter_from_info(
         })()
         .unwrap_or_else(|| {
             (
-                format!("&{}", record_field_info.field_ty),
-                format!("&self.{}", record_field_info.field_name),
+                make::ty_ref(record_field_info.field_ty.clone(), false),
+                make::expr_ref(
+                    make::expr_field(make::ext::expr_self(), &record_field_info.field_name.text()),
+                    false,
+                ),
             )
         })
     };
 
-    format_to!(
-        buf,
-        "    {}fn {}(&{}self) -> {} {{
-        {}
-    }}",
-        vis,
-        record_field_info.fn_name,
-        matches!(info.assist_type, AssistType::MutGet).then_some("mut ").unwrap_or_default(),
-        ty,
-        body,
-    );
+    let self_param = if matches!(info.assist_type, AssistType::MutGet) {
+        make::mut_self_param()
+    } else {
+        make::self_param()
+    };
 
-    buf
+    let strukt = &info.strukt;
+    let fn_name = make::name(&record_field_info.fn_name);
+    let params = make::param_list(Some(self_param), []);
+    let ret_type = Some(make::ret_type(ty));
+    let body = make::block_expr([], Some(body));
+
+    make::fn_(strukt.visibility(), fn_name, None, None, params, body, ret_type, false, false, false)
 }
 
-fn generate_setter_from_info(info: &AssistInfo, record_field_info: &RecordFieldInfo) -> String {
-    let mut buf = String::with_capacity(512);
+fn generate_setter_from_info(info: &AssistInfo, record_field_info: &RecordFieldInfo) -> ast::Fn {
     let strukt = &info.strukt;
-    let fn_name = &record_field_info.fn_name;
+    let field_name = &record_field_info.fn_name;
+    let fn_name = make::name(&format!("set_{field_name}"));
     let field_ty = &record_field_info.field_ty;
-    let vis = strukt.visibility().map_or(String::new(), |v| format!("{v} "));
-    format_to!(
-        buf,
-        "    {vis}fn set_{fn_name}(&mut self, {fn_name}: {field_ty}) {{
-        self.{fn_name} = {fn_name};
-    }}"
-    );
 
-    buf
+    // Make the param list
+    // `(&mut self, $field_name: $field_ty)`
+    let field_param =
+        make::param(make::ident_pat(false, false, make::name(field_name)).into(), field_ty.clone());
+    let params = make::param_list(Some(make::mut_self_param()), [field_param]);
+
+    // Make the assignment body
+    // `self.$field_name = $field_name`
+    let self_expr = make::ext::expr_self();
+    let lhs = make::expr_field(self_expr, field_name);
+    let rhs = make::expr_path(make::ext::ident_path(field_name));
+    let assign_stmt = make::expr_stmt(make::expr_assignment(lhs, rhs));
+    let body = make::block_expr([assign_stmt.into()], None);
+
+    // Make the setter fn
+    make::fn_(strukt.visibility(), fn_name, None, None, params, body, None, false, false, false)
 }
 
 fn extract_and_parse(
@@ -367,74 +378,45 @@ fn build_source_change(
 ) {
     let record_fields_count = info_of_record_fields.len();
 
-    let mut buf = String::with_capacity(512);
+    let impl_def = if let Some(impl_def) = &assist_info.impl_def {
+        // We have an existing impl to add to
+        builder.make_mut(impl_def.clone())
+    } else {
+        // Generate a new impl to add the methods to
+        let impl_def = generate_impl(&ast::Adt::Struct(assist_info.strukt.clone()));
 
-    // Check if an impl exists
-    if let Some(impl_def) = &assist_info.impl_def {
-        // Check if impl is empty
-        if let Some(assoc_item_list) = impl_def.assoc_item_list() {
-            if assoc_item_list.assoc_items().next().is_some() {
-                // If not empty then only insert a new line
-                buf.push('\n');
-            }
-        }
-    }
+        // Insert it after the adt
+        let strukt = builder.make_mut(assist_info.strukt.clone());
 
-    for (i, record_field_info) in info_of_record_fields.iter().enumerate() {
-        // this buf inserts a newline at the end of a getter
-        // automatically, if one wants to add one more newline
-        // for separating it from other assoc items, that needs
-        // to be handled separately
-        let mut getter_buf = match assist_info.assist_type {
-            AssistType::Set => generate_setter_from_info(&assist_info, record_field_info),
-            _ => generate_getter_from_info(ctx, &assist_info, record_field_info),
+        ted::insert_all_raw(
+            ted::Position::after(strukt.syntax()),
+            vec![make::tokens::blank_line().into(), impl_def.syntax().clone().into()],
+        );
+
+        impl_def
     };
 
-        // Insert `$0` only for last getter we generate
-        if i == record_fields_count - 1 && ctx.config.snippet_cap.is_some() {
-            getter_buf = getter_buf.replacen("fn ", "fn $0", 1);
-        }
+    let assoc_item_list = impl_def.get_or_create_assoc_item_list();
 
-        // For first element we do not merge with '\n', as
-        // that can be inserted by impl_def check defined
-        // above, for other cases which are:
-        //
-        // - impl exists but it empty, here we would ideally
-        // not want to keep newline between impl <struct> {
-        // and fn <fn-name>() { line
-        //
-        // - next if impl itself does not exist, in this
-        // case we ourselves generate a new impl and that
-        // again ends up with the same reasoning as above
-        // for not keeping newline
-        if i == 0 {
-            buf = buf + &getter_buf;
-        } else {
-            buf = buf + "\n" + &getter_buf;
+    for (i, record_field_info) in info_of_record_fields.iter().enumerate() {
+        // Make the new getter or setter fn
+        let new_fn = match assist_info.assist_type {
+            AssistType::Set => generate_setter_from_info(&assist_info, record_field_info),
+            _ => generate_getter_from_info(ctx, &assist_info, record_field_info),
         }
+        .clone_for_update();
+        new_fn.indent(1.into());
 
-        // We don't insert a new line at the end of
-        // last getter as it will end up in the end
-        // of an impl where we would not like to keep
-        // getter and end of impl ( i.e. `}` ) with an
-        // extra line for no reason
-        if i < record_fields_count - 1 {
-            buf += "\n";
+        // Insert a tabstop only for last method we generate
+        if i == record_fields_count - 1 {
+            if let Some(cap) = ctx.config.snippet_cap {
+                if let Some(name) = new_fn.name() {
+                    builder.add_tabstop_before(cap, name);
+                }
             }
         }
 
-    let start_offset = assist_info
-        .impl_def
-        .as_ref()
-        .and_then(|impl_def| find_impl_block_end(impl_def.to_owned(), &mut buf))
-        .unwrap_or_else(|| {
-            buf = generate_impl_text(&ast::Adt::Struct(assist_info.strukt.clone()), &buf);
-            assist_info.strukt.syntax().text_range().end()
-        });
-
-    match ctx.config.snippet_cap {
-        Some(cap) => builder.insert_snippet(cap, start_offset, buf),
-        None => builder.insert(start_offset, buf),
+        assoc_item_list.add_item(new_fn.clone().into());
     }
 }
 

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

@@ -1,10 +1,10 @@
-use syntax::ast::{self, AstNode, HasName};
-
-use crate::{
-    utils::{generate_impl_text, generate_trait_impl_text_intransitive},
-    AssistContext, AssistId, AssistKind, Assists,
+use syntax::{
+    ast::{self, make, AstNode, HasName},
+    ted,
 };
 
+use crate::{utils, AssistContext, AssistId, AssistKind, Assists};
+
 // Assist: generate_impl
 //
 // Adds a new inherent impl for a type.
@@ -20,9 +20,7 @@ use crate::{
 //     data: T,
 // }
 //
-// impl<T: Clone> Ctx<T> {
-//     $0
-// }
+// impl<T: Clone> Ctx<T> {$0}
 // ```
 pub(crate) fn generate_impl(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
     let nominal = ctx.find_node_at_offset::<ast::Adt>()?;
@@ -38,17 +36,22 @@ pub(crate) fn generate_impl(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio
         format!("Generate impl for `{name}`"),
         target,
         |edit| {
-            let start_offset = nominal.syntax().text_range().end();
-            match ctx.config.snippet_cap {
-                Some(cap) => {
-                    let snippet = generate_impl_text(&nominal, "    $0");
-                    edit.insert_snippet(cap, start_offset, snippet);
-                }
-                None => {
-                    let snippet = generate_impl_text(&nominal, "");
-                    edit.insert(start_offset, snippet);
+            // Generate the impl
+            let impl_ = utils::generate_impl(&nominal);
+
+            // Add a tabstop after the left curly brace
+            if let Some(cap) = ctx.config.snippet_cap {
+                if let Some(l_curly) = impl_.assoc_item_list().and_then(|it| it.l_curly_token()) {
+                    edit.add_tabstop_after_token(cap, l_curly);
                 }
             }
+
+            // Add the impl after the adt
+            let nominal = edit.make_mut(nominal);
+            ted::insert_all_raw(
+                ted::Position::after(nominal.syntax()),
+                vec![make::tokens::blank_line().into(), impl_.syntax().clone().into()],
+            );
         },
     )
 }
@@ -68,9 +71,7 @@ pub(crate) fn generate_impl(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio
 //     data: T,
 // }
 //
-// impl<T: Clone> $0 for Ctx<T> {
-//
-// }
+// impl<T: Clone> ${0:_} for Ctx<T> {}
 // ```
 pub(crate) fn generate_trait_impl(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
     let nominal = ctx.find_node_at_offset::<ast::Adt>()?;
@@ -86,17 +87,22 @@ pub(crate) fn generate_trait_impl(acc: &mut Assists, ctx: &AssistContext<'_>) ->
         format!("Generate trait impl for `{name}`"),
         target,
         |edit| {
-            let start_offset = nominal.syntax().text_range().end();
-            match ctx.config.snippet_cap {
-                Some(cap) => {
-                    let snippet = generate_trait_impl_text_intransitive(&nominal, "$0", "");
-                    edit.insert_snippet(cap, start_offset, snippet);
-                }
-                None => {
-                    let text = generate_trait_impl_text_intransitive(&nominal, "", "");
-                    edit.insert(start_offset, text);
+            // Generate the impl
+            let impl_ = utils::generate_trait_impl_intransitive(&nominal, make::ty_placeholder());
+
+            // Make the trait type a placeholder snippet
+            if let Some(cap) = ctx.config.snippet_cap {
+                if let Some(trait_) = impl_.trait_() {
+                    edit.add_placeholder_snippet(cap, trait_);
                 }
             }
+
+            // Add the impl after the adt
+            let nominal = edit.make_mut(nominal);
+            ted::insert_all_raw(
+                ted::Position::after(nominal.syntax()),
+                vec![make::tokens::blank_line().into(), impl_.syntax().clone().into()],
+            );
         },
     )
 }
@@ -117,9 +123,7 @@ mod tests {
             r#"
                 struct Foo {}
 
-                impl Foo {
-                    $0
-                }
+                impl Foo {$0}
             "#,
         );
     }
@@ -134,9 +138,7 @@ mod tests {
             r#"
                 struct Foo<T: Clone> {}
 
-                impl<T: Clone> Foo<T> {
-                    $0
-                }
+                impl<T: Clone> Foo<T> {$0}
             "#,
         );
     }
@@ -151,9 +153,7 @@ mod tests {
             r#"
                 struct Foo<'a, T: Foo<'a>> {}
 
-                impl<'a, T: Foo<'a>> Foo<'a, T> {
-                    $0
-                }
+                impl<'a, T: Foo<'a>> Foo<'a, T> {$0}
             "#,
         );
     }
@@ -171,9 +171,7 @@ mod tests {
                 struct Foo<'a, T: Foo<'a>> {}
 
                 #[cfg(feature = "foo")]
-                impl<'a, T: Foo<'a>> Foo<'a, T> {
-                    $0
-                }
+                impl<'a, T: Foo<'a>> Foo<'a, T> {$0}
             "#,
         );
     }
@@ -188,9 +186,7 @@ mod tests {
             r#"
                 struct Defaulted<T = i32> {}
 
-                impl<T> Defaulted<T> {
-                    $0
-                }
+                impl<T> Defaulted<T> {$0}
             "#,
         );
     }
@@ -205,9 +201,7 @@ mod tests {
             r#"
                 struct Defaulted<'a, 'b: 'a, T: Debug + Clone + 'a + 'b = String, const S: usize> {}
 
-                impl<'a, 'b: 'a, T: Debug + Clone + 'a + 'b, const S: usize> Defaulted<'a, 'b, T, S> {
-                    $0
-                }
+                impl<'a, 'b: 'a, T: Debug + Clone + 'a + 'b, const S: usize> Defaulted<'a, 'b, T, S> {$0}
             "#,
         );
     }
@@ -222,9 +216,7 @@ mod tests {
             r#"
                 struct Defaulted<const N: i32 = 0> {}
 
-                impl<const N: i32> Defaulted<N> {
-                    $0
-                }
+                impl<const N: i32> Defaulted<N> {$0}
             "#,
         );
     }
@@ -254,8 +246,7 @@ mod tests {
                 impl<T> Struct<T>
                 where
                     T: Trait,
-                {
-                    $0
+                {$0
                 }
             "#,
         );
@@ -285,9 +276,7 @@ mod tests {
             r#"
                 struct Foo {}
 
-                impl $0 for Foo {
-
-                }
+                impl ${0:_} for Foo {}
             "#,
         );
     }
@@ -302,9 +291,7 @@ mod tests {
             r#"
                 struct Foo<T: Clone> {}
 
-                impl<T: Clone> $0 for Foo<T> {
-
-                }
+                impl<T: Clone> ${0:_} for Foo<T> {}
             "#,
         );
     }
@@ -319,9 +306,7 @@ mod tests {
             r#"
                 struct Foo<'a, T: Foo<'a>> {}
 
-                impl<'a, T: Foo<'a>> $0 for Foo<'a, T> {
-
-                }
+                impl<'a, T: Foo<'a>> ${0:_} for Foo<'a, T> {}
             "#,
         );
     }
@@ -339,9 +324,7 @@ mod tests {
                 struct Foo<'a, T: Foo<'a>> {}
 
                 #[cfg(feature = "foo")]
-                impl<'a, T: Foo<'a>> $0 for Foo<'a, T> {
-
-                }
+                impl<'a, T: Foo<'a>> ${0:_} for Foo<'a, T> {}
             "#,
         );
     }
@@ -356,9 +339,7 @@ mod tests {
             r#"
                 struct Defaulted<T = i32> {}
 
-                impl<T> $0 for Defaulted<T> {
-
-                }
+                impl<T> ${0:_} for Defaulted<T> {}
             "#,
         );
     }
@@ -373,9 +354,7 @@ mod tests {
             r#"
                 struct Defaulted<'a, 'b: 'a, T: Debug + Clone + 'a + 'b = String, const S: usize> {}
 
-                impl<'a, 'b: 'a, T: Debug + Clone + 'a + 'b, const S: usize> $0 for Defaulted<'a, 'b, T, S> {
-
-                }
+                impl<'a, 'b: 'a, T: Debug + Clone + 'a + 'b, const S: usize> ${0:_} for Defaulted<'a, 'b, T, S> {}
             "#,
         );
     }
@@ -390,9 +369,7 @@ mod tests {
             r#"
                 struct Defaulted<const N: i32 = 0> {}
 
-                impl<const N: i32> $0 for Defaulted<N> {
-
-                }
+                impl<const N: i32> ${0:_} for Defaulted<N> {}
             "#,
         );
     }
@@ -419,11 +396,10 @@ mod tests {
                     inner: T,
                 }
 
-                impl<T> $0 for Struct<T>
+                impl<T> ${0:_} for Struct<T>
                 where
                     T: Trait,
                 {
-
                 }
             "#,
         );

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

@@ -1,12 +1,13 @@
 use ide_db::{
     imports::import_assets::item_for_path_search, use_trivial_constructor::use_trivial_constructor,
 };
-use itertools::Itertools;
-use stdx::format_to;
-use syntax::ast::{self, AstNode, HasName, HasVisibility, StructKind};
+use syntax::{
+    ast::{self, edit_in_place::Indent, make, AstNode, HasName, HasVisibility, StructKind},
+    ted,
+};
 
 use crate::{
-    utils::{find_impl_block_start, find_struct_impl, generate_impl_text},
+    utils::{find_struct_impl, generate_impl},
     AssistContext, AssistId, AssistKind, Assists,
 };
 
@@ -26,7 +27,9 @@ use crate::{
 // }
 //
 // impl<T: Clone> Ctx<T> {
-//     fn $0new(data: T) -> Self { Self { data } }
+//     fn $0new(data: T) -> Self {
+//         Self { data }
+//     }
 // }
 // ```
 pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
@@ -46,14 +49,6 @@ pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option
 
     let target = strukt.syntax().text_range();
     acc.add(AssistId("generate_new", AssistKind::Generate), "Generate `new`", target, |builder| {
-        let mut buf = String::with_capacity(512);
-
-        if impl_def.is_some() {
-            buf.push('\n');
-        }
-
-        let vis = strukt.visibility().map_or(String::new(), |v| format!("{v} "));
-
         let trivial_constructors = field_list
             .fields()
             .map(|f| {
@@ -76,54 +71,79 @@ pub(crate) fn generate_new(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option
                     &ty,
                 )?;
 
-                Some(format!("{name}: {expr}"))
+                Some(make::record_expr_field(make::name_ref(&name.text()), Some(expr)))
             })
             .collect::<Vec<_>>();
 
-        let params = field_list
-            .fields()
-            .enumerate()
-            .filter_map(|(i, f)| {
+        let params = field_list.fields().enumerate().filter_map(|(i, f)| {
             if trivial_constructors[i].is_none() {
                 let name = f.name()?;
                 let ty = f.ty()?;
 
-                    Some(format!("{name}: {ty}"))
+                Some(make::param(make::ident_pat(false, false, name).into(), ty))
             } else {
                 None
             }
-            })
-            .format(", ");
+        });
+        let params = make::param_list(None, params);
 
-        let fields = field_list
-            .fields()
-            .enumerate()
-            .filter_map(|(i, f)| {
+        let fields = field_list.fields().enumerate().filter_map(|(i, f)| {
             let constructor = trivial_constructors[i].clone();
             if constructor.is_some() {
                 constructor
             } else {
-                    Some(f.name()?.to_string())
+                Some(make::record_expr_field(make::name_ref(&f.name()?.text()), None))
             }
-            })
-            .format(", ");
-
-        format_to!(buf, "    {vis}fn new({params}) -> Self {{ Self {{ {fields} }} }}");
-
-        let start_offset = impl_def
-            .and_then(|impl_def| find_impl_block_start(impl_def, &mut buf))
-            .unwrap_or_else(|| {
-                buf = generate_impl_text(&ast::Adt::Struct(strukt.clone()), &buf);
-                strukt.syntax().text_range().end()
         });
+        let fields = make::record_expr_field_list(fields);
 
-        match ctx.config.snippet_cap {
-            None => builder.insert(start_offset, buf),
-            Some(cap) => {
-                buf = buf.replace("fn new", "fn $0new");
-                builder.insert_snippet(cap, start_offset, buf);
+        let record_expr = make::record_expr(make::ext::ident_path("Self"), fields);
+        let body = make::block_expr(None, Some(record_expr.into()));
+
+        let ret_type = make::ret_type(make::ty_path(make::ext::ident_path("Self")));
+
+        let fn_ = make::fn_(
+            strukt.visibility(),
+            make::name("new"),
+            None,
+            None,
+            params,
+            body,
+            Some(ret_type),
+            false,
+            false,
+            false,
+        )
+        .clone_for_update();
+        fn_.indent(1.into());
+
+        // Add a tabstop before the name
+        if let Some(cap) = ctx.config.snippet_cap {
+            if let Some(name) = fn_.name() {
+                builder.add_tabstop_before(cap, name);
             }
         }
+
+        // Get the mutable version of the impl to modify
+        let impl_def = if let Some(impl_def) = impl_def {
+            builder.make_mut(impl_def)
+        } else {
+            // Generate a new impl to add the method to
+            let impl_def = generate_impl(&ast::Adt::Struct(strukt.clone()));
+
+            // Insert it after the adt
+            let strukt = builder.make_mut(strukt.clone());
+
+            ted::insert_all_raw(
+                ted::Position::after(strukt.syntax()),
+                vec![make::tokens::blank_line().into(), impl_def.syntax().clone().into()],
+            );
+
+            impl_def
+        };
+
+        // Add the `new` method at the start of the impl
+        impl_def.get_or_create_assoc_item_list().add_item_at_start(fn_.into());
     })
 }
 
@@ -148,7 +168,9 @@ struct Empty;
 struct Foo { empty: Empty }
 
 impl Foo {
-    fn $0new() -> Self { Self { empty: Empty } }
+    fn $0new() -> Self {
+        Self { empty: Empty }
+    }
 }
 "#,
         );
@@ -165,7 +187,9 @@ struct Empty;
 struct Foo { baz: String, empty: Empty }
 
 impl Foo {
-    fn $0new(baz: String) -> Self { Self { baz, empty: Empty } }
+    fn $0new(baz: String) -> Self {
+        Self { baz, empty: Empty }
+    }
 }
 "#,
         );
@@ -182,7 +206,9 @@ enum Empty { Bar }
 struct Foo { empty: Empty }
 
 impl Foo {
-    fn $0new() -> Self { Self { empty: Empty::Bar } }
+    fn $0new() -> Self {
+        Self { empty: Empty::Bar }
+    }
 }
 "#,
         );
@@ -201,7 +227,9 @@ struct Empty {}
 struct Foo { empty: Empty }
 
 impl Foo {
-    fn $0new(empty: Empty) -> Self { Self { empty } }
+    fn $0new(empty: Empty) -> Self {
+        Self { empty }
+    }
 }
 "#,
         );
@@ -218,7 +246,9 @@ enum Empty { Bar {} }
 struct Foo { empty: Empty }
 
 impl Foo {
-    fn $0new(empty: Empty) -> Self { Self { empty } }
+    fn $0new(empty: Empty) -> Self {
+        Self { empty }
+    }
 }
 "#,
         );
@@ -235,7 +265,9 @@ struct Foo {$0}
 struct Foo {}
 
 impl Foo {
-    fn $0new() -> Self { Self {  } }
+    fn $0new() -> Self {
+        Self {  }
+    }
 }
 "#,
         );
@@ -248,7 +280,9 @@ struct Foo<T: Clone> {$0}
 struct Foo<T: Clone> {}
 
 impl<T: Clone> Foo<T> {
-    fn $0new() -> Self { Self {  } }
+    fn $0new() -> Self {
+        Self {  }
+    }
 }
 "#,
         );
@@ -261,7 +295,9 @@ struct Foo<'a, T: Foo<'a>> {$0}
 struct Foo<'a, T: Foo<'a>> {}
 
 impl<'a, T: Foo<'a>> Foo<'a, T> {
-    fn $0new() -> Self { Self {  } }
+    fn $0new() -> Self {
+        Self {  }
+    }
 }
 "#,
         );
@@ -274,7 +310,9 @@ struct Foo { baz: String $0}
 struct Foo { baz: String }
 
 impl Foo {
-    fn $0new(baz: String) -> Self { Self { baz } }
+    fn $0new(baz: String) -> Self {
+        Self { baz }
+    }
 }
 "#,
         );
@@ -287,7 +325,9 @@ struct Foo { baz: String, qux: Vec<i32> $0}
 struct Foo { baz: String, qux: Vec<i32> }
 
 impl Foo {
-    fn $0new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }
+    fn $0new(baz: String, qux: Vec<i32>) -> Self {
+        Self { baz, qux }
+    }
 }
 "#,
         );
@@ -304,7 +344,9 @@ struct Foo { pub baz: String, pub qux: Vec<i32> $0}
 struct Foo { pub baz: String, pub qux: Vec<i32> }
 
 impl Foo {
-    fn $0new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }
+    fn $0new(baz: String, qux: Vec<i32>) -> Self {
+        Self { baz, qux }
+    }
 }
 "#,
         );
@@ -323,7 +365,9 @@ impl Foo {}
 struct Foo {}
 
 impl Foo {
-    fn $0new() -> Self { Self {  } }
+    fn $0new() -> Self {
+        Self {  }
+    }
 }
 "#,
         );
@@ -340,7 +384,9 @@ impl Foo {
 struct Foo {}
 
 impl Foo {
-    fn $0new() -> Self { Self {  } }
+    fn $0new() -> Self {
+        Self {  }
+    }
 
     fn qux(&self) {}
 }
@@ -363,7 +409,9 @@ impl Foo {
 struct Foo {}
 
 impl Foo {
-    fn $0new() -> Self { Self {  } }
+    fn $0new() -> Self {
+        Self {  }
+    }
 
     fn qux(&self) {}
     fn baz() -> i32 {
@@ -385,7 +433,9 @@ pub struct Foo {$0}
 pub struct Foo {}
 
 impl Foo {
-    pub fn $0new() -> Self { Self {  } }
+    pub fn $0new() -> Self {
+        Self {  }
+    }
 }
 "#,
         );
@@ -398,7 +448,9 @@ pub(crate) struct Foo {$0}
 pub(crate) struct Foo {}
 
 impl Foo {
-    pub(crate) fn $0new() -> Self { Self {  } }
+    pub(crate) fn $0new() -> Self {
+        Self {  }
+    }
 }
 "#,
         );
@@ -493,7 +545,9 @@ pub struct Source<T> {
 }
 
 impl<T> Source<T> {
-    pub fn $0new(file_id: HirFileId, ast: T) -> Self { Self { file_id, ast } }
+    pub fn $0new(file_id: HirFileId, ast: T) -> Self {
+        Self { file_id, ast }
+    }
 
     pub fn map<F: FnOnce(T) -> U, U>(self, f: F) -> Source<U> {
         Source { file_id: self.file_id, ast: f(self.ast) }

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

@@ -2,15 +2,17 @@ use hir::{InFile, MacroFileIdExt, ModuleDef};
 use ide_db::{helpers::mod_path_to_ast, imports::import_assets::NameToImport, items_locator};
 use itertools::Itertools;
 use syntax::{
-    ast::{self, AstNode, HasName},
+    ast::{self, make, AstNode, HasName},
+    ted,
     SyntaxKind::WHITESPACE,
+    T,
 };
 
 use crate::{
     assist_context::{AssistContext, Assists, SourceChangeBuilder},
     utils::{
-        add_trait_assoc_items_to_impl, filter_assoc_items, gen_trait_fn_body,
-        generate_trait_impl_text, render_snippet, Cursor, DefaultMethods, IgnoreAssocItems,
+        add_trait_assoc_items_to_impl, filter_assoc_items, gen_trait_fn_body, generate_trait_impl,
+        DefaultMethods, IgnoreAssocItems,
     },
     AssistId, AssistKind,
 };
@@ -132,35 +134,59 @@ fn add_assist(
         label,
         target,
         |builder| {
-            let insert_pos = adt.syntax().text_range().end();
+            let insert_after = ted::Position::after(builder.make_mut(adt.clone()).syntax());
+
             let impl_def_with_items =
                 impl_def_from_trait(&ctx.sema, adt, &annotated_name, trait_, replace_trait_path);
             update_attribute(builder, old_derives, old_tree, old_trait_path, attr);
-            let trait_path = replace_trait_path.to_string();
+
+            let trait_path = make::ty_path(replace_trait_path.clone());
+
             match (ctx.config.snippet_cap, impl_def_with_items) {
                 (None, _) => {
-                    builder.insert(insert_pos, generate_trait_impl_text(adt, &trait_path, ""))
+                    let impl_def = generate_trait_impl(adt, trait_path);
+
+                    ted::insert_all(
+                        insert_after,
+                        vec![make::tokens::blank_line().into(), impl_def.syntax().clone().into()],
+                    );
+                }
+                (Some(cap), None) => {
+                    let impl_def = generate_trait_impl(adt, trait_path);
+
+                    if let Some(l_curly) =
+                        impl_def.assoc_item_list().and_then(|it| it.l_curly_token())
+                    {
+                        builder.add_tabstop_after_token(cap, l_curly);
+                    }
+
+                    ted::insert_all(
+                        insert_after,
+                        vec![make::tokens::blank_line().into(), impl_def.syntax().clone().into()],
+                    );
                 }
-                (Some(cap), None) => builder.insert_snippet(
-                    cap,
-                    insert_pos,
-                    generate_trait_impl_text(adt, &trait_path, "    $0"),
-                ),
                 (Some(cap), Some((impl_def, first_assoc_item))) => {
-                    let mut cursor = Cursor::Before(first_assoc_item.syntax());
-                    let placeholder;
+                    let mut added_snippet = false;
                     if let ast::AssocItem::Fn(ref func) = first_assoc_item {
                         if let Some(m) = func.syntax().descendants().find_map(ast::MacroCall::cast)
                         {
                             if m.syntax().text() == "todo!()" {
-                                placeholder = m;
-                                cursor = Cursor::Replace(placeholder.syntax());
+                                // Make the `todo!()` a placeholder
+                                builder.add_placeholder_snippet(cap, m);
+                                added_snippet = true;
                             }
                         }
                     }
 
-                    let rendered = render_snippet(cap, impl_def.syntax(), cursor);
-                    builder.insert_snippet(cap, insert_pos, format!("\n\n{rendered}"))
+                    if !added_snippet {
+                        // If we haven't already added a snippet, add a tabstop before the generated function
+                        builder.add_tabstop_before(cap, first_assoc_item);
+                    }
+
+                    ted::insert_all(
+                        insert_after,
+                        vec![make::tokens::blank_line().into(), impl_def.syntax().clone().into()],
+                    );
                 }
             };
         },
@@ -190,28 +216,7 @@ fn impl_def_from_trait(
     if trait_items.is_empty() {
         return None;
     }
-    let impl_def = {
-        use syntax::ast::Impl;
-        let text = generate_trait_impl_text(adt, trait_path.to_string().as_str(), "");
-        // 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 => {
-                panic!(
-                    "Failed to make ast node `{}` from text {}",
-                    std::any::type_name::<Impl>(),
-                    text
-                )
-            }
-        };
-        let node = node.clone_for_update();
-        assert_eq!(node.syntax().text_range().start(), 0.into());
-        node
-    };
+    let impl_def = generate_trait_impl(adt, make::ty_path(trait_path.clone()));
 
     let first_assoc_item =
         add_trait_assoc_items_to_impl(sema, &trait_items, trait_, &impl_def, target_scope);
@@ -238,20 +243,34 @@ fn update_attribute(
     let has_more_derives = !new_derives.is_empty();
 
     if has_more_derives {
-        let new_derives = format!("({})", new_derives.iter().format(", "));
-        builder.replace(old_tree.syntax().text_range(), new_derives);
-    } else {
-        let attr_range = attr.syntax().text_range();
-        builder.delete(attr_range);
+        let old_tree = builder.make_mut(old_tree.clone());
 
-        if let Some(line_break_range) = attr
-            .syntax()
-            .next_sibling_or_token()
-            .filter(|t| t.kind() == WHITESPACE)
-            .map(|t| t.text_range())
+        // Make the paths into flat lists of tokens in a vec
+        let tt = new_derives.iter().map(|path| path.syntax().clone()).map(|node| {
+            node.descendants_with_tokens()
+                .filter_map(|element| element.into_token())
+                .collect::<Vec<_>>()
+        });
+        // ...which are interspersed with ", "
+        let tt = Itertools::intersperse(tt, vec![make::token(T![,]), make::tokens::single_space()]);
+        // ...wrap them into the appropriate `NodeOrToken` variant
+        let tt = tt.flatten().map(syntax::NodeOrToken::Token);
+        // ...and make them into a flat list of tokens
+        let tt = tt.collect::<Vec<_>>();
+
+        let new_tree = make::token_tree(T!['('], tt).clone_for_update();
+        ted::replace(old_tree.syntax(), new_tree.syntax());
+    } else {
+        // Remove the attr and any trailing whitespace
+        let attr = builder.make_mut(attr.clone());
+
+        if let Some(line_break) =
+            attr.syntax().next_sibling_or_token().filter(|t| t.kind() == WHITESPACE)
         {
-            builder.delete(line_break_range);
+            ted::remove(line_break)
         }
+
+        ted::remove(attr.syntax())
     }
 }
 
@@ -1168,9 +1187,7 @@ struct Foo {
     bar: String,
 }
 
-impl Debug for Foo {
-    $0
-}
+impl Debug for Foo {$0}
             "#,
         )
     }
@@ -1191,9 +1208,7 @@ pub struct Foo {
     bar: String,
 }
 
-impl Debug for Foo {
-    $0
-}
+impl Debug for Foo {$0}
             "#,
         )
     }
@@ -1211,9 +1226,7 @@ struct Foo {}
 #[derive(Display, Serialize)]
 struct Foo {}
 
-impl Debug for Foo {
-    $0
-}
+impl Debug for Foo {$0}
             "#,
         )
     }

crates/ide-assists/src/tests.rs

@@ -687,12 +687,21 @@ pub fn test_some_range(a: int) -> bool {
                                             delete: 59..60,
                                         },
                                         Indel {
-                                            insert: "\n\nfn $0fun_name() -> i32 {\n    5\n}",
+                                            insert: "\n\nfn fun_name() -> i32 {\n    5\n}",
                                             delete: 110..110,
                                         },
                                     ],
                                 },
-                                None,
+                                Some(
+                                    SnippetEdit(
+                                        [
+                                            (
+                                                0,
+                                                124..124,
+                                            ),
+                                        ],
+                                    ),
+                                ),
                             ),
                         },
                         file_system_edits: [],

crates/ide-assists/src/tests/generated.rs

@@ -1513,9 +1513,7 @@ struct Ctx<T: Clone> {
     data: T,
 }
 
-impl<T: Clone> Ctx<T> {
-    $0
-}
+impl<T: Clone> Ctx<T> {$0}
 "#####,
     )
 }
@@ -1603,7 +1601,9 @@ struct Ctx<T: Clone> {
 }
 
 impl<T: Clone> Ctx<T> {
-    fn $0new(data: T) -> Self { Self { data } }
+    fn $0new(data: T) -> Self {
+        Self { data }
+    }
 }
 "#####,
     )
@@ -1702,9 +1702,7 @@ struct Ctx<T: Clone> {
     data: T,
 }
 
-impl<T: Clone> $0 for Ctx<T> {
-
-}
+impl<T: Clone> ${0:_} for Ctx<T> {}
 "#####,
     )
 }

crates/ide-assists/src/utils.rs

@@ -1,12 +1,10 @@
 //! Assorted functions shared by several assists.
 
-use std::ops;
-
 pub(crate) use gen_trait_fn_body::gen_trait_fn_body;
 use hir::{db::HirDatabase, HasAttrs as HirHasAttrs, HirDisplay, InFile, Semantics};
 use ide_db::{
     famous_defs::FamousDefs, path_transform::PathTransform,
-    syntax_helpers::insert_whitespace_into_node::insert_ws_into, RootDatabase, SnippetCap,
+    syntax_helpers::insert_whitespace_into_node::insert_ws_into, RootDatabase,
 };
 use stdx::format_to;
 use syntax::{
@@ -217,43 +215,6 @@ pub fn add_trait_assoc_items_to_impl(
     first_item.unwrap()
 }
 
-#[derive(Clone, Copy, Debug)]
-pub(crate) enum Cursor<'a> {
-    Replace(&'a SyntaxNode),
-    Before(&'a SyntaxNode),
-}
-
-impl<'a> Cursor<'a> {
-    fn node(self) -> &'a SyntaxNode {
-        match self {
-            Cursor::Replace(node) | Cursor::Before(node) => node,
-        }
-    }
-}
-
-pub(crate) fn render_snippet(_cap: SnippetCap, node: &SyntaxNode, cursor: Cursor<'_>) -> String {
-    assert!(cursor.node().ancestors().any(|it| it == *node));
-    let range = cursor.node().text_range() - node.text_range().start();
-    let range: ops::Range<usize> = range.into();
-
-    let mut placeholder = cursor.node().to_string();
-    escape(&mut placeholder);
-    let tab_stop = match cursor {
-        Cursor::Replace(placeholder) => format!("${{0:{placeholder}}}"),
-        Cursor::Before(placeholder) => format!("$0{placeholder}"),
-    };
-
-    let mut buf = node.to_string();
-    buf.replace_range(range, &tab_stop);
-    return buf;
-
-    fn escape(buf: &mut String) {
-        stdx::replace(buf, '{', r"\{");
-        stdx::replace(buf, '}', r"\}");
-        stdx::replace(buf, '$', r"\$");
-    }
-}
-
 pub(crate) fn vis_offset(node: &SyntaxNode) -> TextSize {
     node.children_with_tokens()
         .find(|it| !matches!(it.kind(), WHITESPACE | COMMENT | ATTR))
@@ -445,15 +406,6 @@ fn has_any_fn(imp: &ast::Impl, names: &[String]) -> bool {
     false
 }
 
-/// Find the start of the `impl` block for the given `ast::Impl`.
-//
-// FIXME: this partially overlaps with `find_struct_impl`
-pub(crate) fn find_impl_block_start(impl_def: ast::Impl, buf: &mut String) -> Option<TextSize> {
-    buf.push('\n');
-    let start = impl_def.assoc_item_list().and_then(|it| it.l_curly_token())?.text_range().end();
-    Some(start)
-}
-
 /// Find the end of the `impl` block for the given `ast::Impl`.
 //
 // FIXME: this partially overlaps with `find_struct_impl`
@@ -470,6 +422,7 @@ pub(crate) fn find_impl_block_end(impl_def: ast::Impl, buf: &mut String) -> Opti
 
 /// Generates the surrounding `impl Type { <code> }` including type and lifetime
 /// parameters.
+// FIXME: migrate remaining uses to `generate_impl`
 pub(crate) fn generate_impl_text(adt: &ast::Adt, code: &str) -> String {
     generate_impl_text_inner(adt, None, true, code)
 }
@@ -478,6 +431,7 @@ pub(crate) fn generate_impl_text(adt: &ast::Adt, code: &str) -> String {
 /// and lifetime parameters, with `<trait>` appended to `impl`'s generic parameters' bounds.
 ///
 /// This is useful for traits like `PartialEq`, since `impl<T> PartialEq for U<T>` often requires `T: PartialEq`.
+// FIXME: migrate remaining uses to `generate_trait_impl`
 pub(crate) fn generate_trait_impl_text(adt: &ast::Adt, trait_text: &str, code: &str) -> String {
     generate_impl_text_inner(adt, Some(trait_text), true, code)
 }
@@ -486,6 +440,7 @@ pub(crate) fn generate_trait_impl_text(adt: &ast::Adt, trait_text: &str, code: &
 /// and lifetime parameters, with `impl`'s generic parameters' bounds kept as-is.
 ///
 /// This is useful for traits like `From<T>`, since `impl<T> From<T> for U<T>` doesn't require `T: From<T>`.
+// FIXME: migrate remaining uses to `generate_trait_impl_intransitive`
 pub(crate) fn generate_trait_impl_text_intransitive(
     adt: &ast::Adt,
     trait_text: &str,
@@ -516,7 +471,7 @@ fn generate_impl_text_inner(
                         // Add the current trait to `bounds` if the trait is transitive,
                         // meaning `impl<T> Trait for U<T>` requires `T: Trait`.
                         if trait_is_transitive {
-                            bounds.push(make::type_bound(trait_));
+                            bounds.push(make::type_bound_text(trait_));
                         }
                     };
                     // `{ty_param}: {bounds}`
@@ -574,6 +529,101 @@ fn generate_impl_text_inner(
     buf
 }
 
+/// Generates the corresponding `impl Type {}` including type and lifetime
+/// parameters.
+pub(crate) fn generate_impl(adt: &ast::Adt) -> ast::Impl {
+    generate_impl_inner(adt, None, true)
+}
+
+/// Generates the corresponding `impl <trait> for Type {}` including type
+/// and lifetime parameters, with `<trait>` appended to `impl`'s generic parameters' bounds.
+///
+/// This is useful for traits like `PartialEq`, since `impl<T> PartialEq for U<T>` often requires `T: PartialEq`.
+pub(crate) fn generate_trait_impl(adt: &ast::Adt, trait_: ast::Type) -> ast::Impl {
+    generate_impl_inner(adt, Some(trait_), true)
+}
+
+/// Generates the corresponding `impl <trait> for Type {}` including type
+/// and lifetime parameters, with `impl`'s generic parameters' bounds kept as-is.
+///
+/// This is useful for traits like `From<T>`, since `impl<T> From<T> for U<T>` doesn't require `T: From<T>`.
+pub(crate) fn generate_trait_impl_intransitive(adt: &ast::Adt, trait_: ast::Type) -> ast::Impl {
+    generate_impl_inner(adt, Some(trait_), false)
+}
+
+fn generate_impl_inner(
+    adt: &ast::Adt,
+    trait_: Option<ast::Type>,
+    trait_is_transitive: bool,
+) -> ast::Impl {
+    // Ensure lifetime params are before type & const params
+    let generic_params = adt.generic_param_list().map(|generic_params| {
+        let lifetime_params =
+            generic_params.lifetime_params().map(ast::GenericParam::LifetimeParam);
+        let ty_or_const_params = generic_params.type_or_const_params().map(|param| {
+            match param {
+                ast::TypeOrConstParam::Type(param) => {
+                    let param = param.clone_for_update();
+                    // remove defaults since they can't be specified in impls
+                    param.remove_default();
+                    let mut bounds =
+                        param.type_bound_list().map_or_else(Vec::new, |it| it.bounds().collect());
+                    if let Some(trait_) = &trait_ {
+                        // Add the current trait to `bounds` if the trait is transitive,
+                        // meaning `impl<T> Trait for U<T>` requires `T: Trait`.
+                        if trait_is_transitive {
+                            bounds.push(make::type_bound(trait_.clone()));
+                        }
+                    };
+                    // `{ty_param}: {bounds}`
+                    let param =
+                        make::type_param(param.name().unwrap(), make::type_bound_list(bounds));
+                    ast::GenericParam::TypeParam(param)
+                }
+                ast::TypeOrConstParam::Const(param) => {
+                    let param = param.clone_for_update();
+                    // remove defaults since they can't be specified in impls
+                    param.remove_default();
+                    ast::GenericParam::ConstParam(param)
+                }
+            }
+        });
+
+        make::generic_param_list(itertools::chain(lifetime_params, ty_or_const_params))
+    });
+    let generic_args =
+        generic_params.as_ref().map(|params| params.to_generic_args().clone_for_update());
+    let ty = make::ty_path(make::ext::ident_path(&adt.name().unwrap().text()));
+
+    let impl_ = match trait_ {
+        Some(trait_) => make::impl_trait(
+            false,
+            None,
+            None,
+            generic_params,
+            generic_args,
+            false,
+            trait_,
+            ty,
+            None,
+            adt.where_clause(),
+            None,
+        ),
+        None => make::impl_(generic_params, generic_args, ty, adt.where_clause(), None),
+    }
+    .clone_for_update();
+
+    // Copy any cfg attrs from the original adt
+    let cfg_attrs = adt
+        .attrs()
+        .filter(|attr| attr.as_simple_call().map(|(name, _arg)| name == "cfg").unwrap_or(false));
+    for attr in cfg_attrs {
+        impl_.add_attr(attr.clone_for_update());
+    }
+
+    impl_
+}
+
 pub(crate) fn add_method_to_adt(
     builder: &mut SourceChangeBuilder,
     adt: &ast::Adt,
@@ -620,8 +670,8 @@ enum ReferenceConversionType {
 }
 
 impl ReferenceConversion {
-    pub(crate) fn convert_type(&self, db: &dyn HirDatabase) -> String {
-        match self.conversion {
+    pub(crate) fn convert_type(&self, db: &dyn HirDatabase) -> ast::Type {
+        let ty = match self.conversion {
             ReferenceConversionType::Copy => self.ty.display(db).to_string(),
             ReferenceConversionType::AsRefStr => "&str".to_string(),
             ReferenceConversionType::AsRefSlice => {
@@ -647,21 +697,25 @@ impl ReferenceConversion {
                     type_arguments.next().unwrap().display(db).to_string();
                 format!("Result<&{first_type_argument_name}, &{second_type_argument_name}>")
             }
-        }
+        };
+
+        make::ty(&ty)
     }
 
-    pub(crate) fn getter(&self, field_name: String) -> String {
+    pub(crate) fn getter(&self, field_name: String) -> ast::Expr {
+        let expr = make::expr_field(make::ext::expr_self(), &field_name);
+
         match self.conversion {
-            ReferenceConversionType::Copy => format!("self.{field_name}"),
+            ReferenceConversionType::Copy => expr,
             ReferenceConversionType::AsRefStr
             | ReferenceConversionType::AsRefSlice
             | ReferenceConversionType::Dereferenced
             | ReferenceConversionType::Option
             | ReferenceConversionType::Result => {
                 if self.impls_deref {
-                    format!("&self.{field_name}")
+                    make::expr_ref(expr, false)
                 } else {
-                    format!("self.{field_name}.as_ref()")
+                    make::expr_method_call(expr, make::name_ref("as_ref"), make::arg_list([]))
                 }
             }
         }

crates/syntax/src/ast/edit_in_place.rs

@@ -627,6 +627,8 @@ impl ast::Impl {
 }
 
 impl ast::AssocItemList {
+    /// Adds a new associated item after all of the existing associated items.
+    ///
     /// Attention! This function does align the first line of `item` with respect to `self`,
     /// but it does _not_ change indentation of other lines (if any).
     pub fn add_item(&self, item: ast::AssocItem) {
@@ -650,6 +652,46 @@ impl ast::AssocItemList {
         ];
         ted::insert_all(position, elements);
     }
+
+    /// Adds a new associated item at the start of the associated item list.
+    ///
+    /// Attention! This function does align the first line of `item` with respect to `self`,
+    /// but it does _not_ change indentation of other lines (if any).
+    pub fn add_item_at_start(&self, item: ast::AssocItem) {
+        match self.assoc_items().next() {
+            Some(first_item) => {
+                let indent = IndentLevel::from_node(first_item.syntax());
+                let before = Position::before(first_item.syntax());
+
+                ted::insert_all(
+                    before,
+                    vec![
+                        item.syntax().clone().into(),
+                        make::tokens::whitespace(&format!("\n\n{indent}")).into(),
+                    ],
+                )
+            }
+            None => {
+                let (indent, position, whitespace) = match self.l_curly_token() {
+                    Some(l_curly) => {
+                        normalize_ws_between_braces(self.syntax());
+                        (IndentLevel::from_token(&l_curly) + 1, Position::after(&l_curly), "\n")
+                    }
+                    None => (IndentLevel::single(), Position::first_child_of(self.syntax()), ""),
+                };
+
+                let mut elements = vec![];
+
+                // Avoid pushing an empty whitespace token
+                if !indent.is_zero() || !whitespace.is_empty() {
+                    elements.push(make::tokens::whitespace(&format!("{whitespace}{indent}")).into())
+                }
+                elements.push(item.syntax().clone().into());
+
+                ted::insert_all(position, elements)
+            }
+        };
+    }
 }
 
 impl ast::Fn {

crates/syntax/src/ast/make.rs

@@ -68,6 +68,9 @@ pub mod ext {
     pub fn expr_ty_new(ty: &ast::Type) -> ast::Expr {
         expr_from_text(&format!("{ty}::new()"))
     }
+    pub fn expr_self() -> ast::Expr {
+        expr_from_text("self")
+    }
 
     pub fn zero_number() -> ast::Expr {
         expr_from_text("0")
@@ -236,23 +239,20 @@ fn merge_where_clause(
 
 pub fn impl_(
     generic_params: Option<ast::GenericParamList>,
-    generic_args: Option<ast::GenericParamList>,
+    generic_args: Option<ast::GenericArgList>,
     path_type: ast::Type,
     where_clause: Option<ast::WhereClause>,
     body: Option<Vec<either::Either<ast::Attr, ast::AssocItem>>>,
 ) -> ast::Impl {
-    let (gen_params, tr_gen_args) = match (generic_params, generic_args) {
-        (None, None) => (String::new(), String::new()),
-        (None, Some(args)) => (String::new(), args.to_generic_args().to_string()),
-        (Some(params), None) => (params.to_string(), params.to_generic_args().to_string()),
-        (Some(params), Some(args)) => match merge_gen_params(Some(params.clone()), Some(args)) {
-            Some(merged) => (params.to_string(), merged.to_generic_args().to_string()),
-            None => (params.to_string(), String::new()),
-        },
-    };
+    let gen_args = generic_args.map_or_else(String::new, |it| it.to_string());
+
+    let gen_params = generic_params.map_or_else(String::new, |it| it.to_string());
+
+    let body_newline =
+        if where_clause.is_some() && body.is_none() { "\n".to_string() } else { String::new() };
 
     let where_clause = match where_clause {
-        Some(pr) => pr.to_string(),
+        Some(pr) => format!("\n{pr}\n"),
         None => " ".to_string(),
     };
 
@@ -261,7 +261,9 @@ pub fn impl_(
         None => String::new(),
     };
 
-    ast_from_text(&format!("impl{gen_params} {path_type}{tr_gen_args}{where_clause}{{{}}}", body))
+    ast_from_text(&format!(
+        "impl{gen_params} {path_type}{gen_args}{where_clause}{{{body_newline}{body}}}"
+    ))
 }
 
 pub fn impl_trait(
@@ -282,13 +284,18 @@ pub fn impl_trait(
     let trait_gen_args = trait_gen_args.map(|args| args.to_string()).unwrap_or_default();
     let type_gen_args = type_gen_args.map(|args| args.to_string()).unwrap_or_default();
 
-    let gen_params = match merge_gen_params(trait_gen_params, type_gen_params) {
-        Some(pars) => pars.to_string(),
-        None => String::new(),
-    };
+    let gen_params = merge_gen_params(trait_gen_params, type_gen_params)
+        .map_or_else(String::new, |it| it.to_string());
 
     let is_negative = if is_negative { "! " } else { "" };
 
+    let body_newline =
+        if (ty_where_clause.is_some() || trait_where_clause.is_some()) && body.is_none() {
+            "\n".to_string()
+        } else {
+            String::new()
+        };
+
     let where_clause = merge_where_clause(ty_where_clause, trait_where_clause)
         .map_or_else(|| " ".to_string(), |wc| format!("\n{}\n", wc));
 
@@ -297,7 +304,7 @@ pub fn impl_trait(
         None => String::new(),
     };
 
-    ast_from_text(&format!("{is_unsafe}impl{gen_params} {is_negative}{path_type}{trait_gen_args} for {ty}{type_gen_args}{where_clause}{{{}}}" , body))
+    ast_from_text(&format!("{is_unsafe}impl{gen_params} {is_negative}{path_type}{trait_gen_args} for {ty}{type_gen_args}{where_clause}{{{body_newline}{body}}}"))
 }
 
 pub fn impl_trait_type(bounds: ast::TypeBoundList) -> ast::ImplTraitType {
@@ -903,7 +910,12 @@ pub fn trait_(
     ast_from_text(&text)
 }
 
-pub fn type_bound(bound: &str) -> ast::TypeBound {
+// FIXME: remove when no one depends on `generate_impl_text_inner`
+pub fn type_bound_text(bound: &str) -> ast::TypeBound {
+    ast_from_text(&format!("fn f<T: {bound}>() {{ }}"))
+}
+
+pub fn type_bound(bound: ast::Type) -> ast::TypeBound {
     ast_from_text(&format!("fn f<T: {bound}>() {{ }}"))
 }