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

internal: Migrate more assists to use the structured snippet API

Continuing from rust-lang#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 🎉.

Author: bors

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)
                 }
             },
         )?;
@@ -244,6 +234,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(
@@ -281,6 +310,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(

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)]
@@ -114,6 +102,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(

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());
             }
         },
     )