Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 16 additions & 0 deletions clippy_lints/src/collapsible_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand All @@ -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>> {
Expand Down
26 changes: 26 additions & 0 deletions tests/ui/collapsible_else_if.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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]
Expand Down
28 changes: 28 additions & 0 deletions tests/ui/collapsible_else_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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]
Expand Down
24 changes: 21 additions & 3 deletions tests/ui/collapsible_else_if.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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{
| __________^
Expand All @@ -151,13 +169,13 @@ 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 {
| ____________^
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

Loading