Skip to content

Commit fe999e8

Browse files
committed
Auto merge of #7741 - surechen:fix_if_then_panic, r=flip1995
Make if_then_panic handle situation of BinOpKind::And || BinOpKind::Or fixes #7731 Make if_then_panic handle situation of cond.kind = ExprKind::DropTemps(ExprKind::Binary(BinOpKind::And || BinOpKind::Or, left, right), ..) = changelog: [`if_then_panic`] Fix suggestion for more complex conditions
2 parents f8303ad + 41e2c68 commit fe999e8

File tree

4 files changed

+71
-7
lines changed

4 files changed

+71
-7
lines changed

clippy_lints/src/if_then_panic.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::higher::PanicExpn;
3-
use clippy_utils::is_expn_of;
43
use clippy_utils::source::snippet_with_applicability;
4+
use clippy_utils::{is_expn_of, sugg};
55
use rustc_errors::Applicability;
66
use rustc_hir::{Block, Expr, ExprKind, StmtKind, UnOp};
77
use rustc_lint::{LateContext, LateLintPass};
@@ -74,12 +74,14 @@ impl LateLintPass<'_> for IfThenPanic {
7474
};
7575
let mut applicability = Applicability::MachineApplicable;
7676
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()
77+
let cond_sugg = if let ExprKind::DropTemps(e, ..) = cond.kind {
78+
if let Expr{kind: ExprKind::Unary(UnOp::Not, not_expr), ..} = e {
79+
sugg::Sugg::hir_with_applicability(cx, not_expr, "..", &mut applicability).maybe_par().to_string()
80+
} else {
81+
format!("!{}", sugg::Sugg::hir_with_applicability(cx, e, "..", &mut applicability).maybe_par().to_string())
82+
}
8183
} else {
82-
format!("!{}", snippet_with_applicability(cx, cond.span, "..", &mut applicability))
84+
format!("!{}", sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par().to_string())
8385
};
8486

8587
span_lint_and_sugg(

tests/ui/if_then_panic.fixed

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,10 @@ fn main() {
3131
} else {
3232
println!("qwq");
3333
}
34+
let b = vec![1, 2, 3];
35+
assert!(!b.is_empty(), "panic1");
36+
assert!(!(b.is_empty() && a.is_empty()), "panic2");
37+
assert!(!(a.is_empty() && !b.is_empty()), "panic3");
38+
assert!(!(b.is_empty() || a.is_empty()), "panic4");
39+
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
3440
}

tests/ui/if_then_panic.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,20 @@ fn main() {
3535
} else {
3636
println!("qwq");
3737
}
38+
let b = vec![1, 2, 3];
39+
if b.is_empty() {
40+
panic!("panic1");
41+
}
42+
if b.is_empty() && a.is_empty() {
43+
panic!("panic2");
44+
}
45+
if a.is_empty() && !b.is_empty() {
46+
panic!("panic3");
47+
}
48+
if b.is_empty() || a.is_empty() {
49+
panic!("panic4");
50+
}
51+
if a.is_empty() || !b.is_empty() {
52+
panic!("panic5");
53+
}
3854
}

tests/ui/if_then_panic.stderr

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,45 @@ LL | | panic!("qwqwq");
1616
LL | | }
1717
| |_____^ help: try: `assert!(a.is_empty(), "qwqwq");`
1818

19-
error: aborting due to 2 previous errors
19+
error: only a `panic!` in `if`-then statement
20+
--> $DIR/if_then_panic.rs:39:5
21+
|
22+
LL | / if b.is_empty() {
23+
LL | | panic!("panic1");
24+
LL | | }
25+
| |_____^ help: try: `assert!(!b.is_empty(), "panic1");`
26+
27+
error: only a `panic!` in `if`-then statement
28+
--> $DIR/if_then_panic.rs:42:5
29+
|
30+
LL | / if b.is_empty() && a.is_empty() {
31+
LL | | panic!("panic2");
32+
LL | | }
33+
| |_____^ help: try: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
34+
35+
error: only a `panic!` in `if`-then statement
36+
--> $DIR/if_then_panic.rs:45:5
37+
|
38+
LL | / if a.is_empty() && !b.is_empty() {
39+
LL | | panic!("panic3");
40+
LL | | }
41+
| |_____^ help: try: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
42+
43+
error: only a `panic!` in `if`-then statement
44+
--> $DIR/if_then_panic.rs:48:5
45+
|
46+
LL | / if b.is_empty() || a.is_empty() {
47+
LL | | panic!("panic4");
48+
LL | | }
49+
| |_____^ help: try: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
50+
51+
error: only a `panic!` in `if`-then statement
52+
--> $DIR/if_then_panic.rs:51:5
53+
|
54+
LL | / if a.is_empty() || !b.is_empty() {
55+
LL | | panic!("panic5");
56+
LL | | }
57+
| |_____^ help: try: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`
58+
59+
error: aborting due to 7 previous errors
2060

0 commit comments

Comments
 (0)