Merge #10382

10382: fix: Fix inline_call breaking RecordExprField shorthands r=Veykril a=Veykril

Fixes #10349
bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

3 files changed, 112 insertions, 20 deletions

diff --git a/crates/ide_assists/src/handlers/inline_call.rs b/crates/ide_assists/src/handlers/inline_call.rs
index 14313fefa77..d252d61a696 100644
--- a/crates/ide_assists/src/handlers/inline_call.rs
+++ b/crates/ide_assists/src/handlers/inline_call.rs
@@ -4,14 +4,14 @@ use hir::{db::HirDatabase, HasSource, PathResolution, Semantics, TypeInfo};
 use ide_db::{
     base_db::{FileId, FileRange},
     defs::Definition,
-    helpers::insert_use::remove_path_if_in_use_stmt,
+    helpers::{insert_use::remove_path_if_in_use_stmt, node_ext::expr_as_name_ref},
     path_transform::PathTransform,
     search::{FileReference, SearchScope},
     RootDatabase,
 };
 use itertools::{izip, Itertools};
 use syntax::{
-    ast::{self, edit_in_place::Indent, HasArgList},
+    ast::{self, edit_in_place::Indent, HasArgList, PathExpr},
     ted, AstNode, SyntaxNode,
 };
 
@@ -359,11 +359,17 @@ fn inline(
     }
     // 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).rev() {
-        let expr_is_name_ref = matches!(&expr,
-            ast::Expr::PathExpr(expr)
-                if expr.path().and_then(|path| path.as_single_name_ref()).is_some()
-        );
-        match &*usages {
+        let inline_direct = |usage, replacement: &ast::Expr| {
+            if let Some(field) = path_expr_as_record_field(usage) {
+                cov_mark::hit!(inline_call_inline_direct_field);
+                field.replace_expr(replacement.clone_for_update());
+            } else {
+                ted::replace(usage.syntax(), &replacement.syntax().clone_for_update());
+            }
+        };
+        // izip confuses RA due to our lack of hygiene info currently losing us typeinfo
+        let usages: &[ast::PathExpr] = &*usages;
+        match usages {
             // inline single use closure arguments
             [usage]
                 if matches!(expr, ast::Expr::ClosureExpr(_))
@@ -371,21 +377,19 @@ fn inline(
             {
                 cov_mark::hit!(inline_call_inline_closure);
                 let expr = make::expr_paren(expr.clone());
-                ted::replace(usage.syntax(), expr.syntax().clone_for_update());
+                inline_direct(usage, &expr);
             }
             // inline single use literals
             [usage] if matches!(expr, ast::Expr::Literal(_)) => {
                 cov_mark::hit!(inline_call_inline_literal);
-                ted::replace(usage.syntax(), expr.syntax().clone_for_update());
+                inline_direct(usage, &expr);
             }
             // inline direct local arguments
-            [_, ..] if expr_is_name_ref => {
+            [_, ..] if expr_as_name_ref(&expr).is_some() => {
                 cov_mark::hit!(inline_call_inline_locals);
-                usages.into_iter().for_each(|usage| {
-                    ted::replace(usage.syntax(), &expr.syntax().clone_for_update());
-                });
+                usages.into_iter().for_each(|usage| inline_direct(usage, &expr));
             }
-            // cant inline, emit a let statement
+            // can't inline, emit a let statement
             _ => {
                 let ty =
                     sema.type_of_expr(expr).filter(TypeInfo::has_adjustment).and(param_ty.clone());
@@ -421,6 +425,12 @@ fn inline(
     }
 }
 
+fn path_expr_as_record_field(usage: &PathExpr) -> Option<ast::RecordExprField> {
+    let path = usage.path()?;
+    let name_ref = path.as_single_name_ref()?;
+    ast::RecordExprField::for_name_ref(&name_ref)
+}
+
 #[cfg(test)]
 mod tests {
     use crate::tests::{check_assist, check_assist_not_applicable};
@@ -1025,4 +1035,59 @@ fn foo() {
 "#,
         );
     }
+
+    #[test]
+    fn inline_call_field_shorthand() {
+        cov_mark::check!(inline_call_inline_direct_field);
+        check_assist(
+            inline_call,
+            r#"
+struct Foo {
+    field: u32,
+    field1: u32,
+    field2: u32,
+    field3: u32,
+}
+fn foo(field: u32, field1: u32, val2: u32, val3: u32) -> Foo {
+    Foo {
+        field,
+        field1,
+        field2: val2,
+        field3: val3,
+    }
+}
+fn main() {
+    let bar = 0;
+    let baz = 0;
+    foo$0(bar, 0, baz, 0);
+}
+"#,
+            r#"
+struct Foo {
+    field: u32,
+    field1: u32,
+    field2: u32,
+    field3: u32,
+}
+fn foo(field: u32, field1: u32, val2: u32, val3: u32) -> Foo {
+    Foo {
+        field,
+        field1,
+        field2: val2,
+        field3: val3,
+    }
+}
+fn main() {
+    let bar = 0;
+    let baz = 0;
+    Foo {
+            field: bar,
+            field1: 0,
+            field2: baz,
+            field3: 0,
+        };
+}
+"#,
+        );
+    }
 }
diff --git a/crates/ide_db/src/helpers/node_ext.rs b/crates/ide_db/src/helpers/node_ext.rs
index fe823f74f8b..82178ed7496 100644
--- a/crates/ide_db/src/helpers/node_ext.rs
+++ b/crates/ide_db/src/helpers/node_ext.rs
@@ -7,18 +7,16 @@ use syntax::{
 pub fn expr_as_name_ref(expr: &ast::Expr) -> Option<ast::NameRef> {
     if let ast::Expr::PathExpr(expr) = expr {
         let path = expr.path()?;
-        let segment = path.segment()?;
-        let name_ref = segment.name_ref()?;
-        if path.qualifier().is_none() {
-            return Some(name_ref);
-        }
+        path.as_single_name_ref()
+    } else {
+        None
     }
-    None
 }
 
 pub fn block_as_lone_tail(block: &ast::BlockExpr) -> Option<ast::Expr> {
     block.statements().next().is_none().then(|| block.tail_expr()).flatten()
 }
+
 /// Preorder walk all the expression's child expressions.
 pub fn walk_expr(expr: &ast::Expr, cb: &mut dyn FnMut(ast::Expr)) {
     preorder_expr(expr, &mut |ev| {
diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs
index 4b354f7b32e..d271b5f836f 100644
--- a/crates/syntax/src/ast/edit_in_place.rs
+++ b/crates/syntax/src/ast/edit_in_place.rs
@@ -451,6 +451,35 @@ impl ast::RecordExprFieldList {
     }
 }
 
+impl ast::RecordExprField {
+    /// This will either replace the initializer, or in the case that this is a shorthand convert
+    /// the initializer into the name ref and insert the expr as the new initializer.
+    pub fn replace_expr(&self, expr: ast::Expr) {
+        if let Some(_) = self.name_ref() {
+            match self.expr() {
+                Some(prev) => ted::replace(prev.syntax(), expr.syntax()),
+                None => ted::append_child(self.syntax(), expr.syntax()),
+            }
+            return;
+        }
+        // this is a shorthand
+        if let Some(ast::Expr::PathExpr(path_expr)) = self.expr() {
+            if let Some(path) = path_expr.path() {
+                if let Some(name_ref) = path.as_single_name_ref() {
+                    path_expr.syntax().detach();
+                    let children = vec![
+                        name_ref.syntax().clone().into(),
+                        ast::make::token(T![:]).into(),
+                        ast::make::tokens::single_space().into(),
+                        expr.syntax().clone().into(),
+                    ];
+                    ted::insert_all_raw(Position::last_child_of(self.syntax()), children);
+                }
+            }
+        }
+    }
+}
+
 impl ast::StmtList {
     pub fn push_front(&self, statement: ast::Stmt) {
         ted::insert(Position::after(self.l_curly_token().unwrap()), statement.syntax());