Skip to content

Commit 5f750a8

Browse files
LabelrayLabelray
committed
Add new lint if_then_panic 
1 parent b556398 commit 5f750a8

14 files changed

+263
-29
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2688,6 +2688,7 @@ Released 2018-09-13
26882688
[`if_let_some_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_some_result
26892689
[`if_not_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else
26902690
[`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
2691+
[`if_then_panic`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_panic
26912692
[`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none
26922693
[`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond
26932694
[`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone

‎clippy_lints/src/if_then_panic.rs

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::higher::PanicExpn;
3+
use clippy_utils::is_expn_of;
4+
use clippy_utils::source::snippet_with_applicability;
5+
use rustc_errors::Applicability;
6+
use rustc_hir::{Block, Expr, ExprKind, StmtKind, UnOp};
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_session::{declare_lint_pass, declare_tool_lint};
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
/// Detects `if`-then-`panic!` that can be replaced with `assert!`.
13+
///
14+
/// ### Why is this bad?
15+
/// `assert!` is simpler than `if`-then-`panic!`.
16+
///
17+
/// ### Example
18+
/// ```rust
19+
/// let sad_people: Vec<&str> = vec![];
20+
/// if !sad_people.is_empty() {
21+
/// panic!("there are sad people: {:?}", sad_people);
22+
/// }
23+
/// ```
24+
/// Use instead:
25+
/// ```rust
26+
/// let sad_people: Vec<&str> = vec![];
27+
/// assert!(sad_people.is_empty(), "there are sad people: {:?}", sad_people);
28+
/// ```
29+
pub IF_THEN_PANIC,
30+
style,
31+
"`panic!` and only a `panic!` in `if`-then statement"
32+
}
33+
34+
declare_lint_pass!(IfThenPanic => [IF_THEN_PANIC]);
35+
36+
impl LateLintPass<'_> for IfThenPanic {
37+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
38+
if_chain! {
39+
if let Expr {
40+
kind: ExprKind:: If(cond, Expr {
41+
kind: ExprKind::Block(
42+
Block {
43+
stmts: [stmt],
44+
..
45+
},
46+
_),
47+
..
48+
}, None),
49+
..
50+
} = &expr;
51+
if is_expn_of(stmt.span, "panic").is_some();
52+
if !matches!(cond.kind, ExprKind::Let(_, _, _));
53+
if let StmtKind::Semi(semi) = stmt.kind;
54+
if !cx.tcx.sess.source_map().is_multiline(cond.span);
55+
56+
then {
57+
let span = if let Some(panic_expn) = PanicExpn::parse(semi) {
58+
match *panic_expn.format_args.value_args {
59+
[] => panic_expn.format_args.format_string_span,
60+
[.., last] => panic_expn.format_args.format_string_span.to(last.span),
61+
}
62+
} else {
63+
if_chain! {
64+
if let ExprKind::Block(block, _) = semi.kind;
65+
if let Some(init) = block.expr;
66+
if let ExprKind::Call(_, [format_args]) = init.kind;
67+
68+
then {
69+
format_args.span
70+
} else {
71+
return
72+
}
73+
}
74+
};
75+
let mut applicability = Applicability::MachineApplicable;
76+
let sugg = snippet_with_applicability(cx, span, "..", &mut applicability);
77+
78+
let cond_sugg =
79+
if let ExprKind::DropTemps(Expr{kind: ExprKind::Unary(UnOp::Not, not_expr), ..}) = cond.kind {
80+
snippet_with_applicability(cx, not_expr.span, "..", &mut applicability).to_string()
81+
} else {
82+
format!("!{}", snippet_with_applicability(cx, cond.span, "..", &mut applicability))
83+
};
84+
85+
span_lint_and_sugg(
86+
cx,
87+
IF_THEN_PANIC,
88+
expr.span,
89+
"only a `panic!` in `if`-then statement",
90+
"try",
91+
format!("assert!({}, {});", cond_sugg, sugg),
92+
Applicability::MachineApplicable,
93+
);
94+
}
95+
}
96+
}
97+
}

‎clippy_lints/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ mod identity_op;
227227
mod if_let_mutex;
228228
mod if_let_some_result;
229229
mod if_not_else;
230+
mod if_then_panic;
230231
mod if_then_some_else_none;
231232
mod implicit_hasher;
232233
mod implicit_return;
@@ -658,6 +659,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
658659
if_let_mutex::IF_LET_MUTEX,
659660
if_let_some_result::IF_LET_SOME_RESULT,
660661
if_not_else::IF_NOT_ELSE,
662+
if_then_panic::IF_THEN_PANIC,
661663
if_then_some_else_none::IF_THEN_SOME_ELSE_NONE,
662664
implicit_hasher::IMPLICIT_HASHER,
663665
implicit_return::IMPLICIT_RETURN,
@@ -1253,6 +1255,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12531255
LintId::of(identity_op::IDENTITY_OP),
12541256
LintId::of(if_let_mutex::IF_LET_MUTEX),
12551257
LintId::of(if_let_some_result::IF_LET_SOME_RESULT),
1258+
LintId::of(if_then_panic::IF_THEN_PANIC),
12561259
LintId::of(indexing_slicing::OUT_OF_BOUNDS_INDEXING),
12571260
LintId::of(infinite_iter::INFINITE_ITER),
12581261
LintId::of(inherent_to_string::INHERENT_TO_STRING),
@@ -1508,6 +1511,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15081511
LintId::of(functions::MUST_USE_UNIT),
15091512
LintId::of(functions::RESULT_UNIT_ERR),
15101513
LintId::of(if_let_some_result::IF_LET_SOME_RESULT),
1514+
LintId::of(if_then_panic::IF_THEN_PANIC),
15111515
LintId::of(inherent_to_string::INHERENT_TO_STRING),
15121516
LintId::of(len_zero::COMPARISON_TO_EMPTY),
15131517
LintId::of(len_zero::LEN_WITHOUT_IS_EMPTY),
@@ -2135,6 +2139,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
21352139
store.register_late_pass(move || Box::new(self_named_constructors::SelfNamedConstructors));
21362140
store.register_late_pass(move || Box::new(feature_name::FeatureName));
21372141
store.register_late_pass(move || Box::new(iter_not_returning_iterator::IterNotReturningIterator));
2142+
store.register_late_pass(move || Box::new(if_then_panic::IfThenPanic));
21382143
}
21392144

21402145
#[rustfmt::skip]

‎clippy_lints/src/utils/internal_lints.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -561,9 +561,7 @@ declare_lint_pass!(ProduceIce => [PRODUCE_ICE]);
561561

562562
impl EarlyLintPass for ProduceIce {
563563
fn check_fn(&mut self, _: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: Span, _: NodeId) {
564-
if is_trigger_fn(fn_kind) {
565-
panic!("Would you like some help with that?");
566-
}
564+
assert!(!is_trigger_fn(fn_kind), "Would you like some help with that?");
567565
}
568566
}
569567

‎clippy_utils/src/higher.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,3 +608,33 @@ pub fn is_from_for_desugar(local: &hir::Local<'_>) -> bool {
608608

609609
false
610610
}
611+
612+
/// A parsed `panic!` expansion
613+
pub struct PanicExpn<'tcx> {
614+
/// Span of `panic!(..)`
615+
pub call_site: Span,
616+
/// Inner `format_args!` expansion
617+
pub format_args: FormatArgsExpn<'tcx>,
618+
}
619+
620+
impl PanicExpn<'tcx> {
621+
/// Parses an expanded `panic!` invocation
622+
pub fn parse(expr: &'tcx Expr<'tcx>) -> Option<Self> {
623+
if_chain! {
624+
if let ExprKind::Block(block, _) = expr.kind;
625+
if let Some(init) = block.expr;
626+
if let ExprKind::Call(_, [format_args]) = init.kind;
627+
let expn_data = expr.span.ctxt().outer_expn_data();
628+
if let ExprKind::AddrOf(_, _, format_args) = format_args.kind;
629+
if let Some(format_args) = FormatArgsExpn::parse(format_args);
630+
then {
631+
Some(PanicExpn {
632+
call_site: expn_data.call_site,
633+
format_args,
634+
})
635+
} else {
636+
None
637+
}
638+
}
639+
}
640+
}

‎tests/compile-test.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,11 @@ fn extern_flags() -> String {
9090
.copied()
9191
.filter(|n| !crates.contains_key(n))
9292
.collect();
93-
if !not_found.is_empty() {
94-
panic!("dependencies not found in depinfo: {:?}", not_found);
95-
}
93+
assert!(
94+
not_found.is_empty(),
95+
"dependencies not found in depinfo: {:?}",
96+
not_found
97+
);
9698
crates
9799
.into_iter()
98100
.map(|(name, path)| format!(" --extern {}={}", name, path))

‎tests/integration.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,11 @@ fn integration_test() {
7474
panic!("incompatible crate versions");
7575
} else if stderr.contains("failed to run `rustc` to learn about target-specific information") {
7676
panic!("couldn't find librustc_driver, consider setting `LD_LIBRARY_PATH`");
77-
} else if stderr.contains("toolchain") && stderr.contains("is not installed") {
78-
panic!("missing required toolchain");
77+
} else {
78+
assert!(
79+
!stderr.contains("toolchain") || !stderr.contains("is not installed"),
80+
"missing required toolchain"
81+
);
7982
}
8083

8184
match output.status.code() {

‎tests/ui/fallible_impl_from.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![deny(clippy::fallible_impl_from)]
2+
#![allow(clippy::if_then_panic)]
23

34
// docs example
45
struct Foo(i32);

‎tests/ui/fallible_impl_from.stderr

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: consider implementing `TryFrom` instead
2-
--> $DIR/fallible_impl_from.rs:5:1
2+
--> $DIR/fallible_impl_from.rs:6:1
33
|
44
LL | / impl From<String> for Foo {
55
LL | | fn from(s: String) -> Self {
@@ -15,13 +15,13 @@ LL | #![deny(clippy::fallible_impl_from)]
1515
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
1616
= help: `From` is intended for infallible conversions only. Use `TryFrom` if there's a possibility for the conversion to fail
1717
note: potential failure(s)
18-
--> $DIR/fallible_impl_from.rs:7:13
18+
--> $DIR/fallible_impl_from.rs:8:13
1919
|
2020
LL | Foo(s.parse().unwrap())
2121
| ^^^^^^^^^^^^^^^^^^
2222

2323
error: consider implementing `TryFrom` instead
24-
--> $DIR/fallible_impl_from.rs:26:1
24+
--> $DIR/fallible_impl_from.rs:27:1
2525
|
2626
LL | / impl From<usize> for Invalid {
2727
LL | | fn from(i: usize) -> Invalid {
@@ -34,14 +34,14 @@ LL | | }
3434
|
3535
= help: `From` is intended for infallible conversions only. Use `TryFrom` if there's a possibility for the conversion to fail
3636
note: potential failure(s)
37-
--> $DIR/fallible_impl_from.rs:29:13
37+
--> $DIR/fallible_impl_from.rs:30:13
3838
|
3939
LL | panic!();
4040
| ^^^^^^^^^
4141
= note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info)
4242

4343
error: consider implementing `TryFrom` instead
44-
--> $DIR/fallible_impl_from.rs:35:1
44+
--> $DIR/fallible_impl_from.rs:36:1
4545
|
4646
LL | / impl From<Option<String>> for Invalid {
4747
LL | | fn from(s: Option<String>) -> Invalid {
@@ -54,7 +54,7 @@ LL | | }
5454
|
5555
= help: `From` is intended for infallible conversions only. Use `TryFrom` if there's a possibility for the conversion to fail
5656
note: potential failure(s)
57-
--> $DIR/fallible_impl_from.rs:37:17
57+
--> $DIR/fallible_impl_from.rs:38:17
5858
|
5959
LL | let s = s.unwrap();
6060
| ^^^^^^^^^^
@@ -68,7 +68,7 @@ LL | panic!("{:?}", s);
6868
= note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info)
6969

7070
error: consider implementing `TryFrom` instead
71-
--> $DIR/fallible_impl_from.rs:53:1
71+
--> $DIR/fallible_impl_from.rs:54:1
7272
|
7373
LL | / impl<'a> From<&'a mut <Box<u32> as ProjStrTrait>::ProjString> for Invalid {
7474
LL | | fn from(s: &'a mut <Box<u32> as ProjStrTrait>::ProjString) -> Invalid {
@@ -81,7 +81,7 @@ LL | | }
8181
|
8282
= help: `From` is intended for infallible conversions only. Use `TryFrom` if there's a possibility for the conversion to fail
8383
note: potential failure(s)
84-
--> $DIR/fallible_impl_from.rs:55:12
84+
--> $DIR/fallible_impl_from.rs:56:12
8585
|
8686
LL | if s.parse::<u32>().ok().unwrap() != 42 {
8787
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

‎tests/ui/if_then_panic.fixed

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// run-rustfix
2+
#![warn(clippy::if_then_panic)]
3+
4+
fn main() {
5+
let a = vec![1, 2, 3];
6+
let c = Some(2);
7+
if !a.is_empty()
8+
&& a.len() == 3
9+
&& c != None
10+
&& !a.is_empty()
11+
&& a.len() == 3
12+
&& !a.is_empty()
13+
&& a.len() == 3
14+
&& !a.is_empty()
15+
&& a.len() == 3
16+
{
17+
panic!("qaqaq{:?}", a);
18+
}
19+
assert!(a.is_empty(), "qaqaq{:?}", a);
20+
assert!(a.is_empty(), "qwqwq");
21+
if a.len() == 3 {
22+
println!("qwq");
23+
println!("qwq");
24+
println!("qwq");
25+
}
26+
if let Some(b) = c {
27+
panic!("orz {}", b);
28+
}
29+
if a.len() == 3 {
30+
panic!("qaqaq");
31+
} else {
32+
println!("qwq");
33+
}
34+
}

‎tests/ui/if_then_panic.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// run-rustfix
2+
#![warn(clippy::if_then_panic)]
3+
4+
fn main() {
5+
let a = vec![1, 2, 3];
6+
let c = Some(2);
7+
if !a.is_empty()
8+
&& a.len() == 3
9+
&& c != None
10+
&& !a.is_empty()
11+
&& a.len() == 3
12+
&& !a.is_empty()
13+
&& a.len() == 3
14+
&& !a.is_empty()
15+
&& a.len() == 3
16+
{
17+
panic!("qaqaq{:?}", a);
18+
}
19+
if !a.is_empty() {
20+
panic!("qaqaq{:?}", a);
21+
}
22+
if !a.is_empty() {
23+
panic!("qwqwq");
24+
}
25+
if a.len() == 3 {
26+
println!("qwq");
27+
println!("qwq");
28+
println!("qwq");
29+
}
30+
if let Some(b) = c {
31+
panic!("orz {}", b);
32+
}
33+
if a.len() == 3 {
34+
panic!("qaqaq");
35+
} else {
36+
println!("qwq");
37+
}
38+
}

‎tests/ui/if_then_panic.stderr

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error: only a `panic!` in `if`-then statement
2+
--> $DIR/if_then_panic.rs:19:5
3+
|
4+
LL | / if !a.is_empty() {
5+
LL | | panic!("qaqaq{:?}", a);
6+
LL | | }
7+
| |_____^ help: try: `assert!(a.is_empty(), "qaqaq{:?}", a);`
8+
|
9+
= note: `-D clippy::if-then-panic` implied by `-D warnings`
10+
11+
error: only a `panic!` in `if`-then statement
12+
--> $DIR/if_then_panic.rs:22:5
13+
|
14+
LL | / if !a.is_empty() {
15+
LL | | panic!("qwqwq");
16+
LL | | }
17+
| |_____^ help: try: `assert!(a.is_empty(), "qwqwq");`
18+
19+
error: aborting due to 2 previous errors
20+

‎tests/ui/ptr_arg.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
#![allow(unused, clippy::many_single_char_names, clippy::redundant_clone)]
1+
#![allow(
2+
unused,
3+
clippy::many_single_char_names,
4+
clippy::redundant_clone,
5+
clippy::if_then_panic
6+
)]
27
#![warn(clippy::ptr_arg)]
38

49
use std::borrow::Cow;

0 commit comments

Comments
 (0)