3 files changed, 145 insertions, 70 deletions

diff --git a/crates/assists/src/handlers/extract_assignment.rs b/crates/assists/src/handlers/pull_assignment_up.rs
index 281cf5d24..560d93e10 100644
--- a/crates/assists/src/handlers/extract_assignment.rs
+++ b/crates/assists/src/handlers/pull_assignment_up.rs
@@ -1,4 +1,3 @@
-use hir::AsName;
 use syntax::{
     ast::{self, edit::AstNodeEdit, make},
     AstNode,
@@ -10,9 +9,9 @@ use crate::{
     AssistId, AssistKind,
 };
 
-// Assist: extract_assignment
+// Assist: pull_assignment_up
 //
-// Extracts variable assigment to outside an if or match statement.
+// Extracts variable assignment to outside an if or match statement.
 //
 // ```
 // fn main() {
@@ -37,16 +36,24 @@ use crate::{
 //     };
 // }
 // ```
-pub(crate) fn extract_assigment(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
-    let name = ctx.find_node_at_offset::<ast::NameRef>()?.as_name();
+pub(crate) fn pull_assignment_up(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+    let assign_expr = ctx.find_node_at_offset::<ast::BinExpr>()?;
+    let name_expr = if assign_expr.op_kind()? == ast::BinOp::Assignment {
+        assign_expr.lhs()?
+    } else {
+        return None;
+    };
 
     let (old_stmt, new_stmt) = if let Some(if_expr) = ctx.find_node_at_offset::<ast::IfExpr>() {
         (
             ast::Expr::cast(if_expr.syntax().to_owned())?,
-            exprify_if(&if_expr, &name)?.indent(if_expr.indent_level()),
+            exprify_if(&if_expr, &ctx.sema, &name_expr)?.indent(if_expr.indent_level()),
         )
     } else if let Some(match_expr) = ctx.find_node_at_offset::<ast::MatchExpr>() {
-        (ast::Expr::cast(match_expr.syntax().to_owned())?, exprify_match(&match_expr, &name)?)
+        (
+            ast::Expr::cast(match_expr.syntax().to_owned())?,
+            exprify_match(&match_expr, &ctx.sema, &name_expr)?,
+        )
     } else {
         return None;
     };
@@ -54,22 +61,26 @@ pub(crate) fn extract_assigment(acc: &mut Assists, ctx: &AssistContext) -> Optio
     let expr_stmt = make::expr_stmt(new_stmt);
 
     acc.add(
-        AssistId("extract_assignment", AssistKind::RefactorExtract),
-        "Extract assignment",
+        AssistId("pull_assignment_up", AssistKind::RefactorExtract),
+        "Pull assignment up",
         old_stmt.syntax().text_range(),
         move |edit| {
-            edit.replace(old_stmt.syntax().text_range(), format!("{} = {};", name, expr_stmt));
+            edit.replace(old_stmt.syntax().text_range(), format!("{} = {};", name_expr, expr_stmt));
         },
     )
 }
 
-fn exprify_match(match_expr: &ast::MatchExpr, name: &hir::Name) -> Option<ast::Expr> {
+fn exprify_match(
+    match_expr: &ast::MatchExpr,
+    sema: &hir::Semantics<ide_db::RootDatabase>,
+    name: &ast::Expr,
+) -> Option<ast::Expr> {
     let new_arm_list = match_expr
         .match_arm_list()?
         .arms()
         .map(|arm| {
             if let ast::Expr::BlockExpr(block) = arm.expr()? {
-                let new_block = exprify_block(&block, name)?.indent(block.indent_level());
+                let new_block = exprify_block(&block, sema, name)?.indent(block.indent_level());
                 Some(arm.replace_descendant(block, new_block))
             } else {
                 None
@@ -82,21 +93,31 @@ fn exprify_match(match_expr: &ast::MatchExpr, name: &hir::Name) -> Option<ast::E
     Some(make::expr_match(match_expr.expr()?, new_arm_list))
 }
 
-fn exprify_if(statement: &ast::IfExpr, name: &hir::Name) -> Option<ast::Expr> {
-    let then_branch = exprify_block(&statement.then_branch()?, name)?;
+fn exprify_if(
+    statement: &ast::IfExpr,
+    sema: &hir::Semantics<ide_db::RootDatabase>,
+    name: &ast::Expr,
+) -> Option<ast::Expr> {
+    let then_branch = exprify_block(&statement.then_branch()?, sema, name)?;
     let else_branch = match statement.else_branch()? {
-        ast::ElseBranch::Block(ref block) => ast::ElseBranch::Block(exprify_block(block, name)?),
+        ast::ElseBranch::Block(ref block) => {
+            ast::ElseBranch::Block(exprify_block(block, sema, name)?)
+        }
         ast::ElseBranch::IfExpr(expr) => {
-            mark::hit!(test_extract_assigment_chained_if);
+            mark::hit!(test_pull_assignment_up_chained_if);
             ast::ElseBranch::IfExpr(ast::IfExpr::cast(
-                exprify_if(&expr, name)?.syntax().to_owned(),
+                exprify_if(&expr, sema, name)?.syntax().to_owned(),
             )?)
         }
     };
     Some(make::expr_if(statement.condition()?, then_branch, Some(else_branch)))
 }
 
-fn exprify_block(block: &ast::BlockExpr, name: &hir::Name) -> Option<ast::BlockExpr> {
+fn exprify_block(
+    block: &ast::BlockExpr,
+    sema: &hir::Semantics<ide_db::RootDatabase>,
+    name: &ast::Expr,
+) -> Option<ast::BlockExpr> {
     if block.expr().is_some() {
         return None;
     }
@@ -106,8 +127,7 @@ fn exprify_block(block: &ast::BlockExpr, name: &hir::Name) -> Option<ast::BlockE
 
     if let ast::Stmt::ExprStmt(stmt) = stmt {
         if let ast::Expr::BinExpr(expr) = stmt.expr()? {
-            if expr.op_kind()? == ast::BinOp::Assignment
-                && &expr.lhs()?.name_ref()?.as_name() == name
+            if expr.op_kind()? == ast::BinOp::Assignment && is_equivalent(sema, &expr.lhs()?, name)
             {
                 // The last statement in the block is an assignment to the name we want
                 return Some(make::block_expr(stmts, Some(expr.rhs()?)));
@@ -117,6 +137,29 @@ fn exprify_block(block: &ast::BlockExpr, name: &hir::Name) -> Option<ast::BlockE
     None
 }
 
+fn is_equivalent(
+    sema: &hir::Semantics<ide_db::RootDatabase>,
+    expr0: &ast::Expr,
+    expr1: &ast::Expr,
+) -> bool {
+    match (expr0, expr1) {
+        (ast::Expr::FieldExpr(field_expr0), ast::Expr::FieldExpr(field_expr1)) => {
+            mark::hit!(test_pull_assignment_up_field_assignment);
+            sema.resolve_field(field_expr0) == sema.resolve_field(field_expr1)
+        }
+        (ast::Expr::PathExpr(path0), ast::Expr::PathExpr(path1)) => {
+            let path0 = path0.path();
+            let path1 = path1.path();
+            if let (Some(path0), Some(path1)) = (path0, path1) {
+                sema.resolve_path(&path0) == sema.resolve_path(&path1)
+            } else {
+                false
+            }
+        }
+        _ => false,
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -124,9 +167,9 @@ mod tests {
     use crate::tests::{check_assist, check_assist_not_applicable};
 
     #[test]
-    fn test_extract_assignment_if() {
+    fn test_pull_assignment_up_if() {
         check_assist(
-            extract_assigment,
+            pull_assignment_up,
             r#"
 fn foo() {
     let mut a = 1;
@@ -151,9 +194,9 @@ fn foo() {
     }
 
     #[test]
-    fn test_extract_assignment_match() {
+    fn test_pull_assignment_up_match() {
         check_assist(
-            extract_assigment,
+            pull_assignment_up,
             r#"
 fn foo() {
     let mut a = 1;
@@ -190,9 +233,9 @@ fn foo() {
     }
 
     #[test]
-    fn test_extract_assignment_not_last_not_applicable() {
+    fn test_pull_assignment_up_not_last_not_applicable() {
         check_assist_not_applicable(
-            extract_assigment,
+            pull_assignment_up,
             r#"
 fn foo() {
     let mut a = 1;
@@ -208,10 +251,10 @@ fn foo() {
     }
 
     #[test]
-    fn test_extract_assignment_chained_if() {
-        mark::check!(test_extract_assigment_chained_if);
+    fn test_pull_assignment_up_chained_if() {
+        mark::check!(test_pull_assignment_up_chained_if);
         check_assist(
-            extract_assigment,
+            pull_assignment_up,
             r#"
 fn foo() {
     let mut a = 1;
@@ -240,9 +283,9 @@ fn foo() {
     }
 
     #[test]
-    fn test_extract_assigment_retains_stmts() {
+    fn test_pull_assignment_up_retains_stmts() {
         check_assist(
-            extract_assigment,
+            pull_assignment_up,
             r#"
 fn foo() {
     let mut a = 1;
@@ -271,9 +314,9 @@ fn foo() {
     }
 
     #[test]
-    fn extract_assignment_let_stmt_not_applicable() {
+    fn pull_assignment_up_let_stmt_not_applicable() {
         check_assist_not_applicable(
-            extract_assigment,
+            pull_assignment_up,
             r#"
 fn foo() {
     let mut a = 1;
@@ -288,9 +331,9 @@ fn foo() {
     }
 
     #[test]
-    fn extract_assignment_if_missing_assigment_not_applicable() {
+    fn pull_assignment_up_if_missing_assigment_not_applicable() {
         check_assist_not_applicable(
-            extract_assigment,
+            pull_assignment_up,
             r#"
 fn foo() {
     let mut a = 1;
@@ -303,9 +346,9 @@ fn foo() {
     }
 
     #[test]
-    fn extract_assignment_match_missing_assigment_not_applicable() {
+    fn pull_assignment_up_match_missing_assigment_not_applicable() {
         check_assist_not_applicable(
-            extract_assigment,
+            pull_assignment_up,
             r#"
 fn foo() {
     let mut a = 1;
@@ -322,4 +365,36 @@ fn foo() {
 }"#,
         )
     }
+
+    #[test]
+    fn test_pull_assignment_up_field_assignment() {
+        mark::check!(test_pull_assignment_up_field_assignment);
+        check_assist(
+            pull_assignment_up,
+            r#"
+struct A(usize);
+
+fn foo() {
+    let mut a = A(1);
+
+    if true {
+        <|>a.0 = 2;
+    } else {
+        a.0 = 3;
+    }
+}"#,
+            r#"
+struct A(usize);
+
+fn foo() {
+    let mut a = A(1);
+
+    a.0 = if true {
+        2
+    } else {
+        3
+    };
+}"#,
+        )
+    }
 }
diff --git a/crates/assists/src/lib.rs b/crates/assists/src/lib.rs
index 212464f85..01baa65fe 100644
--- a/crates/assists/src/lib.rs
+++ b/crates/assists/src/lib.rs
@@ -116,7 +116,6 @@ mod handlers {
     mod convert_integer_literal;
     mod early_return;
     mod expand_glob_import;
-    mod extract_assignment;
     mod extract_module_to_file;
     mod extract_struct_from_enum_variant;
     mod extract_variable;
@@ -125,8 +124,8 @@ mod handlers {
     mod flip_binexpr;
     mod flip_comma;
     mod flip_trait_bound;
-    mod generate_derive;
     mod generate_default_from_enum_variant;
+    mod generate_derive;
     mod generate_from_impl_for_enum;
     mod generate_function;
     mod generate_impl;
@@ -139,6 +138,7 @@ mod handlers {
     mod merge_match_arms;
     mod move_bounds;
     mod move_guard;
+    mod pull_assignment_up;
     mod qualify_path;
     mod raw_string;
     mod remove_dbg;
@@ -168,7 +168,6 @@ mod handlers {
             convert_integer_literal::convert_integer_literal,
             early_return::convert_to_guarded_return,
             expand_glob_import::expand_glob_import,
-            extract_assignment::extract_assigment,
             extract_module_to_file::extract_module_to_file,
             extract_struct_from_enum_variant::extract_struct_from_enum_variant,
             extract_variable::extract_variable,
@@ -177,8 +176,8 @@ mod handlers {
             flip_binexpr::flip_binexpr,
             flip_comma::flip_comma,
             flip_trait_bound::flip_trait_bound,
-            generate_derive::generate_derive,
             generate_default_from_enum_variant::generate_default_from_enum_variant,
+            generate_derive::generate_derive,
             generate_from_impl_for_enum::generate_from_impl_for_enum,
             generate_function::generate_function,
             generate_impl::generate_impl,
@@ -192,6 +191,7 @@ mod handlers {
             move_bounds::move_bounds_to_where_clause,
             move_guard::move_arm_cond_to_match_guard,
             move_guard::move_guard_to_arm_body,
+            pull_assignment_up::pull_assignment_up,
             qualify_path::qualify_path,
             raw_string::add_hash,
             raw_string::make_usual_string,
diff --git a/crates/assists/src/tests/generated.rs b/crates/assists/src/tests/generated.rs
index b91a816e8..85e3c6742 100644
--- a/crates/assists/src/tests/generated.rs
+++ b/crates/assists/src/tests/generated.rs
@@ -238,35 +238,6 @@ fn qux(bar: Bar, baz: Baz) {}
 }
 
 #[test]
-fn doctest_extract_assignment() {
-    check_doc_test(
-        "extract_assignment",
-        r#####"
-fn main() {
-    let mut foo = 6;
-
-    if true {
-        <|>foo = 5;
-    } else {
-        foo = 4;
-    }
-}
-"#####,
-        r#####"
-fn main() {
-    let mut foo = 6;
-
-    foo = if true {
-        5
-    } else {
-        4
-    };
-}
-"#####,
-    )
-}
-
-#[test]
 fn doctest_extract_module_to_file() {
     check_doc_test(
         "extract_module_to_file",
@@ -767,6 +738,35 @@ fn handle(action: Action) {
 }
 
 #[test]
+fn doctest_pull_assignment_up() {
+    check_doc_test(
+        "pull_assignment_up",
+        r#####"
+fn main() {
+    let mut foo = 6;
+
+    if true {
+        <|>foo = 5;
+    } else {
+        foo = 4;
+    }
+}
+"#####,
+        r#####"
+fn main() {
+    let mut foo = 6;
+
+    foo = if true {
+        5
+    } else {
+        4
+    };
+}
+"#####,
+    )
+}
+
+#[test]
 fn doctest_qualify_path() {
     check_doc_test(
         "qualify_path",