Skip to content

Commit 1669d88

Browse files
committed
Add guard support for replace_if_let_with_match
Example --- ```rust fn main() { if $0let true = true && true && false { code() } } ``` ```rust fn main() { match true { true if true && false => code(), _ => (), } } ```
1 parent 1260457 commit 1669d88

File tree

1 file changed

+190
-41
lines changed

1 file changed

+190
-41
lines changed

crates/ide-assists/src/handlers/replace_if_let_with_match.rs

Lines changed: 190 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,10 @@
11
use std::iter::successors;
22

3-
use either::Either;
4-
use ide_db::{
5-
RootDatabase,
6-
defs::NameClass,
7-
syntax_helpers::node_ext::{is_pattern_cond, single_let},
8-
ty_filter::TryEnum,
9-
};
3+
use ide_db::{RootDatabase, defs::NameClass, ty_filter::TryEnum};
104
use syntax::{
11-
AstNode, Edition, T, TextRange,
5+
AstNode, Edition, SyntaxKind, T, TextRange,
126
ast::{self, HasName, edit::IndentLevel, edit_in_place::Indent, syntax_factory::SyntaxFactory},
7+
syntax_editor::SyntaxEditor,
138
};
149

1510
use crate::{
@@ -62,34 +57,30 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext<'
6257
}
6358
});
6459
let scrutinee_to_be_expr = if_expr.condition()?;
65-
let scrutinee_to_be_expr = match single_let(scrutinee_to_be_expr.clone()) {
66-
Some(cond) => cond.expr()?,
67-
None => scrutinee_to_be_expr,
60+
let scrutinee_to_be_expr = match let_and_guard(&scrutinee_to_be_expr) {
61+
(Some(let_expr), _) => let_expr.expr()?,
62+
(None, cond) => cond?,
6863
};
6964

7065
let mut pat_seen = false;
7166
let mut cond_bodies = Vec::new();
7267
for if_expr in if_exprs {
7368
let cond = if_expr.condition()?;
74-
let cond = match single_let(cond.clone()) {
75-
Some(let_) => {
69+
let (cond, guard) = match let_and_guard(&cond) {
70+
(None, guard) => (None, Some(guard?)),
71+
(Some(let_), guard) => {
7672
let pat = let_.pat()?;
7773
let expr = let_.expr()?;
78-
// FIXME: If one `let` is wrapped in parentheses and the second is not,
79-
// we'll exit here.
8074
if scrutinee_to_be_expr.syntax().text() != expr.syntax().text() {
8175
// Only if all condition expressions are equal we can merge them into a match
8276
return None;
8377
}
8478
pat_seen = true;
85-
Either::Left(pat)
79+
(Some(pat), guard)
8680
}
87-
// Multiple `let`, unsupported.
88-
None if is_pattern_cond(cond.clone()) => return None,
89-
None => Either::Right(cond),
9081
};
9182
let body = if_expr.then_branch()?;
92-
cond_bodies.push((cond, body));
83+
cond_bodies.push((cond, guard, body));
9384
}
9485

9586
if !pat_seen && cond_bodies.len() != 1 {
@@ -108,22 +99,21 @@ pub(crate) fn replace_if_let_with_match(acc: &mut Assists, ctx: &AssistContext<'
10899
let make = SyntaxFactory::with_mappings();
109100
let match_expr = {
110101
let else_arm = make_else_arm(ctx, &make, else_block, &cond_bodies);
111-
let make_match_arm = |(pat, body): (_, ast::BlockExpr)| {
112-
let body = make.block_expr(body.statements(), body.tail_expr());
113-
body.indent(IndentLevel::from(1));
114-
let body = unwrap_trivial_block(body);
115-
match pat {
116-
Either::Left(pat) => make.match_arm(pat, None, body),
117-
Either::Right(_) if !pat_seen => {
118-
make.match_arm(make.literal_pat("true").into(), None, body)
102+
let make_match_arm =
103+
|(pat, guard, body): (_, Option<ast::Expr>, ast::BlockExpr)| {
104+
let body = make.block_expr(body.statements(), body.tail_expr());
105+
body.indent(IndentLevel::from(1));
106+
let body = unwrap_trivial_block(body);
107+
match (pat, guard.map(|it| make.match_guard(it))) {
108+
(Some(pat), guard) => make.match_arm(pat, guard, body),
109+
(None, _) if !pat_seen => {
110+
make.match_arm(make.literal_pat("true").into(), None, body)
111+
}
112+
(None, guard) => {
113+
make.match_arm(make.wildcard_pat().into(), guard, body)
114+
}
119115
}
120-
Either::Right(expr) => make.match_arm(
121-
make.wildcard_pat().into(),
122-
Some(make.match_guard(expr)),
123-
body,
124-
),
125-
}
126-
};
116+
};
127117
let arms = cond_bodies.into_iter().map(make_match_arm).chain([else_arm]);
128118
let match_expr = make.expr_match(scrutinee_to_be_expr, make.match_arm_list(arms));
129119
match_expr.indent(IndentLevel::from_node(if_expr.syntax()));
@@ -151,12 +141,12 @@ fn make_else_arm(
151141
ctx: &AssistContext<'_>,
152142
make: &SyntaxFactory,
153143
else_block: Option<ast::BlockExpr>,
154-
conditionals: &[(Either<ast::Pat, ast::Expr>, ast::BlockExpr)],
144+
conditionals: &[(Option<ast::Pat>, Option<ast::Expr>, ast::BlockExpr)],
155145
) -> ast::MatchArm {
156146
let (pattern, expr) = if let Some(else_block) = else_block {
157147
let pattern = match conditionals {
158-
[(Either::Right(_), _)] => make.literal_pat("false").into(),
159-
[(Either::Left(pat), _)] => match ctx
148+
[(None, Some(_), _)] => make.literal_pat("false").into(),
149+
[(Some(pat), _, _)] => match ctx
160150
.sema
161151
.type_of_pat(pat)
162152
.and_then(|ty| TryEnum::from_ty(&ctx.sema, &ty.adjusted()))
@@ -177,7 +167,7 @@ fn make_else_arm(
177167
(pattern, unwrap_trivial_block(else_block))
178168
} else {
179169
let pattern = match conditionals {
180-
[(Either::Right(_), _)] => make.literal_pat("false").into(),
170+
[(None, Some(_), _)] => make.literal_pat("false").into(),
181171
_ => make.wildcard_pat().into(),
182172
};
183173
(pattern, make.expr_unit())
@@ -375,6 +365,48 @@ fn is_sad_pat(sema: &hir::Semantics<'_, RootDatabase>, pat: &ast::Pat) -> bool {
375365
.is_some_and(|it| does_pat_match_variant(pat, &it.sad_pattern()))
376366
}
377367

368+
fn let_and_guard(cond: &ast::Expr) -> (Option<ast::LetExpr>, Option<ast::Expr>) {
369+
if let ast::Expr::ParenExpr(expr) = cond
370+
&& let Some(sub_expr) = expr.expr()
371+
{
372+
let_and_guard(&sub_expr)
373+
} else if let ast::Expr::LetExpr(let_expr) = cond {
374+
(Some(let_expr.clone()), None)
375+
} else if let ast::Expr::BinExpr(bin_expr) = cond
376+
&& let Some(ast::Expr::LetExpr(let_expr)) = and_bin_expr_left(bin_expr).lhs()
377+
{
378+
let new_expr = bin_expr.clone_subtree();
379+
let mut edit = SyntaxEditor::new(new_expr.syntax().clone());
380+
381+
let left_bin = and_bin_expr_left(&new_expr);
382+
if let Some(rhs) = left_bin.rhs() {
383+
edit.replace(left_bin.syntax(), rhs.syntax());
384+
} else {
385+
if let Some(next) = left_bin.syntax().next_sibling_or_token()
386+
&& next.kind() == SyntaxKind::WHITESPACE
387+
{
388+
edit.delete(next);
389+
}
390+
edit.delete(left_bin.syntax());
391+
}
392+
393+
let new_expr = edit.finish().new_root().clone();
394+
(Some(let_expr), ast::Expr::cast(new_expr))
395+
} else {
396+
(None, Some(cond.clone()))
397+
}
398+
}
399+
400+
fn and_bin_expr_left(expr: &ast::BinExpr) -> ast::BinExpr {
401+
if expr.op_kind() == Some(ast::BinaryOp::LogicOp(ast::LogicOp::And))
402+
&& let Some(ast::Expr::BinExpr(left)) = expr.lhs()
403+
{
404+
and_bin_expr_left(&left)
405+
} else {
406+
expr.clone()
407+
}
408+
}
409+
378410
#[cfg(test)]
379411
mod tests {
380412
use super::*;
@@ -507,14 +539,131 @@ impl VariantData {
507539

508540
#[test]
509541
fn test_if_let_with_match_let_chain() {
510-
check_assist_not_applicable(
542+
check_assist(
511543
replace_if_let_with_match,
512544
r#"
545+
#![feature(if_let_guard)]
546+
fn main() {
547+
if $0let true = true && let Some(1) = None {} else { other() }
548+
}
549+
"#,
550+
r#"
551+
#![feature(if_let_guard)]
552+
fn main() {
553+
match true {
554+
true if let Some(1) = None => {
555+
}
556+
_ => other(),
557+
}
558+
}
559+
"#,
560+
);
561+
562+
check_assist(
563+
replace_if_let_with_match,
564+
r#"
565+
#![feature(if_let_guard)]
566+
fn main() {
567+
$0if let ParenExpr(expr) = cond
568+
&& let Some(sub_expr) = expr.expr()
569+
{
570+
branch1()
571+
} else if let LetExpr(let_expr) = cond {
572+
branch2()
573+
} else if let BinExpr(bin_expr) = cond
574+
&& let Some(kind) = bin_expr.op_kind()
575+
&& let Some(LetExpr(let_expr)) = foo(bin_expr)
576+
{
577+
branch3()
578+
} else {
579+
branch4()
580+
}
581+
}
582+
"#,
583+
r#"
584+
#![feature(if_let_guard)]
585+
fn main() {
586+
match cond {
587+
ParenExpr(expr) if let Some(sub_expr) = expr.expr() => branch1(),
588+
LetExpr(let_expr) => branch2(),
589+
BinExpr(bin_expr) if let Some(kind) = bin_expr.op_kind()
590+
&& let Some(LetExpr(let_expr)) = foo(bin_expr) => branch3(),
591+
_ => branch4(),
592+
}
593+
}
594+
"#,
595+
);
596+
597+
check_assist(
598+
replace_if_let_with_match,
599+
r#"
600+
fn main() {
601+
if $0let true = true
602+
&& true
603+
&& false
604+
{
605+
code()
606+
} else {
607+
other()
608+
}
609+
}
610+
"#,
611+
r#"
612+
fn main() {
613+
match true {
614+
true if true
615+
&& false => code(),
616+
_ => other(),
617+
}
618+
}
619+
"#,
620+
);
621+
}
622+
623+
#[test]
624+
fn test_if_let_with_match_let_chain_no_else() {
625+
check_assist(
626+
replace_if_let_with_match,
627+
r#"
628+
#![feature(if_let_guard)]
513629
fn main() {
514630
if $0let true = true && let Some(1) = None {}
515631
}
516632
"#,
517-
)
633+
r#"
634+
#![feature(if_let_guard)]
635+
fn main() {
636+
match true {
637+
true if let Some(1) = None => {
638+
}
639+
_ => (),
640+
}
641+
}
642+
"#,
643+
);
644+
645+
check_assist(
646+
replace_if_let_with_match,
647+
r#"
648+
fn main() {
649+
if $0let true = true
650+
&& true
651+
&& false
652+
{
653+
code()
654+
}
655+
}
656+
"#,
657+
r#"
658+
fn main() {
659+
match true {
660+
true if true
661+
&& false => code(),
662+
_ => (),
663+
}
664+
}
665+
"#,
666+
);
518667
}
519668

520669
#[test]

0 commit comments

Comments
 (0)