Inline parameters in inline_call if possible

Author: Veykril

crates/ide_assists/src/handlers/inline_call.rs

@@ -1,5 +1,7 @@
 use ast::make;
 use hir::{HasSource, PathResolution};
+use ide_db::{defs::Definition, search::FileReference};
+use itertools::izip;
 use syntax::{
     ast::{self, edit::AstNodeEdit, ArgListOwner},
     ted, AstNode,
@@ -69,23 +71,25 @@ pub(crate) fn inline_(
     arg_list: Vec<ast::Expr>,
     expr: ast::Expr,
 ) -> Option<()> {
-    let hir::InFile { value: function_source, .. } = function.source(ctx.db())?;
+    let hir::InFile { value: function_source, file_id } = function.source(ctx.db())?;
     let param_list = function_source.param_list()?;
+    let mut assoc_fn_params = function.assoc_fn_params(ctx.sema.db).into_iter();
 
     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
-        params.push(
+        params.push((
             make::ident_pat(
                 self_param.amp_token().is_some(),
                 self_param.mut_token().is_some(),
                 make::name("this"),
             )
             .into(),
-        );
+            assoc_fn_params.next()?,
+        ));
     }
     for param in param_list.params() {
-        params.push(param.pat()?);
+        params.push((param.pat()?, assoc_fn_params.next()?));
     }
 
     if arg_list.len() != params.len() {
@@ -95,41 +99,95 @@ pub(crate) fn inline_(
         return None;
     }
 
-    let new_bindings = params.into_iter().zip(arg_list);
-
     let body = function_source.body()?;
 
     acc.add(
         AssistId("inline_call", AssistKind::RefactorInline),
         label,
         expr.syntax().text_range(),
         |builder| {
-            // FIXME: emit type ascriptions when a coercion happens?
-            // FIXME: dont create locals when its not required
-            let statements = new_bindings
-                .map(|(pattern, value)| make::let_stmt(pattern, Some(value)).into())
-                .chain(body.statements());
-
-            let original_indentation = expr.indent_level();
-            let mut replacement = make::block_expr(statements, body.tail_expr())
-                .reset_indent()
-                .indent(original_indentation);
-
+            let body = body.clone_for_update();
+
+            let file_id = file_id.original_file(ctx.sema.db);
+            let usages_for_locals = |local| {
+                Definition::Local(local)
+                    .usages(&ctx.sema)
+                    .all()
+                    .references
+                    .remove(&file_id)
+                    .unwrap_or_default()
+                    .into_iter()
+            };
+            // Contains the nodes of usages of parameters.
+            // If the inner Vec for a parameter is empty it either means there are no usages or that the parameter
+            // has a pattern that does not allow inlining
+            let param_use_nodes: Vec<Vec<_>> = params
+                .iter()
+                .map(|(pat, param)| {
+                    if !matches!(pat, ast::Pat::IdentPat(pat) if pat.is_simple_ident()) {
+                        return Vec::new();
+                    }
+                    usages_for_locals(param.as_local(ctx.sema.db))
+                        .map(|FileReference { name, range, .. }| match name {
+                            ast::NameLike::NameRef(_) => body
+                                .syntax()
+                                .covering_element(range)
+                                .ancestors()
+                                .nth(3)
+                                .filter(|it| ast::PathExpr::can_cast(it.kind())),
+                            _ => None,
+                        })
+                        .collect::<Option<Vec<_>>>()
+                        .unwrap_or_default()
+                })
+                .collect();
+
+            // Rewrite `self` to `this`
             if param_list.self_param().is_some() {
-                replacement = replacement.clone_for_update();
                 let this = make::name_ref("this").syntax().clone_for_update();
-                // FIXME dont look into descendant methods
-                replacement
-                    .syntax()
-                    .descendants()
-                    .filter_map(ast::NameRef::cast)
-                    .filter(|n| n.self_token().is_some())
-                    .collect::<Vec<_>>()
-                    .into_iter()
-                    .rev()
-                    .for_each(|self_ref| ted::replace(self_ref.syntax(), &this));
+                usages_for_locals(params[0].1.as_local(ctx.sema.db))
+                    .flat_map(|FileReference { name, range, .. }| match name {
+                        ast::NameLike::NameRef(_) => Some(body.syntax().covering_element(range)),
+                        _ => None,
+                    })
+                    .for_each(|it| {
+                        ted::replace(it, &this);
+                    })
+            }
+
+            // Inline parameter expressions or generate `let` statements depending on whether inlining works or not.
+            for ((pat, _), usages, expr) in izip!(params, param_use_nodes, arg_list).rev() {
+                match &*usages {
+                    // inline single use parameters
+                    [usage] => {
+                        ted::replace(usage, expr.syntax().clone_for_update());
+                    }
+                    // inline parameters whose expression is a simple local reference
+                    [_, ..]
+                        if matches!(&expr,
+                            ast::Expr::PathExpr(expr)
+                                if expr.path().and_then(|path| path.as_single_name_ref()).is_some()
+                        ) =>
+                    {
+                        let expr = expr.syntax().clone_for_update();
+                        usages.into_iter().for_each(|usage| {
+                            ted::replace(usage, &expr);
+                        });
+                    }
+                    // cant inline, emit a let statement
+                    // FIXME: emit type ascriptions when a coercion happens?
+                    _ => body.push_front(make::let_stmt(pat, Some(expr)).clone_for_update().into()),
+                }
             }
-            builder.replace_ast(expr, ast::Expr::BlockExpr(replacement));
+
+            let original_indentation = expr.indent_level();
+            let replacement = body.reset_indent().indent(original_indentation);
+
+            let replacement = match replacement.tail_expr() {
+                Some(expr) if replacement.statements().next().is_none() => expr,
+                _ => ast::Expr::BlockExpr(replacement),
+            };
+            builder.replace_ast(expr, replacement);
         },
     )
 }
@@ -153,9 +211,7 @@ fn main() {
             r#"
 fn foo() { println!("Hello, World!"); }
 fn main() {
-    {
-        println!("Hello, World!");
-    };
+    { println!("Hello, World!"); };
 }
 "#,
         );
@@ -174,10 +230,7 @@ fn main() {
             r#"
 fn foo(name: String) { println!("Hello, {}!", name); }
 fn main() {
-    {
-        let name = String::from("Michael");
-        println!("Hello, {}!", name);
-    };
+    { let name = String::from("Michael"); println!("Hello, {}!", name); };
 }
 "#,
         );
@@ -218,10 +271,8 @@ fn foo(a: u32, b: u32) -> u32 {
 }
 
 fn main() {
-    let x = {
-        let a = 1;
-        let b = 2;
-        let x = a + b;
+    let x = { let b = 2;
+        let x = 1 + b;
         let y = x - b;
         x * y
     };
@@ -257,11 +308,7 @@ impl Foo {
 }
 
 fn main() {
-    let x = {
-        let this = Foo(3);
-        let a = 2;
-        Foo(this.0 + a)
-    };
+    let x = Foo(Foo(3).0 + 2);
 }
 "#,
         );
@@ -294,11 +341,7 @@ impl Foo {
 }
 
 fn main() {
-    let x = {
-        let this = Foo(3);
-        let a = 2;
-        Foo(this.0 + a)
-    };
+    let x = Foo(Foo(3).0 + 2);
 }
 "#,
         );
@@ -331,10 +374,8 @@ impl Foo {
 }
 
 fn main() {
-    let x = {
-        let ref this = Foo(3);
-        let a = 2;
-        Foo(this.0 + a)
+    let x = { let ref this = Foo(3);
+        Foo(this.0 + 2)
     };
 }
 "#,
@@ -370,8 +411,7 @@ impl Foo {
 
 fn main() {
     let mut foo = Foo(3);
-    {
-        let ref mut this = foo;
+    { let ref mut this = foo;
         this.0 = 0;
     };
 }

crates/syntax/src/ast/edit_in_place.rs

@@ -431,6 +431,12 @@ impl ast::RecordExprFieldList {
     }
 }
 
+impl ast::BlockExpr {
+    pub fn push_front(&self, statement: ast::Stmt) {
+        ted::insert(Position::after(self.l_curly_token().unwrap()), statement.syntax());
+    }
+}
+
 fn normalize_ws_between_braces(node: &SyntaxNode) -> Option<()> {
     let l = node
         .children_with_tokens()

crates/syntax/src/ast/node_ext.rs

@@ -641,6 +641,15 @@ impl ast::SlicePat {
     }
 }
 
+impl ast::IdentPat {
+    pub fn is_simple_ident(&self) -> bool {
+        self.at_token().is_none()
+            && self.mut_token().is_none()
+            && self.ref_token().is_none()
+            && self.pat().is_none()
+    }
+}
+
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
 pub enum SelfParamKind {
     /// self