Auto merge of #16378 - roife:fix/issue-15470, r=Veykril

fix: better handling of SelfParam in assist 'inline_call'

fix #15470.

The current `inline_call` directly translates `&self` into `let ref this = ...;` and `&mut self` into `let ref mut this = ...;`. However, it does not handle some complex scenarios.

This PR addresses the following transformations (assuming the receiving object is `obj`):

- `self`: `let this = obj`
- `mut self`: `let mut this = obj`
- `&self`: `let this = &obj`
- `&mut self`
  + If `obj` is `let mut obj = ...`, use a mutable reference: `let this = &mut obj`
  + If `obj` is `let obj = &mut ...;`, perform a reborrow: `let this = &mut *obj`

Author: bors

crates/ide-assists/src/handlers/inline_call.rs

@@ -15,7 +15,7 @@ use ide_db::{
 };
 use itertools::{izip, Itertools};
 use syntax::{
-    ast::{self, edit::IndentLevel, edit_in_place::Indent, HasArgList, PathExpr},
+    ast::{self, edit::IndentLevel, edit_in_place::Indent, HasArgList, Pat, PathExpr},
     ted, AstNode, NodeOrToken, SyntaxKind,
 };
 
@@ -278,7 +278,7 @@ fn get_fn_params(
 
     let mut params = Vec::new();
     if let Some(self_param) = param_list.self_param() {
-        // FIXME this should depend on the receiver as well as the self_param
+        // Keep `ref` and `mut` and transform them into `&` and `mut` later
         params.push((
             make::ident_pat(
                 self_param.amp_token().is_some(),
@@ -409,16 +409,56 @@ fn inline(
     let mut let_stmts = Vec::new();
 
     // Inline parameter expressions or generate `let` statements depending on whether inlining works or not.
-    for ((pat, param_ty, _), usages, expr) in izip!(params, param_use_nodes, arguments) {
+    for ((pat, param_ty, param), usages, expr) in izip!(params, param_use_nodes, arguments) {
         // izip confuses RA due to our lack of hygiene info currently losing us type info causing incorrect errors
         let usages: &[ast::PathExpr] = &usages;
         let expr: &ast::Expr = expr;
 
         let mut insert_let_stmt = || {
             let ty = sema.type_of_expr(expr).filter(TypeInfo::has_adjustment).and(param_ty.clone());
-            let_stmts.push(
-                make::let_stmt(pat.clone(), ty, Some(expr.clone())).clone_for_update().into(),
-            );
+
+            let is_self = param
+                .name(sema.db)
+                .and_then(|name| name.as_text())
+                .is_some_and(|name| name == "self");
+
+            if is_self {
+                let mut this_pat = make::ident_pat(false, false, make::name("this"));
+                let mut expr = expr.clone();
+                match pat {
+                    Pat::IdentPat(pat) => match (pat.ref_token(), pat.mut_token()) {
+                        // self => let this = obj
+                        (None, None) => {}
+                        // mut self => let mut this = obj
+                        (None, Some(_)) => {
+                            this_pat = make::ident_pat(false, true, make::name("this"));
+                        }
+                        // &self => let this = &obj
+                        (Some(_), None) => {
+                            expr = make::expr_ref(expr, false);
+                        }
+                        // let foo = &mut X; &mut self => let this = &mut obj
+                        // let mut foo = X;  &mut self => let this = &mut *obj (reborrow)
+                        (Some(_), Some(_)) => {
+                            let should_reborrow = sema
+                                .type_of_expr(&expr)
+                                .map(|ty| ty.original.is_mutable_reference());
+                            expr = if let Some(true) = should_reborrow {
+                                make::expr_reborrow(expr)
+                            } else {
+                                make::expr_ref(expr, true)
+                            };
+                        }
+                    },
+                    _ => {}
+                };
+                let_stmts
+                    .push(make::let_stmt(this_pat.into(), ty, Some(expr)).clone_for_update().into())
+            } else {
+                let_stmts.push(
+                    make::let_stmt(pat.clone(), ty, Some(expr.clone())).clone_for_update().into(),
+                );
+            }
         };
 
         // check if there is a local var in the function that conflicts with parameter
@@ -484,12 +524,10 @@ fn inline(
             body = make::block_expr(let_stmts, Some(body.into())).clone_for_update();
         }
     } else if let Some(stmt_list) = body.stmt_list() {
-        ted::insert_all(
-            ted::Position::after(
-                stmt_list.l_curly_token().expect("L_CURLY for StatementList is missing."),
-            ),
-            let_stmts.into_iter().map(|stmt| stmt.syntax().clone().into()).collect(),
-        );
+        let position = stmt_list.l_curly_token().expect("L_CURLY for StatementList is missing.");
+        let_stmts.into_iter().rev().for_each(|let_stmt| {
+            ted::insert(ted::Position::after(position.clone()), let_stmt.syntax().clone());
+        });
     }
 
     let original_indentation = match node {
@@ -721,7 +759,7 @@ impl Foo {
 
 fn main() {
     let x = {
-        let ref this = Foo(3);
+        let this = &Foo(3);
         Foo(this.0 + 2)
     };
 }
@@ -757,7 +795,7 @@ impl Foo {
 
 fn main() {
     let x = {
-        let ref this = Foo(3);
+        let this = &Foo(3);
         Foo(this.0 + 2)
     };
 }
@@ -795,7 +833,7 @@ impl Foo {
 fn main() {
     let mut foo = Foo(3);
     {
-        let ref mut this = foo;
+        let this = &mut foo;
         this.0 = 0;
     };
 }
@@ -882,7 +920,7 @@ impl Foo {
     }
     fn bar(&self) {
         {
-            let ref this = self;
+            let this = &self;
             this;
             this;
         };
@@ -1557,7 +1595,7 @@ impl Enum {
 
 fn a() -> bool {
     {
-        let ref this = Enum::A;
+        let this = &Enum::A;
         this == &Enum::A || this == &Enum::B
     }
 }
@@ -1619,6 +1657,82 @@ fn main() {
       a as A
     };
 }
+"#,
+        )
+    }
+
+    #[test]
+    fn method_by_reborrow() {
+        check_assist(
+            inline_call,
+            r#"
+pub struct Foo(usize);
+
+impl Foo {
+    fn add1(&mut self) {
+        self.0 += 1;
+    }
+}
+
+pub fn main() {
+    let f = &mut Foo(0);
+    f.add1$0();
+}
+"#,
+            r#"
+pub struct Foo(usize);
+
+impl Foo {
+    fn add1(&mut self) {
+        self.0 += 1;
+    }
+}
+
+pub fn main() {
+    let f = &mut Foo(0);
+    {
+        let this = &mut *f;
+        this.0 += 1;
+    };
+}
+"#,
+        )
+    }
+
+    #[test]
+    fn method_by_mut() {
+        check_assist(
+            inline_call,
+            r#"
+pub struct Foo(usize);
+
+impl Foo {
+    fn add1(mut self) {
+        self.0 += 1;
+    }
+}
+
+pub fn main() {
+    let mut f = Foo(0);
+    f.add1$0();
+}
+"#,
+            r#"
+pub struct Foo(usize);
+
+impl Foo {
+    fn add1(mut self) {
+        self.0 += 1;
+    }
+}
+
+pub fn main() {
+    let mut f = Foo(0);
+    {
+        let mut this = f;
+        this.0 += 1;
+    };
+}
 "#,
         )
     }

crates/syntax/src/ast/make.rs

@@ -595,6 +595,9 @@ pub fn expr_macro_call(f: ast::Expr, arg_list: ast::ArgList) -> ast::Expr {
 pub fn expr_ref(expr: ast::Expr, exclusive: bool) -> ast::Expr {
     expr_from_text(&if exclusive { format!("&mut {expr}") } else { format!("&{expr}") })
 }
+pub fn expr_reborrow(expr: ast::Expr) -> ast::Expr {
+    expr_from_text(&format!("&mut *{expr}"))
+}
 pub fn expr_closure(pats: impl IntoIterator<Item = ast::Param>, expr: ast::Expr) -> ast::Expr {
     let params = pats.into_iter().join(", ");
     expr_from_text(&format!("|{params}| {expr}"))