From 46afdb6e9b50f1493ca92905fa4ae858fb496c7a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 4 Jan 2021 16:38:34 +0300 Subject: rename exrtract_assignment -> pull Vertical code motions are conventionally called "pull up" / "push down". "extract" is used for introducing new names. --- crates/assists/src/handlers/extract_assignment.rs | 400 ---------------------- crates/assists/src/handlers/pull_assignment_up.rs | 400 ++++++++++++++++++++++ crates/assists/src/lib.rs | 8 +- crates/assists/src/tests/generated.rs | 58 ++-- 4 files changed, 433 insertions(+), 433 deletions(-) delete mode 100644 crates/assists/src/handlers/extract_assignment.rs create mode 100644 crates/assists/src/handlers/pull_assignment_up.rs (limited to 'crates/assists') diff --git a/crates/assists/src/handlers/extract_assignment.rs b/crates/assists/src/handlers/extract_assignment.rs deleted file mode 100644 index ae99598c0..000000000 --- a/crates/assists/src/handlers/extract_assignment.rs +++ /dev/null @@ -1,400 +0,0 @@ -use syntax::{ - ast::{self, edit::AstNodeEdit, make}, - AstNode, -}; -use test_utils::mark; - -use crate::{ - assist_context::{AssistContext, Assists}, - AssistId, AssistKind, -}; - -// Assist: extract_assignment -// -// Extracts variable assigment to outside an if or match statement. -// -// ``` -// fn main() { -// let mut foo = 6; -// -// if true { -// <|>foo = 5; -// } else { -// foo = 4; -// } -// } -// ``` -// -> -// ``` -// fn main() { -// let mut foo = 6; -// -// foo = if true { -// 5 -// } else { -// 4 -// }; -// } -// ``` -pub(crate) fn extract_assigment(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { - let assign_expr = ctx.find_node_at_offset::()?; - 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::Expr::cast(if_expr.syntax().to_owned())?, - 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::Expr::cast(match_expr.syntax().to_owned())?, - exprify_match(&match_expr, &ctx.sema, &name_expr)?, - ) - } else { - return None; - }; - - let expr_stmt = make::expr_stmt(new_stmt); - - acc.add( - AssistId("extract_assignment", AssistKind::RefactorExtract), - "Extract assignment", - old_stmt.syntax().text_range(), - move |edit| { - edit.replace(old_stmt.syntax().text_range(), format!("{} = {};", name_expr, expr_stmt)); - }, - ) -} - -fn exprify_match( - match_expr: &ast::MatchExpr, - sema: &hir::Semantics, - name: &ast::Expr, -) -> Option { - 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, sema, name)?.indent(block.indent_level()); - Some(arm.replace_descendant(block, new_block)) - } else { - None - } - }) - .collect::>>()?; - let new_arm_list = match_expr - .match_arm_list()? - .replace_descendants(match_expr.match_arm_list()?.arms().zip(new_arm_list)); - Some(make::expr_match(match_expr.expr()?, new_arm_list)) -} - -fn exprify_if( - statement: &ast::IfExpr, - sema: &hir::Semantics, - name: &ast::Expr, -) -> Option { - 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, sema, name)?) - } - ast::ElseBranch::IfExpr(expr) => { - mark::hit!(test_extract_assigment_chained_if); - ast::ElseBranch::IfExpr(ast::IfExpr::cast( - 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, - sema: &hir::Semantics, - name: &ast::Expr, -) -> Option { - if block.expr().is_some() { - return None; - } - - let mut stmts: Vec<_> = block.statements().collect(); - let stmt = stmts.pop()?; - - if let ast::Stmt::ExprStmt(stmt) = stmt { - if let ast::Expr::BinExpr(expr) = stmt.expr()? { - 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()?))); - } - } - } - None -} - -fn is_equivalent( - sema: &hir::Semantics, - expr0: &ast::Expr, - expr1: &ast::Expr, -) -> bool { - match (expr0, expr1) { - (ast::Expr::FieldExpr(field_expr0), ast::Expr::FieldExpr(field_expr1)) => { - mark::hit!(test_extract_assignment_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::*; - - use crate::tests::{check_assist, check_assist_not_applicable}; - - #[test] - fn test_extract_assignment_if() { - check_assist( - extract_assigment, - r#" -fn foo() { - let mut a = 1; - - if true { - <|>a = 2; - } else { - a = 3; - } -}"#, - r#" -fn foo() { - let mut a = 1; - - a = if true { - 2 - } else { - 3 - }; -}"#, - ); - } - - #[test] - fn test_extract_assignment_match() { - check_assist( - extract_assigment, - r#" -fn foo() { - let mut a = 1; - - match 1 { - 1 => { - <|>a = 2; - }, - 2 => { - a = 3; - }, - 3 => { - a = 4; - } - } -}"#, - r#" -fn foo() { - let mut a = 1; - - a = match 1 { - 1 => { - 2 - }, - 2 => { - 3 - }, - 3 => { - 4 - } - }; -}"#, - ); - } - - #[test] - fn test_extract_assignment_not_last_not_applicable() { - check_assist_not_applicable( - extract_assigment, - r#" -fn foo() { - let mut a = 1; - - if true { - <|>a = 2; - b = a; - } else { - a = 3; - } -}"#, - ) - } - - #[test] - fn test_extract_assignment_chained_if() { - mark::check!(test_extract_assigment_chained_if); - check_assist( - extract_assigment, - r#" -fn foo() { - let mut a = 1; - - if true { - <|>a = 2; - } else if false { - a = 3; - } else { - a = 4; - } -}"#, - r#" -fn foo() { - let mut a = 1; - - a = if true { - 2 - } else if false { - 3 - } else { - 4 - }; -}"#, - ); - } - - #[test] - fn test_extract_assigment_retains_stmts() { - check_assist( - extract_assigment, - r#" -fn foo() { - let mut a = 1; - - if true { - let b = 2; - <|>a = 2; - } else { - let b = 3; - a = 3; - } -}"#, - r#" -fn foo() { - let mut a = 1; - - a = if true { - let b = 2; - 2 - } else { - let b = 3; - 3 - }; -}"#, - ) - } - - #[test] - fn extract_assignment_let_stmt_not_applicable() { - check_assist_not_applicable( - extract_assigment, - r#" -fn foo() { - let mut a = 1; - - let b = if true { - <|>a = 2 - } else { - a = 3 - }; -}"#, - ) - } - - #[test] - fn extract_assignment_if_missing_assigment_not_applicable() { - check_assist_not_applicable( - extract_assigment, - r#" -fn foo() { - let mut a = 1; - - if true { - <|>a = 2; - } else {} -}"#, - ) - } - - #[test] - fn extract_assignment_match_missing_assigment_not_applicable() { - check_assist_not_applicable( - extract_assigment, - r#" -fn foo() { - let mut a = 1; - - match 1 { - 1 => { - <|>a = 2; - }, - 2 => { - a = 3; - }, - 3 => {}, - } -}"#, - ) - } - - #[test] - fn test_extract_assignment_field_assignment() { - mark::check!(test_extract_assignment_field_assignment); - check_assist( - extract_assigment, - 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/handlers/pull_assignment_up.rs b/crates/assists/src/handlers/pull_assignment_up.rs new file mode 100644 index 000000000..560d93e10 --- /dev/null +++ b/crates/assists/src/handlers/pull_assignment_up.rs @@ -0,0 +1,400 @@ +use syntax::{ + ast::{self, edit::AstNodeEdit, make}, + AstNode, +}; +use test_utils::mark; + +use crate::{ + assist_context::{AssistContext, Assists}, + AssistId, AssistKind, +}; + +// Assist: pull_assignment_up +// +// Extracts variable assignment to outside an if or match statement. +// +// ``` +// fn main() { +// let mut foo = 6; +// +// if true { +// <|>foo = 5; +// } else { +// foo = 4; +// } +// } +// ``` +// -> +// ``` +// fn main() { +// let mut foo = 6; +// +// foo = if true { +// 5 +// } else { +// 4 +// }; +// } +// ``` +pub(crate) fn pull_assignment_up(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let assign_expr = ctx.find_node_at_offset::()?; + 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::Expr::cast(if_expr.syntax().to_owned())?, + 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::Expr::cast(match_expr.syntax().to_owned())?, + exprify_match(&match_expr, &ctx.sema, &name_expr)?, + ) + } else { + return None; + }; + + let expr_stmt = make::expr_stmt(new_stmt); + + acc.add( + 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, expr_stmt)); + }, + ) +} + +fn exprify_match( + match_expr: &ast::MatchExpr, + sema: &hir::Semantics, + name: &ast::Expr, +) -> Option { + 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, sema, name)?.indent(block.indent_level()); + Some(arm.replace_descendant(block, new_block)) + } else { + None + } + }) + .collect::>>()?; + let new_arm_list = match_expr + .match_arm_list()? + .replace_descendants(match_expr.match_arm_list()?.arms().zip(new_arm_list)); + Some(make::expr_match(match_expr.expr()?, new_arm_list)) +} + +fn exprify_if( + statement: &ast::IfExpr, + sema: &hir::Semantics, + name: &ast::Expr, +) -> Option { + 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, sema, name)?) + } + ast::ElseBranch::IfExpr(expr) => { + mark::hit!(test_pull_assignment_up_chained_if); + ast::ElseBranch::IfExpr(ast::IfExpr::cast( + 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, + sema: &hir::Semantics, + name: &ast::Expr, +) -> Option { + if block.expr().is_some() { + return None; + } + + let mut stmts: Vec<_> = block.statements().collect(); + let stmt = stmts.pop()?; + + if let ast::Stmt::ExprStmt(stmt) = stmt { + if let ast::Expr::BinExpr(expr) = stmt.expr()? { + 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()?))); + } + } + } + None +} + +fn is_equivalent( + sema: &hir::Semantics, + 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::*; + + use crate::tests::{check_assist, check_assist_not_applicable}; + + #[test] + fn test_pull_assignment_up_if() { + check_assist( + pull_assignment_up, + r#" +fn foo() { + let mut a = 1; + + if true { + <|>a = 2; + } else { + a = 3; + } +}"#, + r#" +fn foo() { + let mut a = 1; + + a = if true { + 2 + } else { + 3 + }; +}"#, + ); + } + + #[test] + fn test_pull_assignment_up_match() { + check_assist( + pull_assignment_up, + r#" +fn foo() { + let mut a = 1; + + match 1 { + 1 => { + <|>a = 2; + }, + 2 => { + a = 3; + }, + 3 => { + a = 4; + } + } +}"#, + r#" +fn foo() { + let mut a = 1; + + a = match 1 { + 1 => { + 2 + }, + 2 => { + 3 + }, + 3 => { + 4 + } + }; +}"#, + ); + } + + #[test] + fn test_pull_assignment_up_not_last_not_applicable() { + check_assist_not_applicable( + pull_assignment_up, + r#" +fn foo() { + let mut a = 1; + + if true { + <|>a = 2; + b = a; + } else { + a = 3; + } +}"#, + ) + } + + #[test] + fn test_pull_assignment_up_chained_if() { + mark::check!(test_pull_assignment_up_chained_if); + check_assist( + pull_assignment_up, + r#" +fn foo() { + let mut a = 1; + + if true { + <|>a = 2; + } else if false { + a = 3; + } else { + a = 4; + } +}"#, + r#" +fn foo() { + let mut a = 1; + + a = if true { + 2 + } else if false { + 3 + } else { + 4 + }; +}"#, + ); + } + + #[test] + fn test_pull_assignment_up_retains_stmts() { + check_assist( + pull_assignment_up, + r#" +fn foo() { + let mut a = 1; + + if true { + let b = 2; + <|>a = 2; + } else { + let b = 3; + a = 3; + } +}"#, + r#" +fn foo() { + let mut a = 1; + + a = if true { + let b = 2; + 2 + } else { + let b = 3; + 3 + }; +}"#, + ) + } + + #[test] + fn pull_assignment_up_let_stmt_not_applicable() { + check_assist_not_applicable( + pull_assignment_up, + r#" +fn foo() { + let mut a = 1; + + let b = if true { + <|>a = 2 + } else { + a = 3 + }; +}"#, + ) + } + + #[test] + fn pull_assignment_up_if_missing_assigment_not_applicable() { + check_assist_not_applicable( + pull_assignment_up, + r#" +fn foo() { + let mut a = 1; + + if true { + <|>a = 2; + } else {} +}"#, + ) + } + + #[test] + fn pull_assignment_up_match_missing_assigment_not_applicable() { + check_assist_not_applicable( + pull_assignment_up, + r#" +fn foo() { + let mut a = 1; + + match 1 { + 1 => { + <|>a = 2; + }, + 2 => { + a = 3; + }, + 3 => {}, + } +}"#, + ) + } + + #[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 @@ -237,35 +237,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( @@ -766,6 +737,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( -- cgit v1.2.3