Merge #8781

8781: internal: rewrite **Repalce impl Trait** assist to mutable syntax trees r=matklad a=matklad

bors r+
🤖

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

4 files changed, 64 insertions, 77 deletions

diff --git a/crates/ide_assists/src/handlers/extract_variable.rs b/crates/ide_assists/src/handlers/extract_variable.rs
index 136b9a55b..ae084c86c 100644
--- a/crates/ide_assists/src/handlers/extract_variable.rs
+++ b/crates/ide_assists/src/handlers/extract_variable.rs
@@ -54,7 +54,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option
 
             let var_name = match &field_shorthand {
                 Some(it) => it.to_string(),
-                None => suggest_name::variable(&to_extract, &ctx.sema),
+                None => suggest_name::for_variable(&to_extract, &ctx.sema),
             };
             let expr_range = match &field_shorthand {
                 Some(it) => it.syntax().text_range().cover(to_extract.syntax().text_range()),
diff --git a/crates/ide_assists/src/handlers/introduce_named_lifetime.rs b/crates/ide_assists/src/handlers/introduce_named_lifetime.rs
index 9f4f71d6c..cb71ca8bd 100644
--- a/crates/ide_assists/src/handlers/introduce_named_lifetime.rs
+++ b/crates/ide_assists/src/handlers/introduce_named_lifetime.rs
@@ -139,12 +139,12 @@ fn generate_unique_lifetime_param_name(
 
 fn add_lifetime_param(type_params: ast::GenericParamList, new_lifetime_param: char) {
     let generic_param =
-        make::generic_param(format!("'{}", new_lifetime_param), None).clone_for_update();
+        make::generic_param(&format!("'{}", new_lifetime_param), None).clone_for_update();
     type_params.add_generic_param(generic_param);
 }
 
 fn make_ast_lifetime(new_lifetime_param: char) -> ast::Lifetime {
-    make::generic_param(format!("'{}", new_lifetime_param), None)
+    make::generic_param(&format!("'{}", new_lifetime_param), None)
         .syntax()
         .descendants()
         .find_map(ast::Lifetime::cast)
diff --git a/crates/ide_assists/src/handlers/replace_impl_trait_with_generic.rs b/crates/ide_assists/src/handlers/replace_impl_trait_with_generic.rs
index ff25b61ea..16cae0281 100644
--- a/crates/ide_assists/src/handlers/replace_impl_trait_with_generic.rs
+++ b/crates/ide_assists/src/handlers/replace_impl_trait_with_generic.rs
@@ -1,6 +1,9 @@
-use syntax::ast::{self, edit::AstNodeEdit, make, AstNode, GenericParamsOwner};
+use syntax::{
+    ast::{self, edit_in_place::GenericParamsOwnerEdit, make, AstNode},
+    ted,
+};
 
-use crate::{AssistContext, AssistId, AssistKind, Assists};
+use crate::{utils::suggest_name, AssistContext, AssistId, AssistKind, Assists};
 
 // Assist: replace_impl_trait_with_generic
 //
@@ -17,30 +20,29 @@ pub(crate) fn replace_impl_trait_with_generic(
     acc: &mut Assists,
     ctx: &AssistContext,
 ) -> Option<()> {
-    let type_impl_trait = ctx.find_node_at_offset::<ast::ImplTraitType>()?;
-    let type_param = type_impl_trait.syntax().parent().and_then(ast::Param::cast)?;
-    let type_fn = type_param.syntax().ancestors().find_map(ast::Fn::cast)?;
+    let impl_trait_type = ctx.find_node_at_offset::<ast::ImplTraitType>()?;
+    let param = impl_trait_type.syntax().parent().and_then(ast::Param::cast)?;
+    let fn_ = param.syntax().ancestors().find_map(ast::Fn::cast)?;
 
-    let impl_trait_ty = type_impl_trait.type_bound_list()?;
+    let type_bound_list = impl_trait_type.type_bound_list()?;
 
-    let target = type_fn.syntax().text_range();
+    let target = fn_.syntax().text_range();
     acc.add(
         AssistId("replace_impl_trait_with_generic", AssistKind::RefactorRewrite),
         "Replace impl trait with generic",
         target,
         |edit| {
-            let generic_letter = impl_trait_ty.to_string().chars().next().unwrap().to_string();
+            let impl_trait_type = edit.make_ast_mut(impl_trait_type);
+            let fn_ = edit.make_ast_mut(fn_);
 
-            let generic_param_list = type_fn
-                .generic_param_list()
-                .unwrap_or_else(|| make::generic_param_list(None))
-                .append_param(make::generic_param(generic_letter.clone(), Some(impl_trait_ty)));
+            let type_param_name = suggest_name::for_generic_parameter(&impl_trait_type);
 
-            let new_type_fn = type_fn
-                .replace_descendant::<ast::Type>(type_impl_trait.into(), make::ty(&generic_letter))
-                .with_generic_param_list(generic_param_list);
+            let type_param =
+                make::generic_param(&type_param_name, Some(type_bound_list)).clone_for_update();
+            let new_ty = make::ty(&type_param_name).clone_for_update();
 
-            edit.replace_ast(type_fn.clone(), new_type_fn);
+            ted::replace(impl_trait_type.syntax(), new_ty.syntax());
+            fn_.get_or_create_generic_param_list().add_generic_param(type_param)
         },
     )
 }
@@ -55,12 +57,8 @@ mod tests {
     fn replace_impl_trait_with_generic_params() {
         check_assist(
             replace_impl_trait_with_generic,
-            r#"
-            fn foo<G>(bar: $0impl Bar) {}
-            "#,
-            r#"
-            fn foo<G, B: Bar>(bar: B) {}
-            "#,
+            r#"fn foo<G>(bar: $0impl Bar) {}"#,
+            r#"fn foo<G, B: Bar>(bar: B) {}"#,
         );
     }
 
@@ -68,12 +66,8 @@ mod tests {
     fn replace_impl_trait_without_generic_params() {
         check_assist(
             replace_impl_trait_with_generic,
-            r#"
-            fn foo(bar: $0impl Bar) {}
-            "#,
-            r#"
-            fn foo<B: Bar>(bar: B) {}
-            "#,
+            r#"fn foo(bar: $0impl Bar) {}"#,
+            r#"fn foo<B: Bar>(bar: B) {}"#,
         );
     }
 
@@ -81,12 +75,8 @@ mod tests {
     fn replace_two_impl_trait_with_generic_params() {
         check_assist(
             replace_impl_trait_with_generic,
-            r#"
-            fn foo<G>(foo: impl Foo, bar: $0impl Bar) {}
-            "#,
-            r#"
-            fn foo<G, B: Bar>(foo: impl Foo, bar: B) {}
-            "#,
+            r#"fn foo<G>(foo: impl Foo, bar: $0impl Bar) {}"#,
+            r#"fn foo<G, B: Bar>(foo: impl Foo, bar: B) {}"#,
         );
     }
 
@@ -94,12 +84,8 @@ mod tests {
     fn replace_impl_trait_with_empty_generic_params() {
         check_assist(
             replace_impl_trait_with_generic,
-            r#"
-            fn foo<>(bar: $0impl Bar) {}
-            "#,
-            r#"
-            fn foo<B: Bar>(bar: B) {}
-            "#,
+            r#"fn foo<>(bar: $0impl Bar) {}"#,
+            r#"fn foo<B: Bar>(bar: B) {}"#,
         );
     }
 
@@ -108,13 +94,13 @@ mod tests {
         check_assist(
             replace_impl_trait_with_generic,
             r#"
-            fn foo<
-            >(bar: $0impl Bar) {}
-            "#,
+fn foo<
+>(bar: $0impl Bar) {}
+"#,
             r#"
-            fn foo<B: Bar
-            >(bar: B) {}
-            "#,
+fn foo<B: Bar
+>(bar: B) {}
+"#,
         );
     }
 
@@ -123,12 +109,8 @@ mod tests {
     fn replace_impl_trait_with_exist_generic_letter() {
         check_assist(
             replace_impl_trait_with_generic,
-            r#"
-            fn foo<B>(bar: $0impl Bar) {}
-            "#,
-            r#"
-            fn foo<B, C: Bar>(bar: C) {}
-            "#,
+            r#"fn foo<B>(bar: $0impl Bar) {}"#,
+            r#"fn foo<B, C: Bar>(bar: C) {}"#,
         );
     }
 
@@ -137,19 +119,19 @@ mod tests {
         check_assist(
             replace_impl_trait_with_generic,
             r#"
-            fn foo<
-                G: Foo,
-                F,
-                H,
-            >(bar: $0impl Bar) {}
-            "#,
-            r#"
-            fn foo<
-                G: Foo,
-                F,
-                H, B: Bar
-            >(bar: B) {}
-            "#,
+fn foo<
+    G: Foo,
+    F,
+    H,
+>(bar: $0impl Bar) {}
+"#,
+            r#"
+fn foo<
+    G: Foo,
+    F,
+    H, B: Bar,
+>(bar: B) {}
+"#,
         );
     }
 
@@ -157,12 +139,8 @@ mod tests {
     fn replace_impl_trait_multiple() {
         check_assist(
             replace_impl_trait_with_generic,
-            r#"
-            fn foo(bar: $0impl Foo + Bar) {}
-            "#,
-            r#"
-            fn foo<F: Foo + Bar>(bar: F) {}
-            "#,
+            r#"fn foo(bar: $0impl Foo + Bar) {}"#,
+            r#"fn foo<F: Foo + Bar>(bar: F) {}"#,
         );
     }
 }
diff --git a/crates/ide_assists/src/utils/suggest_name.rs b/crates/ide_assists/src/utils/suggest_name.rs
index deafcd630..b3aabeab3 100644
--- a/crates/ide_assists/src/utils/suggest_name.rs
+++ b/crates/ide_assists/src/utils/suggest_name.rs
@@ -6,7 +6,7 @@ use itertools::Itertools;
 use stdx::to_lower_snake_case;
 use syntax::{
     ast::{self, NameOwner},
-    match_ast, AstNode,
+    match_ast, AstNode, SmolStr,
 };
 
 /// Trait names, that will be ignored when in `impl Trait` and `dyn Trait`
@@ -57,6 +57,14 @@ const USELESS_METHODS: &[&str] = &[
     "iter_mut",
 ];
 
+pub(crate) fn for_generic_parameter(ty: &ast::ImplTraitType) -> SmolStr {
+    let c = ty
+        .type_bound_list()
+        .and_then(|bounds| bounds.syntax().text().char_at(0.into()))
+        .unwrap_or('T');
+    c.encode_utf8(&mut [0; 4]).into()
+}
+
 /// Suggest name of variable for given expression
 ///
 /// **NOTE**: it is caller's responsibility to guarantee uniqueness of the name.
@@ -75,7 +83,8 @@ const USELESS_METHODS: &[&str] = &[
 /// It also applies heuristics to filter out less informative names
 ///
 /// Currently it sticks to the first name found.
-pub(crate) fn variable(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> String {
+// FIXME: Microoptimize and return a `SmolStr` here.
+pub(crate) fn for_variable(expr: &ast::Expr, sema: &Semantics<'_, RootDatabase>) -> String {
     // `from_param` does not benifit from stripping
     // it need the largest context possible
     // so we check firstmost
@@ -276,7 +285,7 @@ mod tests {
             frange.range,
             "selection is not an expression(yet contained in one)"
         );
-        let name = variable(&expr, &sema);
+        let name = for_variable(&expr, &sema);
         assert_eq!(&name, expected);
     }