From: Bood Qian <bood@glowing.com>
Date: Sat, 11 Feb 2017 13:42:42 +0000 (+0800)
Subject: Lint on `panic!` only
X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=64d2f8af8e44423edc94f0ac3eb186c66966541b;p=rust.git

Lint on `panic!` only
---

diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index d9e5daf889f..57216ad8d41 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -121,9 +121,11 @@
     "a match with overlapping arms"
 }
 
-/// **What it does:** Checks for arm matches all errors with `Err(_)`.
+/// **What it does:** Checks for arm which matches all errors with `Err(_)`
+/// and take drastic actions like `panic!`.
 ///
-/// **Why is this bad?** It is a bad practice to catch all errors the same way
+/// **Why is this bad?** It is generally a bad practice, just like
+/// catching all exceptions in java with `catch(Exception)`
 ///
 /// **Known problems:** None.
 ///
@@ -132,13 +134,13 @@
 /// let x : Result(i32, &str) = Ok(3);
 /// match x {
 ///     Ok(_) => println!("ok"),
-///     Err(_) => println!("err"),
+///     Err(_) => panic!("err"),
 /// }
 /// ```
 declare_lint! {
     pub MATCH_WILD_ERR_ARM,
     Warn,
-    "a match with `Err(_)` arm"
+    "a match with `Err(_)` arm and take drastic actions"
 }
 
 #[allow(missing_copy_implementations)]
@@ -353,32 +355,28 @@ fn check_wild_err_arm(cx: &LateContext, ex: &Expr, arms: &[Arm]) {
                 if inner.iter().any(|pat| pat.node == PatKind::Wild) &&
                     path_str == "Err" {
                         // `Err(_)` arm found
-                        let mut need_lint = true;
-                        if let ExprBlock(ref block) = arm.body.node {
-                            if is_unreachable_block(cx, block) {
-                                need_lint = false;
-                            }
-                        }
-
-                        if need_lint {
+                        if_let_chain! {[
+                            let ExprBlock(ref block) = arm.body.node,
+                            is_panic_block(cx, block)
+                        ], {
                             span_note_and_lint(cx,
                                                MATCH_WILD_ERR_ARM,
                                                arm.pats[0].span,
                                                "Err(_) will match all errors, maybe not a good idea",
                                                arm.pats[0].span,
                                                "to remove this warning, match each error seperately or use unreachable macro");
-                        }
+                        }}
                 }
             }
         }
     }
 }
 
-// If the block contains only a `unreachable!` macro (as expression or statement)
-fn is_unreachable_block(cx: &LateContext, block: &Block) -> bool {
+// If the block contains only a `panic!` macro (as expression or statement)
+fn is_panic_block(cx: &LateContext, block: &Block) -> bool {
     match (&block.expr, block.stmts.len(), block.stmts.first()) {
-        (&Some(ref exp), 0, _) => is_expn_of(cx, exp.span, "unreachable").is_some(),
-        (&None, 1, Some(ref stmt)) => is_expn_of(cx, stmt.span, "unreachable").is_some(),
+        (&Some(ref exp), 0, _) => is_expn_of(cx, exp.span, "panic").is_some() && is_expn_of(cx, exp.span, "unreachable").is_none(),
+        (&None, 1, Some(ref stmt)) => is_expn_of(cx, stmt.span, "panic").is_some() && is_expn_of(cx, stmt.span, "unreachable").is_none(),
         _ => false
     }
 }
diff --git a/tests/ui/matches.rs b/tests/ui/matches.rs
index 277f96be686..46a99293a35 100644
--- a/tests/ui/matches.rs
+++ b/tests/ui/matches.rs
@@ -289,31 +289,41 @@ fn match_wild_err_arm() {
     match x {
         Ok(3) => println!("ok"),
         Ok(_) => println!("ok"),
-        Err(_) => println!("err")
+        Err(_) => panic!("err")
     }
 
     match x {
         Ok(3) => println!("ok"),
         Ok(_) => println!("ok"),
-        Err(_) => {
-            println!("err");
-            unreachable!()
-        }
+        Err(_) => {panic!()}
     }
 
-    // allowed when using with unreachable as the only statement/expression
     match x {
         Ok(3) => println!("ok"),
         Ok(_) => println!("ok"),
-        Err(_) => unreachable!()
+        Err(_) => {panic!();}
     }
 
+    // allowed when not with `panic!` block
+    match x {
+        Ok(3) => println!("ok"),
+        Ok(_) => println!("ok"),
+        Err(_) => println!("err")
+    }
+
+    // allowed when used with `unreachable!`
     match x {
         Ok(3) => println!("ok"),
         Ok(_) => println!("ok"),
         Err(_) => {unreachable!()}
     }
 
+    match x {
+        Ok(3) => println!("ok"),
+        Ok(_) => println!("ok"),
+        Err(_) => unreachable!()
+    }
+
     match x {
         Ok(3) => println!("ok"),
         Ok(_) => println!("ok"),
diff --git a/tests/ui/matches.stderr b/tests/ui/matches.stderr
index 438c2e12c29..bc8584b8587 100644
--- a/tests/ui/matches.stderr
+++ b/tests/ui/matches.stderr
@@ -391,7 +391,7 @@ note: overlaps with this
 error: Err(_) will match all errors, maybe not a good idea
    --> $DIR/matches.rs:292:9
     |
-292 |         Err(_) => println!("err")
+292 |         Err(_) => panic!("err")
     |         ^^^^^^
     |
     = note: #[deny(match_wild_err_arm)] implied by #[deny(clippy)]
@@ -405,11 +405,20 @@ note: lint level defined here
 error: Err(_) will match all errors, maybe not a good idea
    --> $DIR/matches.rs:298:9
     |
-298 |         Err(_) => {
+298 |         Err(_) => {panic!()}
     |         ^^^^^^
     |
     = note: #[deny(match_wild_err_arm)] implied by #[deny(clippy)]
     = note: to remove this warning, match each error seperately or use unreachable macro
 
-error: aborting due to 25 previous errors
+error: Err(_) will match all errors, maybe not a good idea
+   --> $DIR/matches.rs:304:9
+    |
+304 |         Err(_) => {panic!();}
+    |         ^^^^^^
+    |
+    = note: #[deny(match_wild_err_arm)] implied by #[deny(clippy)]
+    = note: to remove this warning, match each error seperately or use unreachable macro
+
+error: aborting due to 26 previous errors