Auto merge of #15231 - DropDemBits:structured-snippet-migrate-2, r=lowr

internal: Migrate more assists to use the structured snippet API

Continuing from #14979

Migrates the following assists:
- `generate_derive`
- `wrap_return_type_in_result`
- `generate_delegate_methods`

As a bonus, `generate_delegate_methods` now generates the function and impl block at the correct indentation 🎉.

6 files changed, 297 insertions, 124 deletions

diff --git a/crates/ide-assists/src/handlers/generate_delegate_methods.rs b/crates/ide-assists/src/handlers/generate_delegate_methods.rs
index 1c2ec79809b..31fc69562c9 100644
--- a/crates/ide-assists/src/handlers/generate_delegate_methods.rs
+++ b/crates/ide-assists/src/handlers/generate_delegate_methods.rs
@@ -1,13 +1,17 @@
 use std::collections::HashSet;
 
 use hir::{self, HasCrate, HasSource, HasVisibility};
-use syntax::ast::{self, make, AstNode, HasGenericParams, HasName, HasVisibility as _};
+use syntax::{
+    ast::{
+        self, edit_in_place::Indent, make, AstNode, HasGenericParams, HasName, HasVisibility as _,
+    },
+    ted,
+};
 
 use crate::{
-    utils::{convert_param_list_to_arg_list, find_struct_impl, render_snippet, Cursor},
+    utils::{convert_param_list_to_arg_list, find_struct_impl},
     AssistContext, AssistId, AssistKind, Assists, GroupLabel,
 };
-use syntax::ast::edit::AstNodeEdit;
 
 // Assist: generate_delegate_methods
 //
@@ -96,7 +100,7 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<'
             AssistId("generate_delegate_methods", AssistKind::Generate),
             format!("Generate delegate for `{field_name}.{name}()`",),
             target,
-            |builder| {
+            |edit| {
                 // Create the function
                 let method_source = match method.source(ctx.db()) {
                     Some(source) => source.value,
@@ -135,36 +139,12 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<'
                     is_const,
                     is_unsafe,
                 )
-                .indent(ast::edit::IndentLevel(1))
                 .clone_for_update();
 
-                let cursor = Cursor::Before(f.syntax());
-
-                // Create or update an impl block, attach the function to it,
-                // then insert into our code.
-                match impl_def {
-                    Some(impl_def) => {
-                        // Remember where in our source our `impl` block lives.
-                        let impl_def = impl_def.clone_for_update();
-                        let old_range = impl_def.syntax().text_range();
-
-                        // Attach the function to the impl block
-                        let assoc_items = impl_def.get_or_create_assoc_item_list();
-                        assoc_items.add_item(f.clone().into());
-
-                        // Update the impl block.
-                        match ctx.config.snippet_cap {
-                            Some(cap) => {
-                                let snippet = render_snippet(cap, impl_def.syntax(), cursor);
-                                builder.replace_snippet(cap, old_range, snippet);
-                            }
-                            None => {
-                                builder.replace(old_range, impl_def.syntax().to_string());
-                            }
-                        }
-                    }
+                // Get the impl to update, or create one if we need to.
+                let impl_def = match impl_def {
+                    Some(impl_def) => edit.make_mut(impl_def),
                     None => {
-                        // Attach the function to the impl block
                         let name = &strukt_name.to_string();
                         let params = strukt.generic_param_list();
                         let ty_params = params.clone();
@@ -178,24 +158,34 @@ pub(crate) fn generate_delegate_methods(acc: &mut Assists, ctx: &AssistContext<'
                             None,
                         )
                         .clone_for_update();
-                        let assoc_items = impl_def.get_or_create_assoc_item_list();
-                        assoc_items.add_item(f.clone().into());
+
+                        // Fixup impl_def indentation
+                        let indent = strukt.indent_level();
+                        impl_def.reindent_to(indent);
 
                         // Insert the impl block.
-                        match ctx.config.snippet_cap {
-                            Some(cap) => {
-                                let offset = strukt.syntax().text_range().end();
-                                let snippet = render_snippet(cap, impl_def.syntax(), cursor);
-                                let snippet = format!("\n\n{snippet}");
-                                builder.insert_snippet(cap, offset, snippet);
-                            }
-                            None => {
-                                let offset = strukt.syntax().text_range().end();
-                                let snippet = format!("\n\n{}", impl_def.syntax());
-                                builder.insert(offset, snippet);
-                            }
-                        }
+                        let strukt = edit.make_mut(strukt.clone());
+                        ted::insert_all(
+                            ted::Position::after(strukt.syntax()),
+                            vec![
+                                make::tokens::whitespace(&format!("\n\n{indent}")).into(),
+                                impl_def.syntax().clone().into(),
+                            ],
+                        );
+
+                        impl_def
                     }
+                };
+
+                // Fixup function indentation.
+                // FIXME: Should really be handled by `AssocItemList::add_item`
+                f.reindent_to(impl_def.indent_level() + 1);
+
+                let assoc_items = impl_def.get_or_create_assoc_item_list();
+                assoc_items.add_item(f.clone().into());
+
+                if let Some(cap) = ctx.config.snippet_cap {
+                    edit.add_tabstop_before(cap, f)
                 }
             },
         )?;
@@ -245,6 +235,45 @@ impl Person {
     }
 
     #[test]
+    fn test_generate_delegate_create_impl_block_match_indent() {
+        check_assist(
+            generate_delegate_methods,
+            r#"
+mod indent {
+    struct Age(u8);
+    impl Age {
+        fn age(&self) -> u8 {
+            self.0
+        }
+    }
+
+    struct Person {
+        ag$0e: Age,
+    }
+}"#,
+            r#"
+mod indent {
+    struct Age(u8);
+    impl Age {
+        fn age(&self) -> u8 {
+            self.0
+        }
+    }
+
+    struct Person {
+        age: Age,
+    }
+
+    impl Person {
+        $0fn age(&self) -> u8 {
+            self.age.age()
+        }
+    }
+}"#,
+        );
+    }
+
+    #[test]
     fn test_generate_delegate_update_impl_block() {
         check_assist(
             generate_delegate_methods,
@@ -282,6 +311,47 @@ impl Person {
     }
 
     #[test]
+    fn test_generate_delegate_update_impl_block_match_indent() {
+        check_assist(
+            generate_delegate_methods,
+            r#"
+mod indent {
+    struct Age(u8);
+    impl Age {
+        fn age(&self) -> u8 {
+            self.0
+        }
+    }
+
+    struct Person {
+        ag$0e: Age,
+    }
+
+    impl Person {}
+}"#,
+            r#"
+mod indent {
+    struct Age(u8);
+    impl Age {
+        fn age(&self) -> u8 {
+            self.0
+        }
+    }
+
+    struct Person {
+        age: Age,
+    }
+
+    impl Person {
+        $0fn age(&self) -> u8 {
+            self.age.age()
+        }
+    }
+}"#,
+        );
+    }
+
+    #[test]
     fn test_generate_delegate_tuple_struct() {
         check_assist(
             generate_delegate_methods,
diff --git a/crates/ide-assists/src/handlers/generate_derive.rs b/crates/ide-assists/src/handlers/generate_derive.rs
index 78ac2eb30e5..747f70f9f6f 100644
--- a/crates/ide-assists/src/handlers/generate_derive.rs
+++ b/crates/ide-assists/src/handlers/generate_derive.rs
@@ -1,7 +1,6 @@
 use syntax::{
-    ast::{self, edit::IndentLevel, AstNode, HasAttrs},
-    SyntaxKind::{COMMENT, WHITESPACE},
-    TextSize,
+    ast::{self, edit_in_place::AttrsOwnerEdit, make, AstNode, HasAttrs},
+    T,
 };
 
 use crate::{AssistContext, AssistId, AssistKind, Assists};
@@ -27,48 +26,37 @@ use crate::{AssistContext, AssistId, AssistKind, Assists};
 pub(crate) fn generate_derive(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> {
     let cap = ctx.config.snippet_cap?;
     let nominal = ctx.find_node_at_offset::<ast::Adt>()?;
-    let node_start = derive_insertion_offset(&nominal)?;
     let target = nominal.syntax().text_range();
-    acc.add(
-        AssistId("generate_derive", AssistKind::Generate),
-        "Add `#[derive]`",
-        target,
-        |builder| {
-            let derive_attr = nominal
-                .attrs()
-                .filter_map(|x| x.as_simple_call())
-                .filter(|(name, _arg)| name == "derive")
-                .map(|(_name, arg)| arg)
-                .next();
-            match derive_attr {
-                None => {
-                    let indent_level = IndentLevel::from_node(nominal.syntax());
-                    builder.insert_snippet(
-                        cap,
-                        node_start,
-                        format!("#[derive($0)]\n{indent_level}"),
-                    );
-                }
-                Some(tt) => {
-                    // Just move the cursor.
-                    builder.insert_snippet(
-                        cap,
-                        tt.syntax().text_range().end() - TextSize::of(')'),
-                        "$0",
-                    )
-                }
-            };
-        },
-    )
-}
+    acc.add(AssistId("generate_derive", AssistKind::Generate), "Add `#[derive]`", target, |edit| {
+        let derive_attr = nominal
+            .attrs()
+            .filter_map(|x| x.as_simple_call())
+            .filter(|(name, _arg)| name == "derive")
+            .map(|(_name, arg)| arg)
+            .next();
+        match derive_attr {
+            None => {
+                let derive = make::attr_outer(make::meta_token_tree(
+                    make::ext::ident_path("derive"),
+                    make::token_tree(T!['('], vec![]).clone_for_update(),
+                ))
+                .clone_for_update();
+
+                let nominal = edit.make_mut(nominal);
+                nominal.add_attr(derive.clone());
 
-// Insert `derive` after doc comments.
-fn derive_insertion_offset(nominal: &ast::Adt) -> Option<TextSize> {
-    let non_ws_child = nominal
-        .syntax()
-        .children_with_tokens()
-        .find(|it| it.kind() != COMMENT && it.kind() != WHITESPACE)?;
-    Some(non_ws_child.text_range().start())
+                edit.add_tabstop_before_token(
+                    cap,
+                    derive.meta().unwrap().token_tree().unwrap().r_paren_token().unwrap(),
+                );
+            }
+            Some(tt) => {
+                // Just move the cursor.
+                let tt = edit.make_mut(tt);
+                edit.add_tabstop_before_token(cap, tt.right_delimiter_token().unwrap());
+            }
+        };
+    })
 }
 
 #[cfg(test)]
@@ -115,6 +103,38 @@ mod m {
     }
 
     #[test]
+    fn add_derive_existing_with_brackets() {
+        check_assist(
+            generate_derive,
+            "
+#[derive[Clone]]
+struct Foo { a: i32$0, }
+",
+            "
+#[derive[Clone$0]]
+struct Foo { a: i32, }
+",
+        );
+    }
+
+    #[test]
+    fn add_derive_existing_missing_delimiter() {
+        // since `#[derive]` isn't a simple attr call (i.e. `#[derive()]`)
+        // we don't consider it as a proper derive attr and generate a new
+        // one instead
+        check_assist(
+            generate_derive,
+            "
+#[derive]
+struct Foo { a: i32$0, }",
+            "
+#[derive]
+#[derive($0)]
+struct Foo { a: i32, }",
+        );
+    }
+
+    #[test]
     fn add_derive_new_with_doc_comment() {
         check_assist(
             generate_derive,
diff --git a/crates/ide-assists/src/handlers/wrap_return_type_in_result.rs b/crates/ide-assists/src/handlers/wrap_return_type_in_result.rs
index b6c489eb62e..24c3387457a 100644
--- a/crates/ide-assists/src/handlers/wrap_return_type_in_result.rs
+++ b/crates/ide-assists/src/handlers/wrap_return_type_in_result.rs
@@ -6,7 +6,7 @@ use ide_db::{
 };
 use syntax::{
     ast::{self, make, Expr},
-    match_ast, AstNode,
+    match_ast, ted, AstNode,
 };
 
 use crate::{AssistContext, AssistId, AssistKind, Assists};
@@ -52,8 +52,8 @@ pub(crate) fn wrap_return_type_in_result(acc: &mut Assists, ctx: &AssistContext<
         AssistId("wrap_return_type_in_result", AssistKind::RefactorRewrite),
         "Wrap return type in Result",
         type_ref.syntax().text_range(),
-        |builder| {
-            let body = ast::Expr::BlockExpr(body);
+        |edit| {
+            let body = edit.make_mut(ast::Expr::BlockExpr(body));
 
             let mut exprs_to_wrap = Vec::new();
             let tail_cb = &mut |e: &_| tail_cb_impl(&mut exprs_to_wrap, e);
@@ -70,17 +70,24 @@ pub(crate) fn wrap_return_type_in_result(acc: &mut Assists, ctx: &AssistContext<
                 let ok_wrapped = make::expr_call(
                     make::expr_path(make::ext::ident_path("Ok")),
                     make::arg_list(iter::once(ret_expr_arg.clone())),
-                );
-                builder.replace_ast(ret_expr_arg, ok_wrapped);
+                )
+                .clone_for_update();
+                ted::replace(ret_expr_arg.syntax(), ok_wrapped.syntax());
             }
 
-            match ctx.config.snippet_cap {
-                Some(cap) => {
-                    let snippet = format!("Result<{type_ref}, ${{0:_}}>");
-                    builder.replace_snippet(cap, type_ref.syntax().text_range(), snippet)
-                }
-                None => builder
-                    .replace(type_ref.syntax().text_range(), format!("Result<{type_ref}, _>")),
+            let new_result_ty =
+                make::ext::ty_result(type_ref.clone(), make::ty_placeholder()).clone_for_update();
+            let old_result_ty = edit.make_mut(type_ref.clone());
+
+            ted::replace(old_result_ty.syntax(), new_result_ty.syntax());
+
+            if let Some(cap) = ctx.config.snippet_cap {
+                let generic_args = new_result_ty
+                    .syntax()
+                    .descendants()
+                    .find_map(ast::GenericArgList::cast)
+                    .unwrap();
+                edit.add_placeholder_snippet(cap, generic_args.generic_args().last().unwrap());
             }
         },
     )
diff --git a/crates/ide-db/src/source_change.rs b/crates/ide-db/src/source_change.rs
index 061fb0f05cf..fad0ca51a02 100644
--- a/crates/ide-db/src/source_change.rs
+++ b/crates/ide-db/src/source_change.rs
@@ -9,7 +9,10 @@ use crate::SnippetCap;
 use base_db::{AnchoredPathBuf, FileId};
 use nohash_hasher::IntMap;
 use stdx::never;
-use syntax::{algo, ast, ted, AstNode, SyntaxNode, SyntaxNodePtr, TextRange, TextSize};
+use syntax::{
+    algo, ast, ted, AstNode, SyntaxElement, SyntaxNode, SyntaxNodePtr, SyntaxToken, TextRange,
+    TextSize,
+};
 use text_edit::{TextEdit, TextEditBuilder};
 
 #[derive(Default, Debug, Clone)]
@@ -237,19 +240,31 @@ impl SourceChangeBuilder {
     /// Adds a tabstop snippet to place the cursor before `node`
     pub fn add_tabstop_before(&mut self, _cap: SnippetCap, node: impl AstNode) {
         assert!(node.syntax().parent().is_some());
-        self.add_snippet(PlaceSnippet::Before(node.syntax().clone()));
+        self.add_snippet(PlaceSnippet::Before(node.syntax().clone().into()));
     }
 
     /// Adds a tabstop snippet to place the cursor after `node`
     pub fn add_tabstop_after(&mut self, _cap: SnippetCap, node: impl AstNode) {
         assert!(node.syntax().parent().is_some());
-        self.add_snippet(PlaceSnippet::After(node.syntax().clone()));
+        self.add_snippet(PlaceSnippet::After(node.syntax().clone().into()));
+    }
+
+    /// Adds a tabstop snippet to place the cursor before `token`
+    pub fn add_tabstop_before_token(&mut self, _cap: SnippetCap, token: SyntaxToken) {
+        assert!(token.parent().is_some());
+        self.add_snippet(PlaceSnippet::Before(token.clone().into()));
+    }
+
+    /// Adds a tabstop snippet to place the cursor after `token`
+    pub fn add_tabstop_after_token(&mut self, _cap: SnippetCap, token: SyntaxToken) {
+        assert!(token.parent().is_some());
+        self.add_snippet(PlaceSnippet::After(token.clone().into()));
     }
 
     /// Adds a snippet to move the cursor selected over `node`
     pub fn add_placeholder_snippet(&mut self, _cap: SnippetCap, node: impl AstNode) {
         assert!(node.syntax().parent().is_some());
-        self.add_snippet(PlaceSnippet::Over(node.syntax().clone()))
+        self.add_snippet(PlaceSnippet::Over(node.syntax().clone().into()))
     }
 
     fn add_snippet(&mut self, snippet: PlaceSnippet) {
@@ -282,38 +297,40 @@ impl From<FileSystemEdit> for SourceChange {
 }
 
 enum PlaceSnippet {
-    /// Place a tabstop before a node
-    Before(SyntaxNode),
-    /// Place a tabstop before a node
-    After(SyntaxNode),
-    /// Place a placeholder snippet in place of the node
-    Over(SyntaxNode),
+    /// Place a tabstop before an element
+    Before(SyntaxElement),
+    /// Place a tabstop before an element
+    After(SyntaxElement),
+    /// Place a placeholder snippet in place of the element
+    Over(SyntaxElement),
 }
 
 impl PlaceSnippet {
-    /// Places the snippet before or over a node with the given tab stop index
+    /// Places the snippet before or over an element with the given tab stop index
     fn place(self, order: usize) {
-        // ensure the target node is still attached
+        // ensure the target element is still attached
         match &self {
-            PlaceSnippet::Before(node) | PlaceSnippet::After(node) | PlaceSnippet::Over(node) => {
-                // node should still be in the tree, but if it isn't
+            PlaceSnippet::Before(element)
+            | PlaceSnippet::After(element)
+            | PlaceSnippet::Over(element) => {
+                // element should still be in the tree, but if it isn't
                 // then it's okay to just ignore this place
-                if stdx::never!(node.parent().is_none()) {
+                if stdx::never!(element.parent().is_none()) {
                     return;
                 }
             }
         }
 
         match self {
-            PlaceSnippet::Before(node) => {
-                ted::insert_raw(ted::Position::before(&node), Self::make_tab_stop(order));
+            PlaceSnippet::Before(element) => {
+                ted::insert_raw(ted::Position::before(&element), Self::make_tab_stop(order));
             }
-            PlaceSnippet::After(node) => {
-                ted::insert_raw(ted::Position::after(&node), Self::make_tab_stop(order));
+            PlaceSnippet::After(element) => {
+                ted::insert_raw(ted::Position::after(&element), Self::make_tab_stop(order));
             }
-            PlaceSnippet::Over(node) => {
-                let position = ted::Position::before(&node);
-                node.detach();
+            PlaceSnippet::Over(element) => {
+                let position = ted::Position::before(&element);
+                element.detach();
 
                 let snippet = ast::SourceFile::parse(&format!("${{{order}:_}}"))
                     .syntax_node()
@@ -321,7 +338,7 @@ impl PlaceSnippet {
 
                 let placeholder =
                     snippet.descendants().find_map(ast::UnderscoreExpr::cast).unwrap();
-                ted::replace(placeholder.syntax(), node);
+                ted::replace(placeholder.syntax(), element);
 
                 ted::insert_raw(position, snippet);
             }
diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs
index 5919609d91e..1f5ed206b4a 100644
--- a/crates/syntax/src/ast/edit_in_place.rs
+++ b/crates/syntax/src/ast/edit_in_place.rs
@@ -213,6 +213,28 @@ pub trait AttrsOwnerEdit: ast::HasAttrs {
             }
         }
     }
+
+    fn add_attr(&self, attr: ast::Attr) {
+        add_attr(self.syntax(), attr);
+
+        fn add_attr(node: &SyntaxNode, attr: ast::Attr) {
+            let indent = IndentLevel::from_node(node);
+            attr.reindent_to(indent);
+
+            let after_attrs_and_comments = node
+                .children_with_tokens()
+                .find(|it| !matches!(it.kind(), WHITESPACE | COMMENT | ATTR))
+                .map_or(Position::first_child_of(node), |it| Position::before(it));
+
+            ted::insert_all(
+                after_attrs_and_comments,
+                vec![
+                    attr.syntax().clone().into(),
+                    make::tokens::whitespace(&format!("\n{indent}")).into(),
+                ],
+            )
+        }
+    }
 }
 
 impl<T: ast::HasAttrs> AttrsOwnerEdit for T {}
diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs
index 3facd90a11d..4c6db0ef06c 100644
--- a/crates/syntax/src/ast/make.rs
+++ b/crates/syntax/src/ast/make.rs
@@ -10,6 +10,8 @@
 //! `parse(format!())` we use internally is an implementation detail -- long
 //! term, it will be replaced with direct tree manipulation.
 use itertools::Itertools;
+use parser::T;
+use rowan::NodeOrToken;
 use stdx::{format_to, never};
 
 use crate::{ast, utils::is_raw_identifier, AstNode, SourceFile, SyntaxKind, SyntaxToken};
@@ -1030,6 +1032,41 @@ pub fn struct_(
     ast_from_text(&format!("{visibility}struct {strukt_name}{type_params}{field_list}{semicolon}",))
 }
 
+pub fn attr_outer(meta: ast::Meta) -> ast::Attr {
+    ast_from_text(&format!("#[{meta}]"))
+}
+
+pub fn attr_inner(meta: ast::Meta) -> ast::Attr {
+    ast_from_text(&format!("#![{meta}]"))
+}
+
+pub fn meta_expr(path: ast::Path, expr: ast::Expr) -> ast::Meta {
+    ast_from_text(&format!("#[{path} = {expr}]"))
+}
+
+pub fn meta_token_tree(path: ast::Path, tt: ast::TokenTree) -> ast::Meta {
+    ast_from_text(&format!("#[{path}{tt}]"))
+}
+
+pub fn meta_path(path: ast::Path) -> ast::Meta {
+    ast_from_text(&format!("#[{path}]"))
+}
+
+pub fn token_tree(
+    delimiter: SyntaxKind,
+    tt: Vec<NodeOrToken<ast::TokenTree, SyntaxToken>>,
+) -> ast::TokenTree {
+    let (l_delimiter, r_delimiter) = match delimiter {
+        T!['('] => ('(', ')'),
+        T!['['] => ('[', ']'),
+        T!['{'] => ('{', '}'),
+        _ => panic!("invalid delimiter `{delimiter:?}`"),
+    };
+    let tt = tt.into_iter().join("");
+
+    ast_from_text(&format!("tt!{l_delimiter}{tt}{r_delimiter}"))
+}
+
 #[track_caller]
 fn ast_from_text<N: AstNode>(text: &str) -> N {
     let parse = SourceFile::parse(text);