fix: remove else-if collapse suggestion when all branches contain only an if {..} else {..} expression

If two `if` arms contain only a single `if {..} else {..}` statement,
don't suggest a collapse as this can lead to less readable code.
This commit is contained in:
Ryan Ward 2025-12-22 19:06:46 +10:30
parent 26a5677e16
commit 646f583b3a
4 changed files with 91 additions and 3 deletions

View file

@ -267,6 +267,9 @@ impl LateLintPass<'_> for CollapsibleIf {
&& !expr.span.from_expansion()
{
if let Some(else_) = else_
// Short circuit if both `if` branches contain only a single `if {..} else {}`, as
// collapsing such blocks can lead to less readable code (#4971)
&& !(single_inner_if_else(then) && single_inner_if_else(else_))
&& let ExprKind::Block(else_, None) = else_.kind
{
self.check_collapsible_else_if(cx, then.span, else_);
@ -280,6 +283,19 @@ impl LateLintPass<'_> for CollapsibleIf {
}
}
/// Returns true if `expr` is a block that contains only one `if {..} else {}` statement
fn single_inner_if_else(expr: &Expr<'_>) -> bool {
if let ExprKind::Block(block, None) = expr.kind
&& let Some(inner_expr) = expr_block(block)
&& let ExprKind::If(_, _, else_) = inner_expr.kind
&& else_.is_some()
{
true
} else {
false
}
}
/// If `block` is a block with either one expression or a statement containing an expression,
/// return the expression. We don't peel blocks recursively, as extra blocks might be intentional.
fn expr_block<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> {

View file

@ -70,6 +70,17 @@ fn main() {
}
//~^^^^^^^^ collapsible_else_if
if x == "hello" {
if y == "world" {
print!("Hello ");
} else {
println!("world");
}
} else if let Some(42) = Some(42) {
println!("42");
}
//~^^^^^ collapsible_else_if
if x == "hello" {
print!("Hello ");
} else {
@ -78,6 +89,21 @@ fn main() {
println!("world!")
}
}
if x == "hello" {
if y == "world" {
print!("Hello ");
} else {
println!("world");
}
} else {
if let Some(42) = Some(42) {
println!("42");
} else {
println!("!");
}
}
}
#[rustfmt::skip]

View file

@ -84,6 +84,19 @@ fn main() {
}
//~^^^^^^^^ collapsible_else_if
if x == "hello" {
if y == "world" {
print!("Hello ");
} else {
println!("world");
}
} else {
if let Some(42) = Some(42) {
println!("42");
}
}
//~^^^^^ collapsible_else_if
if x == "hello" {
print!("Hello ");
} else {
@ -92,6 +105,21 @@ fn main() {
println!("world!")
}
}
if x == "hello" {
if y == "world" {
print!("Hello ");
} else {
println!("world");
}
} else {
if let Some(42) = Some(42) {
println!("42");
} else {
println!("!");
}
}
}
#[rustfmt::skip]

View file

@ -142,7 +142,25 @@ LL + }
|
error: this `else { if .. }` block can be collapsed
--> tests/ui/collapsible_else_if.rs:100:10
--> tests/ui/collapsible_else_if.rs:93:12
|
LL | } else {
| ____________^
LL | | if let Some(42) = Some(42) {
LL | | println!("42");
LL | | }
LL | | }
| |_____^
|
help: collapse nested if block
|
LL ~ } else if let Some(42) = Some(42) {
LL + println!("42");
LL + }
|
error: this `else { if .. }` block can be collapsed
--> tests/ui/collapsible_else_if.rs:128:10
|
LL | }else{
| __________^
@ -151,7 +169,7 @@ LL | | }
| |_____^ help: collapse nested if block: `if false {}`
error: this `else { if .. }` block can be collapsed
--> tests/ui/collapsible_else_if.rs:139:12
--> tests/ui/collapsible_else_if.rs:167:12
|
LL | } else {
| ____________^
@ -159,5 +177,5 @@ LL | | (if y == "world" { println!("world") } else { println!("!") })
LL | | }
| |_____^ help: collapse nested if block: `if y == "world" { println!("world") } else { println!("!") }`
error: aborting due to 9 previous errors
error: aborting due to 10 previous errors