From 95f59668e65d53a985897af1c7ae4c2b449016ba Mon Sep 17 00:00:00 2001
From: Sebastian Ziebell <sebastian.ziebell@ferrous-systems.com>
Date: Mon, 15 May 2023 17:38:25 +0200
Subject: [PATCH] Fix removal of generic param from list

This removes an existing generic param from the `GenericParamList`. It
also considers to remove the extra colon & whitespace to the previous
sibling.

* change order to get all param types first and mark them as mutable
  before the first edit happens
* add helper function to remove a generic parameter
* fix test output
---
 .../replace_named_generic_with_impl.rs        | 32 +++++++++----------
 crates/syntax/src/ast/edit_in_place.rs        | 15 +++++++++
 2 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/crates/ide-assists/src/handlers/replace_named_generic_with_impl.rs b/crates/ide-assists/src/handlers/replace_named_generic_with_impl.rs
index bd73c7b9c9d..f658ba768d8 100644
--- a/crates/ide-assists/src/handlers/replace_named_generic_with_impl.rs
+++ b/crates/ide-assists/src/handlers/replace_named_generic_with_impl.rs
@@ -1,9 +1,5 @@
 use syntax::{
-    ast::{
-        self,
-        make::{self, impl_trait_type},
-        HasGenericParams, HasName, HasTypeBounds,
-    },
+    ast::{self, make::impl_trait_type, HasGenericParams, HasName, HasTypeBounds},
     ted, AstNode,
 };
 
@@ -27,7 +23,7 @@ pub(crate) fn replace_named_generic_with_impl(
     // finds `<P: AsRef<Path>>`
     let type_param = ctx.find_node_at_offset::<ast::TypeParam>()?;
 
-    // The list of type bounds / traits for generic name `P`
+    // The list of type bounds / traits: `AsRef<Path>`
     let type_bound_list = type_param.type_bound_list()?;
 
     // returns `P`
@@ -67,24 +63,26 @@ pub(crate) fn replace_named_generic_with_impl(
             let type_param = edit.make_mut(type_param);
             let fn_ = edit.make_mut(fn_);
 
-            // Replace generic type in `<P: AsRef<Path>>` to `<P>`
-            let new_ty = make::ty(&type_param_name.to_string()).clone_for_update();
-            ted::replace(type_param.syntax(), new_ty.syntax());
+            // get all params
+            let param_types = params
+                .iter()
+                .filter_map(|param| match param.ty() {
+                    Some(ast::Type::PathType(param_type)) => Some(edit.make_mut(param_type)),
+                    _ => None,
+                })
+                .collect::<Vec<_>>();
 
             if let Some(generic_params) = fn_.generic_param_list() {
+                generic_params.remove_generic_param(ast::GenericParam::TypeParam(type_param));
                 if generic_params.generic_params().count() == 0 {
                     ted::remove(generic_params.syntax());
                 }
             }
 
-            // Replace generic type parameter: `foo(p: P)` -> `foo(p: impl AsRef<Path>)`
-            let new_bounds = impl_trait_type(type_bound_list).clone_for_update();
-
-            for param in params {
-                if let Some(ast::Type::PathType(param_type)) = param.ty() {
-                    let param_type = edit.make_mut(param_type).clone_for_update();
-                    ted::replace(param_type.syntax(), new_bounds.syntax());
-                }
+            // get type bounds in signature type: `P` -> `impl AsRef<Path>`
+            let new_bounds = impl_trait_type(type_bound_list);
+            for param_type in param_types.iter().rev() {
+                ted::replace(param_type.syntax(), new_bounds.clone_for_update().syntax());
             }
         },
     )
diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs
index 995e8d8d152..b3ea6ca8d46 100644
--- a/crates/syntax/src/ast/edit_in_place.rs
+++ b/crates/syntax/src/ast/edit_in_place.rs
@@ -236,6 +236,21 @@ impl ast::GenericParamList {
         }
     }
 
+    /// Removes the existing generic param
+    pub fn remove_generic_param(&self, generic_param: ast::GenericParam) {
+        if let Some(previous) = generic_param.syntax().prev_sibling() {
+            if let Some(next_token) = previous.next_sibling_or_token() {
+                ted::remove_all(next_token..=generic_param.syntax().clone().into());
+            }
+        } else if let Some(next) = generic_param.syntax().next_sibling() {
+            if let Some(next_token) = next.prev_sibling_or_token() {
+                ted::remove_all(generic_param.syntax().clone().into()..=next_token);
+            }
+        } else {
+            ted::remove(generic_param.syntax());
+        }
+    }
+
     /// Constructs a matching [`ast::GenericArgList`]
     pub fn to_generic_args(&self) -> ast::GenericArgList {
         let args = self.generic_params().filter_map(|param| match param {