Skip to content

Commit 0f18d92

Browse files
committed
Add else-block support for convert_to_guarded_return
Add support for else-block of never-type for `convert_to_guarded_return` Example --- ```rust fn main() { if$0 let Ok(x) = Err(92) { foo(x); } else { return } } ``` **Before this PR**: Assist not applicable **After this PR**: ```rust fn main() { let Ok(x) = Err(92) else { return }; foo(x); } ```
1 parent a56e577 commit 0f18d92

File tree

3 files changed

+86
-44
lines changed

3 files changed

+86
-44
lines changed

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

Lines changed: 69 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use std::iter::once;
2-
31
use either::Either;
42
use hir::{Semantics, TypeInfo};
53
use ide_db::{RootDatabase, ty_filter::TryEnum};
@@ -17,7 +15,7 @@ use syntax::{
1715
use crate::{
1816
AssistId,
1917
assist_context::{AssistContext, Assists},
20-
utils::invert_boolean_expression_legacy,
18+
utils::{invert_boolean_expression_legacy, is_never_block},
2119
};
2220

2321
// Assist: convert_to_guarded_return
@@ -35,9 +33,7 @@ use crate::{
3533
// ->
3634
// ```
3735
// fn main() {
38-
// if !cond {
39-
// return;
40-
// }
36+
// if !cond { return }
4137
// foo();
4238
// bar();
4339
// }
@@ -54,9 +50,13 @@ fn if_expr_to_guarded_return(
5450
acc: &mut Assists,
5551
ctx: &AssistContext<'_>,
5652
) -> Option<()> {
57-
if if_expr.else_branch().is_some() {
58-
return None;
59-
}
53+
let else_block = match if_expr.else_branch() {
54+
Some(ast::ElseBranch::Block(block_expr)) if is_never_block(&ctx.sema, &block_expr) => {
55+
Some(block_expr.into())
56+
}
57+
Some(_) => return None,
58+
_ => None,
59+
};
6060

6161
let cond = if_expr.condition()?;
6262

@@ -96,7 +96,11 @@ fn if_expr_to_guarded_return(
9696

9797
let parent_container = parent_block.syntax().parent()?;
9898

99-
let early_expression: ast::Expr = early_expression(parent_container, &ctx.sema)?;
99+
let early_expression = else_block
100+
.or_else(|| {
101+
early_expression(parent_container, &ctx.sema).map(ast::make::tail_only_block_expr)
102+
})?
103+
.reset_indent();
100104

101105
then_block.syntax().first_child_or_token().map(|t| t.kind() == T!['{'])?;
102106

@@ -123,21 +127,14 @@ fn if_expr_to_guarded_return(
123127
&& let (Some(pat), Some(expr)) = (let_expr.pat(), let_expr.expr())
124128
{
125129
// If-let.
126-
let let_else_stmt = make::let_else_stmt(
127-
pat,
128-
None,
129-
expr,
130-
ast::make::tail_only_block_expr(early_expression.clone()),
131-
);
130+
let let_else_stmt =
131+
make::let_else_stmt(pat, None, expr, early_expression.clone());
132132
let let_else_stmt = let_else_stmt.indent(if_indent_level);
133133
let_else_stmt.syntax().clone()
134134
} else {
135135
// If.
136136
let new_expr = {
137-
let then_branch = make::block_expr(
138-
once(make::expr_stmt(early_expression.clone()).into()),
139-
None,
140-
);
137+
let then_branch = early_expression.clone();
141138
let cond = invert_boolean_expression_legacy(expr);
142139
make::expr_if(cond, then_branch, None).indent(if_indent_level)
143140
};
@@ -296,9 +293,7 @@ fn main() {
296293
r#"
297294
fn main() {
298295
bar();
299-
if false {
300-
return;
301-
}
296+
if false { return }
302297
foo();
303298
304299
// comment
@@ -327,9 +322,7 @@ fn ret_option() -> Option<()> {
327322
r#"
328323
fn ret_option() -> Option<()> {
329324
bar();
330-
if false {
331-
return None;
332-
}
325+
if false { return None }
333326
foo();
334327
335328
// comment
@@ -360,9 +353,7 @@ fn main() {
360353
fn main() {
361354
let _f = || {
362355
bar();
363-
if false {
364-
return;
365-
}
356+
if false { return }
366357
foo();
367358
368359
// comment
@@ -421,6 +412,51 @@ fn main() {
421412
);
422413
}
423414

415+
#[test]
416+
fn convert_if_let_has_never_type_else_block() {
417+
check_assist(
418+
convert_to_guarded_return,
419+
r#"
420+
fn main() {
421+
if$0 let Ok(x) = Err(92) {
422+
foo(x);
423+
} else {
424+
return ;
425+
}
426+
}
427+
"#,
428+
r#"
429+
fn main() {
430+
let Ok(x) = Err(92) else {
431+
return ;
432+
};
433+
foo(x);
434+
}
435+
"#,
436+
);
437+
438+
check_assist(
439+
convert_to_guarded_return,
440+
r#"
441+
fn main() {
442+
if$0 let Ok(x) = Err(92) {
443+
foo(x);
444+
} else {
445+
return
446+
}
447+
}
448+
"#,
449+
r#"
450+
fn main() {
451+
let Ok(x) = Err(92) else {
452+
return
453+
};
454+
foo(x);
455+
}
456+
"#,
457+
);
458+
}
459+
424460
#[test]
425461
fn convert_if_let_result_inside_let() {
426462
check_assist(
@@ -462,9 +498,7 @@ fn main() {
462498
r#"
463499
fn main() {
464500
let Ok(x) = Err(92) else { return };
465-
if x >= 30 {
466-
return;
467-
}
501+
if x >= 30 { return }
468502
let Some(y) = Some(8) else { return };
469503
foo(x, y);
470504
}
@@ -487,9 +521,7 @@ fn main() {
487521
r#"
488522
fn main() {
489523
let Ok(x) = Err(92) else { return };
490-
if !(x < 30 && y < 20) {
491-
return;
492-
}
524+
if !(x < 30 && y < 20) { return }
493525
let Some(y) = Some(8) else { return };
494526
foo(x, y);
495527
}
@@ -598,9 +630,7 @@ fn main() {
598630
r#"
599631
fn main() {
600632
while true {
601-
if false {
602-
continue;
603-
}
633+
if false { continue }
604634
foo();
605635
bar();
606636
}
@@ -652,9 +682,7 @@ fn main() {
652682
r#"
653683
fn main() {
654684
loop {
655-
if false {
656-
continue;
657-
}
685+
if false { continue }
658686
foo();
659687
bar();
660688
}

crates/ide-assists/src/tests/generated.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -726,9 +726,7 @@ fn main() {
726726
"#####,
727727
r#####"
728728
fn main() {
729-
if !cond {
730-
return;
731-
}
729+
if !cond { return }
732730
foo();
733731
bar();
734732
}

crates/ide-assists/src/utils.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,3 +1150,19 @@ pub fn is_body_const(sema: &Semantics<'_, RootDatabase>, expr: &ast::Expr) -> bo
11501150
});
11511151
is_const
11521152
}
1153+
1154+
// FIXME: #20460 When hir-ty can analyze the `never` statement at the end of block, remove it
1155+
pub(crate) fn is_never_block(
1156+
sema: &Semantics<'_, RootDatabase>,
1157+
block_expr: &ast::BlockExpr,
1158+
) -> bool {
1159+
if let Some(tail_expr) = block_expr.tail_expr() {
1160+
sema.type_of_expr(&tail_expr).is_some_and(|ty| ty.original.is_never())
1161+
} else if let Some(ast::Stmt::ExprStmt(expr_stmt)) = block_expr.statements().last()
1162+
&& let Some(expr) = expr_stmt.expr()
1163+
{
1164+
sema.type_of_expr(&expr).is_some_and(|ty| ty.original.is_never())
1165+
} else {
1166+
return false;
1167+
}
1168+
}

0 commit comments

Comments
 (0)