single_match_else - single expr/stmt else block corner case

rust-lang · Jul 7, 2020 · c8c286c · c8c286c
1 parent 57cdf2d
commit c8c286c
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 9 deletions.
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
@@ -530,16 +530,22 @@ fn check_single_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], exp
             // the lint noisy in unnecessary situations
             return;
         }
-        let els = remove_blocks(&arms[1].body);
-        let els = if is_unit_expr(els) {
+        let els = arms[1].body;
+        let els = if is_unit_expr(remove_blocks(els)) {
             None
-        } else if let ExprKind::Block(_, _) = els.kind {
-            // matches with blocks that contain statements are prettier as `if let + else`
-            Some(els)
+        } else if let ExprKind::Block(Block { stmts, expr: block_expr, .. }, _) = els.kind {
+            if stmts.len() == 1 && block_expr.is_none() || stmts.is_empty() && block_expr.is_some() {
+                // single statement/expr "else" block, don't lint
+                return;
+            } else {
+                // block with 2+ statements or 1 expr and 1+ statement 
+                Some(els)
+            }
         } else {
-            // allow match arms with just expressions
-            return;
+            // not a block, don't lint  
+            return; 
         };
+
         let ty = cx.tables().expr_ty(ex);
         if ty.kind != ty::Bool || is_allowed(cx, MATCH_BOOL, ex.hir_id) {
             check_single_match_single_pattern(cx, ex, arms, expr, els);

diff --git a/tests/ui/single_match_else.rs b/tests/ui/single_match_else.rs
@@ -1,4 +1,6 @@
 #![warn(clippy::single_match_else)]
+#![allow(clippy::needless_return)]
+#![allow(clippy::no_effect)]
 
 enum ExprNode {
     ExprAddrOf,
@@ -30,6 +32,55 @@ macro_rules! unwrap_addr {
     };
 }
 
+#[rustfmt::skip]
 fn main() {
     unwrap_addr!(ExprNode::Unicorns);
+
+    //
+    // don't lint single exprs/statements
+    //
+
+    // don't lint here
+    match Some(1) {
+        Some(a) => println!("${:?}", a),
+        None => return,
+    }
+
+    // don't lint here
+    match Some(1) {
+        Some(a) => println!("${:?}", a),
+        None => {
+            return
+        },
+    }
+
+    // don't lint here
+    match Some(1) {
+        Some(a) => println!("${:?}", a),
+        None => {
+            return;
+        },
+    }
+
+    //
+    // lint multiple exprs/statements "else" blocks
+    //
+
+    // lint here
+    match Some(1) {
+        Some(a) => println!("${:?}", a),
+        None => {
+            println!("else block");
+            return
+        },
+    }
+
+    // lint here
+    match Some(1) {
+        Some(a) => println!("${:?}", a),
+        None => {
+            println!("else block");
+            return;
+        },
+    }
 }
diff --git a/tests/ui/single_match_else.stderr b/tests/ui/single_match_else.stderr
@@ -1,5 +1,5 @@
 error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let`
-  --> $DIR/single_match_else.rs:12:5
+  --> $DIR/single_match_else.rs:14:5
    |
 LL | /     match ExprNode::Butterflies {
 LL | |         ExprNode::ExprAddrOf => Some(&NODE),
@@ -19,5 +19,45 @@ LL |         None
 LL |     }
    |
 
-error: aborting due to previous error
+error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let`
+  --> $DIR/single_match_else.rs:70:5
+   |
+LL | /     match Some(1) {
+LL | |         Some(a) => println!("${:?}", a),
+LL | |         None => {
+LL | |             println!("else block");
+LL | |             return
+LL | |         },
+LL | |     }
+   | |_____^
+   |
+help: try this
+   |
+LL |     if let Some(a) = Some(1) { println!("${:?}", a) } else {
+LL |         println!("else block");
+LL |         return
+LL |     }
+   |
+
+error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let`
+  --> $DIR/single_match_else.rs:79:5
+   |
+LL | /     match Some(1) {
+LL | |         Some(a) => println!("${:?}", a),
+LL | |         None => {
+LL | |             println!("else block");
+LL | |             return;
+LL | |         },
+LL | |     }
+   | |_____^
+   |
+help: try this
+   |
+LL |     if let Some(a) = Some(1) { println!("${:?}", a) } else {
+LL |         println!("else block");
+LL |         return;
+LL |     }
+   |
+
+error: aborting due to 3 previous errors