Skip to content

feat: supports multi arms for replace_match_with_if_let #22231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
A4-Tacks wants to merge 1 commit into
base: master
Choose a base branch
from
+130 −74
Open

feat: supports multi arms for replace_match_with_if_let #22231

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
204 changes: 130 additions & 74 deletions crates/ide-assists/src/handlers/replace_if_let_with_match.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::iter::successors;

use ide_db::{RootDatabase, defs::NameClass, ty_filter::TryEnum};
use syntax::{
AstNode, Edition, SyntaxKind, T, TextRange,
AstNode, SyntaxKind, T, TextRange,
ast::{
self, HasName,
edit::{AstNodeEdit, IndentLevel},
Expand Down Expand Up @@ -235,28 +235,19 @@ pub(crate) fn replace_match_with_if_let(acc: &mut Assists, ctx: &AssistContext<'
return None;
}

let mut arms = match_arm_list.arms();
let (first_arm, second_arm) = (arms.next()?, arms.next()?);
if arms.next().is_some() || second_arm.guard().is_some() {
return None;
}
if first_arm.guard().is_some() && ctx.edition() < Edition::Edition2024 {
let arms = match_arm_list
.arms()
.map(|arm| Some((arm.pat()?, arm.expr()?, arm.guard())))
.collect::<Option<Vec<_>>>()?;

if arms.len() < 2 {
return None;
}

let (if_let_pat, guard, then_expr, else_expr) = pick_pattern_and_expr_order(
&ctx.sema,
first_arm.pat()?,
second_arm.pat()?,
first_arm.expr()?,
second_arm.expr()?,
first_arm.guard(),
second_arm.guard(),
)?;
let (if_exprs, else_expr) = pick_pattern_and_expr_order(&ctx.sema, arms)?;
let scrutinee = match_expr.expr()?.reset_indent();
let guard = guard.and_then(|it| it.condition());

let let_ = match &if_let_pat {
let let_ = match &if_exprs.first()?.0 {
ast::Pat::LiteralPat(p)
if p.literal()
.map(|it| it.token().kind())
Expand All @@ -283,36 +274,51 @@ pub(crate) fn replace_match_with_if_let(acc: &mut Assists, ctx: &AssistContext<'
}
};

let condition = match if_let_pat {
ast::Pat::LiteralPat(p)
if p.literal().is_some_and(|it| it.token().kind() == T![true]) =>
{
scrutinee
}
ast::Pat::LiteralPat(p)
if p.literal().is_some_and(|it| it.token().kind() == T![false]) =>
let if_branches = if_exprs.into_iter().map(|(pat, then_expr, guard)| {
let guard = guard.and_then(|it| it.condition());

let condition = match pat {
ast::Pat::LiteralPat(p)
if p.literal().is_some_and(|it| it.token().kind() == T![true]) =>
{
scrutinee.clone()
}
ast::Pat::LiteralPat(p)
if p.literal().is_some_and(|it| it.token().kind() == T![false]) =>
{
make.expr_prefix(T![!], scrutinee.clone()).into()
}
ast::Pat::WildcardPat(_) => make.expr_literal("true").into(),
_ => make.expr_let(pat, scrutinee.clone()).into(),
};
let condition = if condition.syntax().text() == "true"
&& let Some(guard) = guard
{
make.expr_prefix(T![!], scrutinee).into()
}
_ => make.expr_let(if_let_pat, scrutinee).into(),
};
let condition = if let Some(guard) = guard {
let guard = wrap_paren(guard, make, ast::prec::ExprPrecedence::LAnd);
make.expr_bin(condition, ast::BinaryOp::LogicOp(ast::LogicOp::And), guard).into()
} else {
condition
};
let then_expr = then_expr.reset_indent();
guard
} else if let Some(guard) = guard {
let guard = wrap_paren(guard, make, ast::prec::ExprPrecedence::LAnd);
make.expr_bin(condition, ast::BinaryOp::LogicOp(ast::LogicOp::And), guard)
.into()
} else {
condition
};

let then_expr = then_expr.reset_indent();
let then_block = make_block_expr(then_expr);

(condition, then_block)
});

let else_expr = else_expr.reset_indent();
let then_block = make_block_expr(then_expr);
let else_expr = if is_empty_expr(&else_expr) { None } else { Some(else_expr) };
let if_let_expr = make
.expr_if(
condition,
then_block,
else_expr.map(make_block_expr).map(ast::ElseBranch::Block),
)
.indent(IndentLevel::from_node(match_expr.syntax()));
let else_branch = else_expr.map(make_block_expr).map(ast::ElseBranch::Block);

let if_let_expr =
if_branches.rfold(else_branch, |else_branch, (condition, then_branch)| {
Some(make.expr_if(condition, then_branch, else_branch).into())
});
let Some(ast::ElseBranch::IfExpr(if_let_expr)) = if_let_expr else { unreachable!() };
let if_let_expr = if_let_expr.indent(match_expr.indent_level());

editor.replace(match_expr.syntax(), if_let_expr.syntax());
builder.add_file_edits(ctx.vfs_file_id(), editor);
Expand All @@ -323,37 +329,33 @@ pub(crate) fn replace_match_with_if_let(acc: &mut Assists, ctx: &AssistContext<'
/// Pick the pattern for the if let condition and return the expressions for the `then` body and `else` body in that order.
fn pick_pattern_and_expr_order(
sema: &hir::Semantics<'_, RootDatabase>,
pat: ast::Pat,
pat2: ast::Pat,
expr: ast::Expr,
expr2: ast::Expr,
guard: Option<ast::MatchGuard>,
guard2: Option<ast::MatchGuard>,
) -> Option<(ast::Pat, Option<ast::MatchGuard>, ast::Expr, ast::Expr)> {
if guard.is_some() && guard2.is_some() {
mut arms: Vec<(ast::Pat, ast::Expr, Option<ast::MatchGuard>)>,
) -> Option<(Vec<(ast::Pat, ast::Expr, Option<ast::MatchGuard>)>, ast::Expr)> {
if arms.last()?.2.is_some() {
return None;
}
let res = match (pat, pat2) {
(ast::Pat::WildcardPat(_), _) => return None,
(pat, ast::Pat::WildcardPat(_)) => (pat, guard, expr, expr2),
(pat, _) if is_empty_expr(&expr2) => (pat, guard, expr, expr2),
(_, pat) if is_empty_expr(&expr) => (pat, guard, expr2, expr),
(pat, pat2) => match (binds_name(sema, &pat), binds_name(sema, &pat2)) {
(true, true) => return None,
(true, false) => (pat, guard, expr, expr2),
(false, true) => {
// This pattern triggers an invalid transformation.
// See issues #11373, #19443
if let ast::Pat::IdentPat(_) = pat2 {
return None;
}
(pat2, guard2, expr2, expr)
}
_ if is_sad_pat(sema, &pat) => (pat2, guard2, expr2, expr),
(false, false) => (pat, guard, expr, expr2),
},
};
Some(res)

if matches!(arms.last()?.0, ast::Pat::WildcardPat(_)) {
let (_, last, _) = arms.pop()?;
return Some((arms, last));
}

if let Some(sad_arm) = arms.iter().rposition(|arm| {
matches!(arm, (_, expr, None) if is_empty_expr(expr)) || is_sad_pat(sema, &arm.0)
}) {
let (_, sad_arm, _) = arms.remove(sad_arm);
return Some((arms, sad_arm));
}

// This pattern triggers an invalid transformation.
// See issues #11373, #19443
// XXX: Perhaps we can extract a let statement to improve
if !binds_name(sema, &arms.last()?.0) {
let (_, last, _) = arms.pop()?;
return Some((arms, last));
}

None
}

fn is_empty_expr(expr: &ast::Expr) -> bool {
Expand Down Expand Up @@ -2448,6 +2450,60 @@ fn main() {
);
}

#[test]
fn test_replace_match_multi_arms_with_if_let() {
check_assist(
replace_match_with_if_let,
r#"
fn main() {
match$0 Some(0) {
Some(n) if n % 2 == 0 && n != 6 => (),
Some(n) if valided(n) => foo(),
_ => code(),
}
}
"#,
r#"
fn main() {
if let Some(n) = Some(0) && (n % 2 == 0 && n != 6) {
()
} else if let Some(n) = Some(0) && valided(n) {
foo()
} else {
code()
}
}
"#,
);
}

#[test]
fn test_replace_match_with_if_let_other_guards() {
check_assist(
replace_match_with_if_let,
r#"
fn main() {
match$0 Some(0) {
Some(n) if n % 2 == 0 && n != 6 => (),
_ if cond => foo(),
_ => code(),
}
}
"#,
r#"
fn main() {
if let Some(n) = Some(0) && (n % 2 == 0 && n != 6) {
()
} else if cond {
foo()
} else {
code()
}
}
"#,
);
}

#[test]
fn test_replace_match_with_if_let_not_applicable_pat2_is_ident_pat() {
check_assist_not_applicable(
Expand Down