Add new lint if_then_panic

rust-lang · Sep 14, 2021 · bf3194e · bf3194e
1 parent b556398
commit bf3194e
Show file tree

Hide file tree

Showing 10 changed files with 155 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2688,6 +2688,7 @@ Released 2018-09-13
 [`if_let_some_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_some_result
 [`if_not_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else
 [`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
+[`if_then_panic`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_panic
 [`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none
 [`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond
 [`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone

diff --git a/clippy_lints/src/if_then_panic.rs b/clippy_lints/src/if_then_panic.rs
@@ -0,0 +1,63 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::is_expn_of;
+use rustc_hir::{Block, Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Detects `if`-then-`panic!` that can be replaced with `assert!`.
+    ///
+    /// ### Why is this bad?
+    /// `assert!` is simpler than `if`-then-`panic!`.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let sad_people: Vec<&str> = vec![];
+    /// if !sad_people.is_empty() {
+    ///     panic!("there are sad people: {:?}", sad_people);
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let sad_people Vec<&str> = vec![];
+    /// assert!(sad_people.is_empty(), "there are sad people: {:?}", sad_people);
+    /// ```
+    pub IF_THEN_PANIC,
+    style,
+    "`panic!` and only a `panic!` in `if`-then statement"
+}
+
+declare_lint_pass!(IfThenPanic => [IF_THEN_PANIC]);
+
+impl LateLintPass<'_> for IfThenPanic {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
+        if_chain! {
+            if let Expr {
+                kind: ExprKind:: If(cond, Expr {
+                    kind: ExprKind::Block(
+                        Block {
+                            stmts: [stmt],
+                            ..
+                        },
+                        _),
+                    ..
+                }, None),
+                ..
+            } = &expr;
+            if is_expn_of(stmt.span, "panic").is_some();
+            if !matches!(cond.kind, ExprKind::Let(_, _, _));
+
+            then {
+                span_lint_and_help(
+                    cx,
+                    IF_THEN_PANIC,
+                    expr.span,
+                    "only a `panic!` in `if`-then statement",
+                    None,
+                    "considering use `assert!` instead"
+                )
+            }
+        }
+    }
+}
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -227,6 +227,7 @@ mod identity_op;
 mod if_let_mutex;
 mod if_let_some_result;
 mod if_not_else;
+mod if_then_panic;
 mod if_then_some_else_none;
 mod implicit_hasher;
 mod implicit_return;
@@ -658,6 +659,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         if_let_mutex::IF_LET_MUTEX,
         if_let_some_result::IF_LET_SOME_RESULT,
         if_not_else::IF_NOT_ELSE,
+        if_then_panic::IF_THEN_PANIC,
         if_then_some_else_none::IF_THEN_SOME_ELSE_NONE,
         implicit_hasher::IMPLICIT_HASHER,
         implicit_return::IMPLICIT_RETURN,
@@ -1253,6 +1255,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(identity_op::IDENTITY_OP),
         LintId::of(if_let_mutex::IF_LET_MUTEX),
         LintId::of(if_let_some_result::IF_LET_SOME_RESULT),
+        LintId::of(if_then_panic::IF_THEN_PANIC),
         LintId::of(indexing_slicing::OUT_OF_BOUNDS_INDEXING),
         LintId::of(infinite_iter::INFINITE_ITER),
         LintId::of(inherent_to_string::INHERENT_TO_STRING),
@@ -1508,6 +1511,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(functions::MUST_USE_UNIT),
         LintId::of(functions::RESULT_UNIT_ERR),
         LintId::of(if_let_some_result::IF_LET_SOME_RESULT),
+        LintId::of(if_then_panic::IF_THEN_PANIC),
         LintId::of(inherent_to_string::INHERENT_TO_STRING),
         LintId::of(len_zero::COMPARISON_TO_EMPTY),
         LintId::of(len_zero::LEN_WITHOUT_IS_EMPTY),
@@ -2135,6 +2139,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(move || Box::new(self_named_constructors::SelfNamedConstructors));
     store.register_late_pass(move || Box::new(feature_name::FeatureName));
     store.register_late_pass(move || Box::new(iter_not_returning_iterator::IterNotReturningIterator));
+    store.register_late_pass(move || Box::new(if_then_panic::IfThenPanic));
 }
 
 #[rustfmt::skip]

diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs
@@ -561,9 +561,7 @@ declare_lint_pass!(ProduceIce => [PRODUCE_ICE]);
 
 impl EarlyLintPass for ProduceIce {
     fn check_fn(&mut self, _: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: Span, _: NodeId) {
-        if is_trigger_fn(fn_kind) {
-            panic!("Would you like some help with that?");
-        }
+        assert!(!is_trigger_fn(fn_kind), "Would you like some help with that?");
     }
 }
 

diff --git a/tests/compile-test.rs b/tests/compile-test.rs
@@ -90,9 +90,11 @@ fn extern_flags() -> String {
         .copied()
         .filter(|n| !crates.contains_key(n))
         .collect();
-    if !not_found.is_empty() {
-        panic!("dependencies not found in depinfo: {:?}", not_found);
-    }
+    assert!(
+        not_found.is_empty(),
+        "dependencies not found in depinfo: {:?}",
+        not_found
+    );
     crates
         .into_iter()
         .map(|(name, path)| format!(" --extern {}={}", name, path))

diff --git a/tests/integration.rs b/tests/integration.rs
@@ -74,8 +74,11 @@ fn integration_test() {
         panic!("incompatible crate versions");
     } else if stderr.contains("failed to run `rustc` to learn about target-specific information") {
         panic!("couldn't find librustc_driver, consider setting `LD_LIBRARY_PATH`");
-    } else if stderr.contains("toolchain") && stderr.contains("is not installed") {
-        panic!("missing required toolchain");
+    } else {
+        assert!(
+            !stderr.contains("toolchain") || !stderr.contains("is not installed"),
+            "missing required toolchain"
+        );
     }
 
     match output.status.code() {

diff --git a/tests/ui/fallible_impl_from.stderr b/tests/ui/fallible_impl_from.stderr
@@ -40,6 +40,17 @@ LL |             panic!();
    |             ^^^^^^^^^
    = note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info)
 
+error: only a `panic!` in `if`-then statement
+  --> $DIR/fallible_impl_from.rs:28:9
+   |
+LL | /         if i != 42 {
+LL | |             panic!();
+LL | |         }
+   | |_________^
+   |
+   = note: `-D clippy::if-then-panic` implied by `-D warnings`
+   = help: considering use `assert!` instead
+
 error: consider implementing `TryFrom` instead
   --> $DIR/fallible_impl_from.rs:35:1
    |
@@ -67,6 +78,17 @@ LL |             panic!("{:?}", s);
    |             ^^^^^^^^^^^^^^^^^^
    = note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info)
 
+error: only a `panic!` in `if`-then statement
+  --> $DIR/fallible_impl_from.rs:40:16
+   |
+LL |           } else if s.parse::<u32>().unwrap() != 42 {
+   |  ________________^
+LL | |             panic!("{:?}", s);
+LL | |         }
+   | |_________^
+   |
+   = help: considering use `assert!` instead
+
 error: consider implementing `TryFrom` instead
   --> $DIR/fallible_impl_from.rs:53:1
    |
@@ -89,5 +111,15 @@ LL |             panic!("{:?}", s);
    |             ^^^^^^^^^^^^^^^^^^
    = note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: aborting due to 4 previous errors
+error: only a `panic!` in `if`-then statement
+  --> $DIR/fallible_impl_from.rs:55:9
+   |
+LL | /         if s.parse::<u32>().ok().unwrap() != 42 {
+LL | |             panic!("{:?}", s);
+LL | |         }
+   | |_________^
+   |
+   = help: considering use `assert!` instead
+
+error: aborting due to 7 previous errors
 
diff --git a/tests/ui/if_then_panic.rs b/tests/ui/if_then_panic.rs
@@ -0,0 +1,17 @@
+#![warn(clippy::if_then_panic)]
+
+fn main() {
+    let a = vec![1, 2, 3];
+    let c = Some(2);
+    if a.len() == 3 {
+        panic!("qaqaq");
+    }
+    if let Some(b) = c {
+        panic!("orz");
+    }
+    if a.len() == 3 {
+        panic!("qaqaq");
+    } else {
+        println!("qwq");
+    }
+}
diff --git a/tests/ui/if_then_panic.stderr b/tests/ui/if_then_panic.stderr
@@ -0,0 +1,13 @@
+error: only a `panic!` in `if`-then statement
+  --> $DIR/if_then_panic.rs:6:5
+   |
+LL | /     if a.len() == 3 {
+LL | |         panic!("qaqaq");
+LL | |     }
+   | |_____^
+   |
+   = note: `-D clippy::if-then-panic` implied by `-D warnings`
+   = help: considering use `assert!` instead
+
+error: aborting due to previous error
+
diff --git a/tests/ui/ptr_arg.stderr b/tests/ui/ptr_arg.stderr
@@ -108,6 +108,17 @@ help: change `y.as_str()` to
 LL |     let c = y;
    |             ~
 
+error: only a `panic!` in `if`-then statement
+  --> $DIR/ptr_arg.rs:83:5
+   |
+LL | /     if x.capacity() > 1024 {
+LL | |         panic!("Too large!");
+LL | |     }
+   | |_____^
+   |
+   = note: `-D clippy::if-then-panic` implied by `-D warnings`
+   = help: considering use `assert!` instead
+
 error: using a reference to `Cow` is not recommended
   --> $DIR/ptr_arg.rs:90:25
    |
@@ -171,5 +182,5 @@ help: change `str.clone()` to
 LL |         let _ = str.to_path_buf().clone();
    |                 ~~~~~~~~~~~~~~~~~
 
-error: aborting due to 12 previous errors
+error: aborting due to 13 previous errors