Merge #4530

4530: Use snippets in change_return_type_to_result r=matklad a=matklad



bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

6 files changed, 83 insertions, 90 deletions

diff --git a/crates/ra_assists/src/handlers/add_new.rs b/crates/ra_assists/src/handlers/add_new.rs
index fe7451dcf..837aa8377 100644
--- a/crates/ra_assists/src/handlers/add_new.rs
+++ b/crates/ra_assists/src/handlers/add_new.rs
@@ -3,7 +3,7 @@ use ra_syntax::{
     ast::{
         self, AstNode, NameOwner, StructKind, TypeAscriptionOwner, TypeParamsOwner, VisibilityOwner,
     },
-    TextSize, T,
+    T,
 };
 use stdx::{format_to, SepBy};
 
@@ -25,7 +25,7 @@ use crate::{AssistContext, AssistId, Assists};
 // }
 //
 // impl<T: Clone> Ctx<T> {
-//     fn new(data: T) -> Self { Self { data } }
+//     fn $0new(data: T) -> Self { Self { data } }
 // }
 //
 // ```
@@ -42,31 +42,26 @@ pub(crate) fn add_new(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let impl_def = find_struct_impl(&ctx, &strukt)?;
 
     let target = strukt.syntax().text_range();
-    acc.add(AssistId("add_new"), "Add default constructor", target, |edit| {
+    acc.add(AssistId("add_new"), "Add default constructor", target, |builder| {
         let mut buf = String::with_capacity(512);
 
         if impl_def.is_some() {
             buf.push('\n');
         }
 
-        let vis = strukt.visibility().map(|v| format!("{} ", v));
-        let vis = vis.as_deref().unwrap_or("");
+        let vis = strukt.visibility().map_or(String::new(), |v| format!("{} ", v));
 
         let params = field_list
             .fields()
             .filter_map(|f| {
-                Some(format!(
-                    "{}: {}",
-                    f.name()?.syntax().text(),
-                    f.ascribed_type()?.syntax().text()
-                ))
+                Some(format!("{}: {}", f.name()?.syntax(), f.ascribed_type()?.syntax()))
             })
             .sep_by(", ");
         let fields = field_list.fields().filter_map(|f| f.name()).sep_by(", ");
 
         format_to!(buf, "    {}fn new({}) -> Self {{ Self {{ {} }} }}", vis, params, fields);
 
-        let (start_offset, end_offset) = impl_def
+        let start_offset = impl_def
             .and_then(|impl_def| {
                 buf.push('\n');
                 let start = impl_def
@@ -76,17 +71,20 @@ pub(crate) fn add_new(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
                     .text_range()
                     .end();
 
-                Some((start, TextSize::of("\n")))
+                Some(start)
             })
             .unwrap_or_else(|| {
                 buf = generate_impl_text(&strukt, &buf);
-                let start = strukt.syntax().text_range().end();
-
-                (start, TextSize::of("\n}\n"))
+                strukt.syntax().text_range().end()
             });
 
-        edit.set_cursor(start_offset + TextSize::of(&buf) - end_offset);
-        edit.insert(start_offset, buf);
+        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);
+            }
+        }
     })
 }
 
@@ -191,7 +189,7 @@ mod tests {
 "struct Foo {}
 
 impl Foo {
-    fn new() -> Self { Self {  } }<|>
+    fn $0new() -> Self { Self {  } }
 }
 ",
         );
@@ -201,7 +199,7 @@ impl Foo {
 "struct Foo<T: Clone> {}
 
 impl<T: Clone> Foo<T> {
-    fn new() -> Self { Self {  } }<|>
+    fn $0new() -> Self { Self {  } }
 }
 ",
         );
@@ -211,7 +209,7 @@ impl<T: Clone> Foo<T> {
 "struct Foo<'a, T: Foo<'a>> {}
 
 impl<'a, T: Foo<'a>> Foo<'a, T> {
-    fn new() -> Self { Self {  } }<|>
+    fn $0new() -> Self { Self {  } }
 }
 ",
         );
@@ -221,7 +219,7 @@ impl<'a, T: Foo<'a>> Foo<'a, T> {
 "struct Foo { baz: String }
 
 impl Foo {
-    fn new(baz: String) -> Self { Self { baz } }<|>
+    fn $0new(baz: String) -> Self { Self { baz } }
 }
 ",
         );
@@ -231,7 +229,7 @@ impl Foo {
 "struct Foo { baz: String, qux: Vec<i32> }
 
 impl Foo {
-    fn new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }<|>
+    fn $0new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }
 }
 ",
         );
@@ -243,7 +241,7 @@ impl Foo {
 "struct Foo { pub baz: String, pub qux: Vec<i32> }
 
 impl Foo {
-    fn new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }<|>
+    fn $0new(baz: String, qux: Vec<i32>) -> Self { Self { baz, qux } }
 }
 ",
         );
@@ -258,7 +256,7 @@ impl Foo {}
 "struct Foo {}
 
 impl Foo {
-    fn new() -> Self { Self {  } }<|>
+    fn $0new() -> Self { Self {  } }
 }
 ",
         );
@@ -273,7 +271,7 @@ impl Foo {
 "struct Foo {}
 
 impl Foo {
-    fn new() -> Self { Self {  } }<|>
+    fn $0new() -> Self { Self {  } }
 
     fn qux(&self) {}
 }
@@ -294,7 +292,7 @@ impl Foo {
 "struct Foo {}
 
 impl Foo {
-    fn new() -> Self { Self {  } }<|>
+    fn $0new() -> Self { Self {  } }
 
     fn qux(&self) {}
     fn baz() -> i32 {
@@ -311,7 +309,7 @@ impl Foo {
 "pub struct Foo {}
 
 impl Foo {
-    pub fn new() -> Self { Self {  } }<|>
+    pub fn $0new() -> Self { Self {  } }
 }
 ",
         );
@@ -321,7 +319,7 @@ impl Foo {
 "pub(crate) struct Foo {}
 
 impl Foo {
-    pub(crate) fn new() -> Self { Self {  } }<|>
+    pub(crate) fn $0new() -> Self { Self {  } }
 }
 ",
         );
@@ -414,7 +412,7 @@ pub struct Source<T> {
 }
 
 impl<T> Source<T> {
-    pub fn new(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) }
diff --git a/crates/ra_assists/src/handlers/auto_import.rs b/crates/ra_assists/src/handlers/auto_import.rs
index 78d23150d..f6d25579e 100644
--- a/crates/ra_assists/src/handlers/auto_import.rs
+++ b/crates/ra_assists/src/handlers/auto_import.rs
@@ -50,7 +50,12 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
             format!("Import `{}`", &import),
             range,
             |builder| {
-                insert_use_statement(&auto_import_assets.syntax_under_caret, &import, ctx, builder);
+                insert_use_statement(
+                    &auto_import_assets.syntax_under_caret,
+                    &import,
+                    ctx,
+                    builder.text_edit_builder(),
+                );
             },
         );
     }
diff --git a/crates/ra_assists/src/handlers/change_return_type_to_result.rs b/crates/ra_assists/src/handlers/change_return_type_to_result.rs
index 5c907097e..c6baa0a57 100644
--- a/crates/ra_assists/src/handlers/change_return_type_to_result.rs
+++ b/crates/ra_assists/src/handlers/change_return_type_to_result.rs
@@ -1,8 +1,6 @@
 use ra_syntax::{
     ast::{self, BlockExpr, Expr, LoopBodyOwner},
-    AstNode,
-    SyntaxKind::{COMMENT, WHITESPACE},
-    SyntaxNode, TextSize,
+    AstNode, SyntaxNode,
 };
 
 use crate::{AssistContext, AssistId, Assists};
@@ -16,39 +14,40 @@ use crate::{AssistContext, AssistId, Assists};
 // ```
 // ->
 // ```
-// fn foo() -> Result<i32, > { Ok(42i32) }
+// fn foo() -> Result<i32, ${0:_}> { Ok(42i32) }
 // ```
 pub(crate) fn change_return_type_to_result(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
-    let fn_def = ctx.find_node_at_offset::<ast::FnDef>();
-    let fn_def = &mut fn_def?;
-    let ret_type = &fn_def.ret_type()?.type_ref()?;
-    if ret_type.syntax().text().to_string().starts_with("Result<") {
+    let ret_type = ctx.find_node_at_offset::<ast::RetType>()?;
+    // FIXME: extend to lambdas as well
+    let fn_def = ret_type.syntax().parent().and_then(ast::FnDef::cast)?;
+
+    let type_ref = &ret_type.type_ref()?;
+    if type_ref.syntax().text().to_string().starts_with("Result<") {
         return None;
     }
 
     let block_expr = &fn_def.body()?;
-    let cursor_in_ret_type =
-        fn_def.ret_type()?.syntax().text_range().contains_range(ctx.frange.range);
-    if !cursor_in_ret_type {
-        return None;
-    }
 
     acc.add(
         AssistId("change_return_type_to_result"),
         "Change return type to Result",
-        ret_type.syntax().text_range(),
-        |edit| {
+        type_ref.syntax().text_range(),
+        |builder| {
             let mut tail_return_expr_collector = TailReturnCollector::new();
             tail_return_expr_collector.collect_jump_exprs(block_expr, false);
             tail_return_expr_collector.collect_tail_exprs(block_expr);
 
             for ret_expr_arg in tail_return_expr_collector.exprs_to_wrap {
-                edit.replace_node_and_indent(&ret_expr_arg, format!("Ok({})", ret_expr_arg));
+                builder.replace_node_and_indent(&ret_expr_arg, format!("Ok({})", ret_expr_arg));
             }
-            edit.replace_node_and_indent(ret_type.syntax(), format!("Result<{}, >", ret_type));
 
-            if let Some(node_start) = result_insertion_offset(&ret_type) {
-                edit.set_cursor(node_start + TextSize::of(&format!("Result<{}, ", ret_type)));
+            match ctx.config.snippet_cap {
+                Some(cap) => {
+                    let snippet = format!("Result<{}, ${{0:_}}>", type_ref);
+                    builder.replace_snippet(cap, type_ref.syntax().text_range(), snippet)
+                }
+                None => builder
+                    .replace(type_ref.syntax().text_range(), format!("Result<{}, _>", type_ref)),
             }
         },
     )
@@ -250,17 +249,8 @@ fn get_tail_expr_from_block(expr: &Expr) -> Option<Vec<NodeType>> {
     }
 }
 
-fn result_insertion_offset(ret_type: &ast::TypeRef) -> Option<TextSize> {
-    let non_ws_child = ret_type
-        .syntax()
-        .children_with_tokens()
-        .find(|it| it.kind() != COMMENT && it.kind() != WHITESPACE)?;
-    Some(non_ws_child.text_range().start())
-}
-
 #[cfg(test)]
 mod tests {
-
     use crate::tests::{check_assist, check_assist_not_applicable};
 
     use super::*;
@@ -273,7 +263,7 @@ mod tests {
                 let test = "test";
                 return 42i32;
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let test = "test";
                 return Ok(42i32);
             }"#,
@@ -288,7 +278,7 @@ mod tests {
                 let test = "test";
                 return 42i32;
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let test = "test";
                 return Ok(42i32);
             }"#,
@@ -314,7 +304,7 @@ mod tests {
                 let test = "test";
                 return 42i32;
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let test = "test";
                 return Ok(42i32);
             }"#,
@@ -329,7 +319,7 @@ mod tests {
                 let test = "test";
                 42i32
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let test = "test";
                 Ok(42i32)
             }"#,
@@ -343,7 +333,7 @@ mod tests {
             r#"fn foo() -> i32<|> {
                 42i32
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 Ok(42i32)
             }"#,
         );
@@ -359,7 +349,7 @@ mod tests {
                     24i32
                 }
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 if true {
                     Ok(42i32)
                 } else {
@@ -384,7 +374,7 @@ mod tests {
                     24i32
                 }
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 if true {
                     if false {
                         Ok(1)
@@ -413,7 +403,7 @@ mod tests {
                     24i32.await
                 }
             }"#,
-            r#"async fn foo() -> Result<i32, <|>> {
+            r#"async fn foo() -> Result<i32, ${0:_}> {
                 if true {
                     if false {
                         Ok(1.await)
@@ -434,7 +424,7 @@ mod tests {
             r#"fn foo() -> [i32;<|> 3] {
                 [1, 2, 3]
             }"#,
-            r#"fn foo() -> Result<[i32; 3], <|>> {
+            r#"fn foo() -> Result<[i32; 3], ${0:_}> {
                 Ok([1, 2, 3])
             }"#,
         );
@@ -455,7 +445,7 @@ mod tests {
                     24 as i32
                 }
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 if true {
                     if false {
                         Ok(1 as i32)
@@ -480,7 +470,7 @@ mod tests {
                     _ => 24i32,
                 }
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let my_var = 5;
                 match my_var {
                     5 => Ok(42i32),
@@ -503,7 +493,7 @@ mod tests {
 
                 my_var
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let my_var = 5;
                 loop {
                     println!("test");
@@ -526,7 +516,7 @@ mod tests {
 
                 my_var
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let my_var = let x = loop {
                     break 1;
                 };
@@ -549,7 +539,7 @@ mod tests {
 
                 res
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let my_var = 5;
                 let res = match my_var {
                     5 => 42i32,
@@ -572,7 +562,7 @@ mod tests {
 
                 res
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let my_var = 5;
                 let res = if my_var == 5 {
                     42i32
@@ -608,7 +598,7 @@ mod tests {
                     },
                 }
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let my_var = 5;
                 match my_var {
                     5 => {
@@ -641,7 +631,7 @@ mod tests {
                 }
                 53i32
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let test = "test";
                 if test == "test" {
                     return Ok(24i32);
@@ -672,7 +662,7 @@ mod tests {
 
                 the_field
             }"#,
-            r#"fn foo(the_field: u32) -> Result<u32, <|>> {
+            r#"fn foo(the_field: u32) -> Result<u32, ${0:_}> {
                 let true_closure = || {
                     return true;
                 };
@@ -711,7 +701,7 @@ mod tests {
 
                 t.unwrap_or_else(|| the_field)
             }"#,
-            r#"fn foo(the_field: u32) -> Result<u32, <|>> {
+            r#"fn foo(the_field: u32) -> Result<u32, ${0:_}> {
                 let true_closure = || {
                     return true;
                 };
@@ -749,7 +739,7 @@ mod tests {
                     i += 1;
                 }
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let test = "test";
                 if test == "test" {
                     return Ok(24i32);
@@ -781,7 +771,7 @@ mod tests {
                     }
                 }
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let test = "test";
                 if test == "test" {
                     return Ok(24i32);
@@ -819,7 +809,7 @@ mod tests {
                     }
                 }
             }"#,
-            r#"fn foo() -> Result<i32, <|>> {
+            r#"fn foo() -> Result<i32, ${0:_}> {
                 let test = "test";
                 let other = 5;
                 if test == "test" {
@@ -860,7 +850,7 @@ mod tests {
 
                 the_field
             }"#,
-            r#"fn foo(the_field: u32) -> Result<u32, <|>> {
+            r#"fn foo(the_field: u32) -> Result<u32, ${0:_}> {
                 if the_field < 5 {
                     let mut i = 0;
                     loop {
@@ -894,7 +884,7 @@ mod tests {
 
                 the_field
             }"#,
-            r#"fn foo(the_field: u32) -> Result<u32, <|>> {
+            r#"fn foo(the_field: u32) -> Result<u32, ${0:_}> {
                 if the_field < 5 {
                     let mut i = 0;
 
@@ -923,7 +913,7 @@ mod tests {
 
                 the_field
             }"#,
-            r#"fn foo(the_field: u32) -> Result<u32, <|>> {
+            r#"fn foo(the_field: u32) -> Result<u32, ${0:_}> {
                 if the_field < 5 {
                     let mut i = 0;
 
@@ -953,7 +943,7 @@ mod tests {
 
                 the_field
             }"#,
-            r#"fn foo(the_field: u32) -> Result<u32, <|>> {
+            r#"fn foo(the_field: u32) -> Result<u32, ${0:_}> {
                 if the_field < 5 {
                     let mut i = 0;
 
diff --git a/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs b/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs
index 1a81d8a0e..d9f84208d 100644
--- a/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs
+++ b/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs
@@ -39,7 +39,7 @@ pub(crate) fn replace_qualified_name_with_use(
         target,
         |builder| {
             let path_to_import = hir_path.mod_path().clone();
-            insert_use_statement(path.syntax(), &path_to_import, ctx, builder);
+            insert_use_statement(path.syntax(), &path_to_import, ctx, builder.text_edit_builder());
 
             if let Some(last) = path.segment() {
                 // Here we are assuming the assist will provide a correct use statement
diff --git a/crates/ra_assists/src/tests/generated.rs b/crates/ra_assists/src/tests/generated.rs
index 3808aded1..cd6129dc5 100644
--- a/crates/ra_assists/src/tests/generated.rs
+++ b/crates/ra_assists/src/tests/generated.rs
@@ -204,7 +204,7 @@ struct Ctx<T: Clone> {
 }
 
 impl<T: Clone> Ctx<T> {
-    fn new(data: T) -> Self { Self { data } }
+    fn $0new(data: T) -> Self { Self { data } }
 }
 
 "#####,
@@ -276,7 +276,7 @@ fn doctest_change_return_type_to_result() {
 fn foo() -> i32<|> { 42i32 }
 "#####,
         r#####"
-fn foo() -> Result<i32, > { Ok(42i32) }
+fn foo() -> Result<i32, ${0:_}> { Ok(42i32) }
 "#####,
     )
 }
diff --git a/crates/ra_assists/src/utils/insert_use.rs b/crates/ra_assists/src/utils/insert_use.rs
index 1214e3cd4..0ee43482f 100644
--- a/crates/ra_assists/src/utils/insert_use.rs
+++ b/crates/ra_assists/src/utils/insert_use.rs
@@ -11,7 +11,7 @@ use ra_syntax::{
 };
 use ra_text_edit::TextEditBuilder;
 
-use crate::assist_context::{AssistBuilder, AssistContext};
+use crate::assist_context::AssistContext;
 
 /// Creates and inserts a use statement for the given path to import.
 /// The use statement is inserted in the scope most appropriate to the
@@ -21,7 +21,7 @@ pub(crate) fn insert_use_statement(
     position: &SyntaxNode,
     path_to_import: &ModPath,
     ctx: &AssistContext,
-    builder: &mut AssistBuilder,
+    builder: &mut TextEditBuilder,
 ) {
     let target = path_to_import.to_string().split("::").map(SmolStr::new).collect::<Vec<_>>();
     let container = ctx.sema.ancestors_with_macros(position.clone()).find_map(|n| {
@@ -33,7 +33,7 @@ pub(crate) fn insert_use_statement(
 
     if let Some(container) = container {
         let action = best_action_for_target(container, position.clone(), &target);
-        make_assist(&action, &target, builder.text_edit_builder());
+        make_assist(&action, &target, builder);
     }
 }