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.

Author: rwardd

clippy_lints/src/collapsible_if.rs

@@ -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>> {

tests/ui/collapsible_else_if.fixed

@@ -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]

tests/ui/collapsible_else_if.rs

@@ -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]

tests/ui/collapsible_else_if.stderr

@@ -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,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