From 7fee0881db85600a4a31442555a305d215064283 Mon Sep 17 00:00:00 2001 From: Yu Zeng Date: Fri, 29 Dec 2023 18:32:37 +0800 Subject: [PATCH 1/5] complete merge_nested_if with bugs. --- .../src/handlers/merge_nested_if.rs | 230 ++++++++++++++++++ crates/ide-assists/src/lib.rs | 2 + crates/ide-assists/src/tests/generated.rs | 15 ++ 3 files changed, 247 insertions(+) create mode 100644 crates/ide-assists/src/handlers/merge_nested_if.rs diff --git a/crates/ide-assists/src/handlers/merge_nested_if.rs b/crates/ide-assists/src/handlers/merge_nested_if.rs new file mode 100644 index 000000000000..af99d6fdef0b --- /dev/null +++ b/crates/ide-assists/src/handlers/merge_nested_if.rs @@ -0,0 +1,230 @@ +use ide_db::syntax_helpers::node_ext::is_pattern_cond; +use syntax::{ + ast::{self, AstNode, BinaryOp}, + T, +}; + +use crate::{ + assist_context::{AssistContext, Assists}, + AssistId, AssistKind, +}; +/// Assist: merge_nested_if +/// +/// This transforms if expressions of the form `if x { if y {A} }` into `if x && y {A}` +/// This assist can only be applied with the cursor on `if`. +/// +/// ``` +/// fn main() { +/// i$0f x == 3 { if y == 4 { 1 } } +/// } +/// ``` +/// -> +/// ``` +/// fn main() { +/// if x == 3 && y == 4 { 1 } +/// } +/// ``` +pub(crate) fn merge_nested_if(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { + let if_keyword = ctx.find_token_syntax_at_offset(T![if])?; + let expr = ast::IfExpr::cast(if_keyword.parent()?)?; + let if_range = if_keyword.text_range(); + let cursor_in_range = if_range.contains_range(ctx.selection_trimmed()); + if !cursor_in_range { + return None; + } + + //should not apply to if with else branch. + if expr.else_branch().is_some() { + return None; + } + + let cond = expr.condition()?; + let cond_range = cond.syntax().text_range(); + //should not apply for if-let + if is_pattern_cond(cond.clone()) { + return None; + } + + //check if the then branch is a nested if + let then_branch = expr.then_branch()?; + + let nested_if_to_merge = then_branch.syntax().descendants().find_map(ast::IfExpr::cast)?; + // should not apply to nested if with else branch. + if nested_if_to_merge.else_branch().is_some() { + return None; + } + let nested_if_cond = nested_if_to_merge.condition()?; + if is_pattern_cond(nested_if_cond.clone()) { + return None; + } + + let nested_if_then_branch = nested_if_to_merge.then_branch()?; + let then_branch_range = then_branch.syntax().text_range(); + + acc.add( + AssistId("merge_nested_if", AssistKind::RefactorRewrite), + "Merge nested if", + if_range, + |edit| { + let cond_text = if has_logic_op_or(&cond) { + format!("({})", cond.syntax().text()) + } else { + cond.syntax().text().to_string() + }; + + let nested_if_cond_text = if has_logic_op_or(&nested_if_cond) { + format!("({})", nested_if_cond.syntax().text()) + } else { + nested_if_cond.syntax().text().to_string() + }; + + let replace_cond = format!("{} && {}", cond_text, nested_if_cond_text); + + edit.replace(cond_range, replace_cond); + edit.replace(then_branch_range, nested_if_then_branch.syntax().text()); + }, + ) +} + +/// Returns whether the given if condition has logical operators. +fn has_logic_op_or(expr: &ast::Expr) -> bool { + match expr { + ast::Expr::BinExpr(bin_expr) => { + if let Some(kind) = bin_expr.op_kind() { + matches!(kind, BinaryOp::LogicOp(ast::LogicOp::Or)) + } else { + false + } + } + _ => false, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tests::{check_assist, check_assist_not_applicable}; + + #[test] + fn merge_nested_if_test1() { + check_assist( + merge_nested_if, + "fn f() { i$0f x == 3 { if y == 4 { 1 } } }", + "fn f() { if x == 3 && y == 4 { 1 } }", + ) + } + + #[test] + fn merge_nested_if_test2() { + check_assist( + merge_nested_if, + "fn f() { i$0f x == 3 || y == 1 { if z == 4 { 1 } } }", + "fn f() { if (x == 3 || y == 1) && z == 4 { 1 } }", + ) + } + + #[test] + fn merge_nested_if_test3() { + check_assist( + merge_nested_if, + "fn f() { i$0f x == 3 && y == 1 { if z == 4 { 1 } } }", + "fn f() { if x == 3 && y == 1 && z == 4 { 1 } }", + ) + } + + #[test] + fn merge_nested_if_test4() { + check_assist( + merge_nested_if, + "fn f() { i$0f x == 3 && y == 1 { if z == 4 && q == 3 { 1 } } }", + "fn f() { if x == 3 && y == 1 && z == 4 && q == 3 { 1 } }", + ) + } + + #[test] + fn merge_nested_if_test5() { + check_assist( + merge_nested_if, + "fn f() { i$0f x == 3 && y == 1 { if z == 4 || q == 3 { 1 } } }", + "fn f() { if x == 3 && y == 1 && (z == 4 || q == 3) { 1 } }", + ) + } + + #[test] + fn merge_nested_if_test6() { + check_assist( + merge_nested_if, + "fn f() { i$0f x == 3 || y == 1 { if z == 4 || q == 3 { 1 } } }", + "fn f() { if (x == 3 || y == 1) && (z == 4 || q == 3) { 1 } }", + ) + } + + #[test] + fn merge_nested_if_test7() { + check_assist( + merge_nested_if, + "fn f() { i$0f x == 3 || y == 1 { if z == 4 && q == 3 { 1 } } }", + "fn f() { if (x == 3 || y == 1) && z == 4 && q == 3 { 1 } }", + ) + } + + #[test] + fn merge_nested_if_do_not_apply_to_if_with_else_branch() { + check_assist_not_applicable( + merge_nested_if, + "fn f() { i$0f x == 3 { if y == 4 { 1 } } else { 2 } }", + ) + } + + #[test] + fn merge_nested_if_do_not_apply_to_nested_if_with_else_branch() { + check_assist_not_applicable( + merge_nested_if, + "fn f() { i$0f x == 3 { if y == 4 { 1 } else { 2 } } }", + ) + } + + #[test] + fn merge_nested_if_do_not_apply_to_if_let() { + check_assist_not_applicable( + merge_nested_if, + "fn f() { i$0f let Some(x) = y { if x == 4 { 1 } } }", + ) + } + + #[test] + fn merge_nested_if_do_not_apply_to_nested_if_let() { + check_assist_not_applicable( + merge_nested_if, + "fn f() { i$0f y == 0 { if let Some(x) = y { 1 } } }", + ) + } + + #[test] + fn merge_nested_if_do_not_apply_to_if_with_else_branch_and_nested_if() { + check_assist_not_applicable( + merge_nested_if, + "fn f() { i$0f x == 3 { if y == 4 { 1 } } else { if z == 5 { 2 } } }", + ) + } + + #[test] + fn merge_nested_if_do_not_apply_with_cursor_not_on_if() { + check_assist_not_applicable(merge_nested_if, "fn f() { if $0x==0 { if y == 3 { 1 } } }") + } + + #[test] + fn merge_nested_if_do_not_apply_with_mulpiple_if() { + check_assist_not_applicable( + merge_nested_if, + "fn f() { i$0f x == 0 { if y == 3 { 1 } else if y == 4 { 2 } } }", + ) + } + #[test] + fn merge_nested_if_do_not_apply_with_not_only_has_nested_if(){ + check_assist_not_applicable( + merge_nested_if, + "fn f() { i$0f x == 0 { if y == 3 { foo(); } foo(); } }", + ) + } +} diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs index 1e4d1c94f5be..1eb4903ab203 100644 --- a/crates/ide-assists/src/lib.rs +++ b/crates/ide-assists/src/lib.rs @@ -217,6 +217,7 @@ mod handlers { mod unqualify_method_call; mod wrap_return_type_in_result; mod into_to_qualified_from; + mod merge_nested_if; pub(crate) fn all() -> &'static [Handler] { &[ @@ -291,6 +292,7 @@ mod handlers { invert_if::invert_if, merge_imports::merge_imports, merge_match_arms::merge_match_arms, + merge_nested_if::merge_nested_if, move_bounds::move_bounds_to_where_clause, move_const_to_impl::move_const_to_impl, move_guard::move_arm_cond_to_match_guard, diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index da5822bba9c8..79958a2292fb 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -2051,6 +2051,21 @@ fn handle(action: Action) { ) } +#[test] +fn doctest_merge_nested_if() { + check_doc_test( + "merge_nested_if", + r#####" + fn main() { + i$0f x == 3 { if y == 4 { 1 } } + }"#####, + r#####" + fn main() { + if x == 3 && y == 4 { 1 } + }"#####, + ) +} + #[test] fn doctest_move_arm_cond_to_match_guard() { check_doc_test( From edb9ad21bd8f0a692d100f6d3282e2b442fabc64 Mon Sep 17 00:00:00 2001 From: l1nxy Date: Mon, 1 Jan 2024 21:53:57 +0800 Subject: [PATCH 2/5] apply to only has nested if. --- crates/ide-assists/src/handlers/merge_nested_if.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/ide-assists/src/handlers/merge_nested_if.rs b/crates/ide-assists/src/handlers/merge_nested_if.rs index af99d6fdef0b..39db42faaea4 100644 --- a/crates/ide-assists/src/handlers/merge_nested_if.rs +++ b/crates/ide-assists/src/handlers/merge_nested_if.rs @@ -47,6 +47,10 @@ pub(crate) fn merge_nested_if(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opt //check if the then branch is a nested if let then_branch = expr.then_branch()?; + let stmt = then_branch.stmt_list()?; + if stmt.statements().count() != 0 { + return None; + } let nested_if_to_merge = then_branch.syntax().descendants().find_map(ast::IfExpr::cast)?; // should not apply to nested if with else branch. @@ -221,7 +225,7 @@ mod tests { ) } #[test] - fn merge_nested_if_do_not_apply_with_not_only_has_nested_if(){ + fn merge_nested_if_do_not_apply_with_not_only_has_nested_if() { check_assist_not_applicable( merge_nested_if, "fn f() { i$0f x == 0 { if y == 3 { foo(); } foo(); } }", From b6a14ce5b8d1081ea330a0b85733fc6b646dfa33 Mon Sep 17 00:00:00 2001 From: l1nxy Date: Mon, 1 Jan 2024 22:11:45 +0800 Subject: [PATCH 3/5] fix doc test. --- .../src/handlers/merge_nested_if.rs | 32 +++++++++---------- crates/ide-assists/src/tests/generated.rs | 14 ++++---- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/crates/ide-assists/src/handlers/merge_nested_if.rs b/crates/ide-assists/src/handlers/merge_nested_if.rs index 39db42faaea4..206bc3e08be0 100644 --- a/crates/ide-assists/src/handlers/merge_nested_if.rs +++ b/crates/ide-assists/src/handlers/merge_nested_if.rs @@ -8,22 +8,22 @@ use crate::{ assist_context::{AssistContext, Assists}, AssistId, AssistKind, }; -/// Assist: merge_nested_if -/// -/// This transforms if expressions of the form `if x { if y {A} }` into `if x && y {A}` -/// This assist can only be applied with the cursor on `if`. -/// -/// ``` -/// fn main() { -/// i$0f x == 3 { if y == 4 { 1 } } -/// } -/// ``` -/// -> -/// ``` -/// fn main() { -/// if x == 3 && y == 4 { 1 } -/// } -/// ``` +// Assist: merge_nested_if +// +// This transforms if expressions of the form `if x { if y {A} }` into `if x && y {A}` +// This assist can only be applied with the cursor on `if`. +// +// ``` +// fn main() { +// i$0f x == 3 { if y == 4 { 1 } } +// } +// ``` +// -> +// ``` +// fn main() { +// if x == 3 && y == 4 { 1 } +// } +// ``` pub(crate) fn merge_nested_if(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let if_keyword = ctx.find_token_syntax_at_offset(T![if])?; let expr = ast::IfExpr::cast(if_keyword.parent()?)?; diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index 79958a2292fb..d8b3b32a1aba 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -2056,13 +2056,15 @@ fn doctest_merge_nested_if() { check_doc_test( "merge_nested_if", r#####" - fn main() { - i$0f x == 3 { if y == 4 { 1 } } - }"#####, +fn main() { + i$0f x == 3 { if y == 4 { 1 } } +} +"#####, r#####" - fn main() { - if x == 3 && y == 4 { 1 } - }"#####, +fn main() { + if x == 3 && y == 4 { 1 } +} +"#####, ) } From a3be52cbc0ff88f2e06e117b8e0843531832a37d Mon Sep 17 00:00:00 2001 From: l1nxy Date: Mon, 1 Jan 2024 22:31:04 +0800 Subject: [PATCH 4/5] tidy. --- crates/ide-assists/src/handlers/merge_nested_if.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ide-assists/src/handlers/merge_nested_if.rs b/crates/ide-assists/src/handlers/merge_nested_if.rs index 206bc3e08be0..5742f60a1e35 100644 --- a/crates/ide-assists/src/handlers/merge_nested_if.rs +++ b/crates/ide-assists/src/handlers/merge_nested_if.rs @@ -39,12 +39,13 @@ pub(crate) fn merge_nested_if(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opt } let cond = expr.condition()?; - let cond_range = cond.syntax().text_range(); //should not apply for if-let if is_pattern_cond(cond.clone()) { return None; } + let cond_range = cond.syntax().text_range(); + //check if the then branch is a nested if let then_branch = expr.then_branch()?; let stmt = then_branch.stmt_list()?; From 161d3055d192e36b87bfb0abd2c66f1356ed8681 Mon Sep 17 00:00:00 2001 From: l1nxy Date: Wed, 3 Jan 2024 09:48:49 +0800 Subject: [PATCH 5/5] use tail_expr(). --- crates/ide-assists/src/handlers/merge_nested_if.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/ide-assists/src/handlers/merge_nested_if.rs b/crates/ide-assists/src/handlers/merge_nested_if.rs index 5742f60a1e35..2f3136f027b0 100644 --- a/crates/ide-assists/src/handlers/merge_nested_if.rs +++ b/crates/ide-assists/src/handlers/merge_nested_if.rs @@ -53,7 +53,10 @@ pub(crate) fn merge_nested_if(acc: &mut Assists, ctx: &AssistContext<'_>) -> Opt return None; } - let nested_if_to_merge = then_branch.syntax().descendants().find_map(ast::IfExpr::cast)?; + let nested_if_to_merge = then_branch.tail_expr().and_then(|e| match e { + ast::Expr::IfExpr(e) => Some(e), + _ => None, + })?; // should not apply to nested if with else branch. if nested_if_to_merge.else_branch().is_some() { return None; @@ -232,4 +235,12 @@ mod tests { "fn f() { i$0f x == 0 { if y == 3 { foo(); } foo(); } }", ) } + + #[test] + fn merge_nested_if_do_not_apply_with_multiply_nested_if() { + check_assist_not_applicable( + merge_nested_if, + "fn f() { i$0f x == 0 { if y == 3 { foo(); } if z == 3 { 2 } } }", + ) + } }