From 5761bb741e7aa75beb03e74213ade70a3004ca8f Mon Sep 17 00:00:00 2001 From: Labelray Date: Tue, 14 Sep 2021 16:28:09 +0800 Subject: [PATCH] Add new lint `if_then_panic` --- CHANGELOG.md | 1 + clippy_lints/src/if_then_panic.rs | 63 ++++++++++++++++++++++++ clippy_lints/src/lib.rs | 5 ++ clippy_lints/src/utils/internal_lints.rs | 4 +- tests/compile-test.rs | 8 +-- tests/integration.rs | 7 ++- tests/ui/fallible_impl_from.stderr | 34 ++++++++++++- tests/ui/if_then_panic.rs | 17 +++++++ tests/ui/if_then_panic.stderr | 13 +++++ tests/ui/ptr_arg.stderr | 13 ++++- 10 files changed, 155 insertions(+), 10 deletions(-) create mode 100644 clippy_lints/src/if_then_panic.rs create mode 100644 tests/ui/if_then_panic.rs create mode 100644 tests/ui/if_then_panic.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 71383e8116b0..b6a3655cb862 100644 --- 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 new file mode 100644 index 000000000000..00bed7e8c8a2 --- /dev/null +++ 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 index e3dd43d69825..2ffe3bc9bc8e 100644 --- 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 index 756c33d70c26..273684b8e86c 100644 --- 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 index c9d101115958..d7596f6ff0ca 100644 --- 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 index 7e3eff3c7324..c64425fa01a4 100644 --- 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 index 64c8ea857277..f2496a4f022c 100644 --- 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::().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::().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 new file mode 100644 index 000000000000..4f9cebb6ed13 --- /dev/null +++ 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 new file mode 100644 index 000000000000..fa6dcbf48fc8 --- /dev/null +++ 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 index 64594eb870c2..d599ed060fca 100644 --- 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