Skip to content

Commit 0e5ccd7

Browse files
LabelrayLabelray
committed
Add new lint if_then_panic 
1 parent b556398 commit 0e5ccd7

File tree

11 files changed

+308
-10
lines changed

11 files changed

+308
-10
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: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
use clippy_utils::diagnostics::span_lint_and_help;
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+
55+
then {
56+
let span = if let Some(panic_expn) = PanicExpn::parse(semi) {
57+
match *panic_expn.format_args.value_args {
58+
[] => panic_expn.format_args.format_string_span,
59+
[.., last] => panic_expn.format_args.format_string_span.to(last.span),
60+
}
61+
} else {
62+
if_chain! {
63+
if let ExprKind::Block(block, _) = semi.kind;
64+
if let Some(init) = block.expr;
65+
if let ExprKind::Call(_, [format_args]) = init.kind;
66+
67+
then {
68+
format_args.span
69+
} else {
70+
return
71+
}
72+
}
73+
};
74+
let mut applicability = Applicability::MachineApplicable;
75+
let sugg = snippet_with_applicability(cx, span, "..", &mut applicability);
76+
77+
let cond_sugg =
78+
if let ExprKind::DropTemps(Expr{kind: ExprKind::Unary(UnOp::Not, not_expr), ..}) = cond.kind {
79+
snippet_with_applicability(cx, not_expr.span, "..", &mut applicability).to_string()
80+
} else {
81+
format!("!{}", snippet_with_applicability(cx, cond.span, "..", &mut applicability))
82+
};
83+
84+
span_lint_and_help(
85+
cx,
86+
IF_THEN_PANIC,
87+
expr.span,
88+
"only a `panic!` in `if`-then statement",
89+
None,
90+
&format!("considering use `assert!({}, {})` instead", cond_sugg, sugg)
91+
);
92+
}
93+
}
94+
}
95+
}

‎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: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,3 +608,106 @@ 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: PanicArgsExpn<'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 Some(format_args) = PanicArgsExpn::parse(format_args);
629+
then {
630+
Some(PanicExpn {
631+
call_site: expn_data.call_site,
632+
format_args,
633+
})
634+
} else {
635+
None
636+
}
637+
}
638+
}
639+
}
640+
641+
/// A parsed `format_args!` expansion
642+
pub struct PanicArgsExpn<'tcx> {
643+
/// Span of the first argument, the format string
644+
pub format_string_span: Span,
645+
/// Values passed after the format string
646+
pub value_args: Vec<&'tcx Expr<'tcx>>,
647+
648+
/// String literal expressions which represent the format string split by "{}"
649+
pub format_string_parts: &'tcx [Expr<'tcx>],
650+
/// Symbols corresponding to [`Self::format_string_parts`]
651+
pub format_string_symbols: Vec<Symbol>,
652+
/// Expressions like `ArgumentV1::new(arg0, Debug::fmt)`
653+
pub args: &'tcx [Expr<'tcx>],
654+
/// The final argument passed to `Arguments::new_v1_formatted`, if applicable
655+
pub fmt_expr: Option<&'tcx Expr<'tcx>>,
656+
}
657+
658+
impl PanicArgsExpn<'tcx> {
659+
/// Parses an expanded `format_args!` or `format_args_nl!` invocation
660+
pub fn parse(expr: &'tcx Expr<'tcx>) -> Option<Self> {
661+
if_chain! {
662+
if let ExprKind::AddrOf(_, _, matched) = expr.kind;
663+
if let ExprKind::Match(inner_match, [arm], _) = matched.kind;
664+
665+
// `match match`, if you will
666+
if let ExprKind::Match(args, [inner_arm], _) = inner_match.kind;
667+
if let ExprKind::Tup(value_args) = args.kind;
668+
if let Some(value_args) = value_args
669+
.iter()
670+
.map(|e| match e.kind {
671+
ExprKind::AddrOf(_, _, e) => Some(e),
672+
_ => None,
673+
})
674+
.collect();
675+
if let ExprKind::Array(args) = inner_arm.body.kind;
676+
677+
if let ExprKind::Block(Block { stmts: [], expr: Some(expr), .. }, _) = arm.body.kind;
678+
if let ExprKind::Call(_, call_args) = expr.kind;
679+
if let Some((strs_ref, fmt_expr)) = match call_args {
680+
// Arguments::new_v1
681+
[strs_ref, _] => Some((strs_ref, None)),
682+
// Arguments::new_v1_formatted
683+
[strs_ref, _, fmt_expr] => Some((strs_ref, Some(fmt_expr))),
684+
_ => None,
685+
};
686+
if let ExprKind::AddrOf(BorrowKind::Ref, _, strs_arr) = strs_ref.kind;
687+
if let ExprKind::Array(format_string_parts) = strs_arr.kind;
688+
if let Some(format_string_symbols) = format_string_parts
689+
.iter()
690+
.map(|e| {
691+
if let ExprKind::Lit(lit) = &e.kind {
692+
if let LitKind::Str(symbol, _style) = lit.node {
693+
return Some(symbol);
694+
}
695+
}
696+
None
697+
})
698+
.collect();
699+
then {
700+
Some(PanicArgsExpn {
701+
format_string_span: strs_ref.span,
702+
value_args,
703+
format_string_parts,
704+
format_string_symbols,
705+
args,
706+
fmt_expr,
707+
})
708+
} else {
709+
None
710+
}
711+
}
712+
}
713+
}

‎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.stderr‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,17 @@ 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

43+
error: only a `panic!` in `if`-then statement
44+
--> $DIR/fallible_impl_from.rs:28:9
45+
|
46+
LL | / if i != 42 {
47+
LL | | panic!();
48+
LL | | }
49+
| |_________^
50+
|
51+
= note: `-D clippy::if-then-panic` implied by `-D warnings`
52+
= help: considering use `assert!(!i != 42, "explicit panic")` instead
53+
4354
error: consider implementing `TryFrom` instead
4455
--> $DIR/fallible_impl_from.rs:35:1
4556
|
@@ -67,6 +78,17 @@ LL | panic!("{:?}", s);
6778
| ^^^^^^^^^^^^^^^^^^
6879
= note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info)
6980

81+
error: only a `panic!` in `if`-then statement
82+
--> $DIR/fallible_impl_from.rs:40:16
83+
|
84+
LL | } else if s.parse::<u32>().unwrap() != 42 {
85+
| ________________^
86+
LL | | panic!("{:?}", s);
87+
LL | | }
88+
| |_________^
89+
|
90+
= help: considering use `assert!(!s.parse::<u32>().unwrap() != 42, "{:?}", s)` instead
91+
7092
error: consider implementing `TryFrom` instead
7193
--> $DIR/fallible_impl_from.rs:53:1
7294
|
@@ -89,5 +111,15 @@ LL | panic!("{:?}", s);
89111
| ^^^^^^^^^^^^^^^^^^
90112
= note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info)
91113

92-
error: aborting due to 4 previous errors
114+
error: only a `panic!` in `if`-then statement
115+
--> $DIR/fallible_impl_from.rs:55:9
116+
|
117+
LL | / if s.parse::<u32>().ok().unwrap() != 42 {
118+
LL | | panic!("{:?}", s);
119+
LL | | }
120+
| |_________^
121+
|
122+
= help: considering use `assert!(!s.parse::<u32>().ok().unwrap() != 42, "{:?}", s)` instead
123+
124+
error: aborting due to 7 previous errors
93125

‎tests/ui/if_then_panic.rs‎

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

‎tests/ui/if_then_panic.stderr‎

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

0 commit comments

Comments
 (0)