Skip to content

Commit

Permalink
Fix if_then_panic for #![no_std] and Rust 2021
Browse files Browse the repository at this point in the history
  • Loading branch information
nbdd0121 committed Oct 20, 2021
1 parent 300b821 commit 001d071
Show file tree
Hide file tree
Showing 8 changed files with 286 additions and 17 deletions.
15 changes: 11 additions & 4 deletions clippy_lints/src/if_then_panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,23 @@ impl LateLintPass<'_> for IfThenPanic {
if !cx.tcx.sess.source_map().is_multiline(cond.span);

then {
let span = if let Some(panic_expn) = PanicExpn::parse(semi) {
let call = if_chain! {
if let ExprKind::Block(block, _) = semi.kind;
if let Some(init) = block.expr;
then {
init
} else {
semi
}
};
let span = if let Some(panic_expn) = PanicExpn::parse(call) {
match *panic_expn.format_args.value_args {
[] => panic_expn.format_args.format_string_span,
[.., last] => panic_expn.format_args.format_string_span.to(last.span),
}
} else {
if_chain! {
if let ExprKind::Block(block, _) = semi.kind;
if let Some(init) = block.expr;
if let ExprKind::Call(_, [format_args]) = init.kind;
if let ExprKind::Call(_, [format_args]) = call.kind;

then {
format_args.span
Expand Down
24 changes: 14 additions & 10 deletions clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -968,8 +968,7 @@ fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm
}
if_chain! {
if matching_wild;
if let ExprKind::Block(block, _) = arm.body.kind;
if is_panic_block(block);
if is_panic_call(arm.body);
then {
// `Err(_)` or `Err(_e)` arm with `panic!` found
span_lint_and_note(cx,
Expand Down Expand Up @@ -1172,14 +1171,19 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>])
}

// If the block contains only a `panic!` macro (as expression or statement)
fn is_panic_block(block: &Block<'_>) -> bool {
match (&block.expr, block.stmts.len(), block.stmts.first()) {
(&Some(exp), 0, _) => is_expn_of(exp.span, "panic").is_some() && is_expn_of(exp.span, "unreachable").is_none(),
(&None, 1, Some(stmt)) => {
is_expn_of(stmt.span, "panic").is_some() && is_expn_of(stmt.span, "unreachable").is_none()
},
_ => false,
}
fn is_panic_call(expr: &Expr<'_>) -> bool {
// Unwrap any wrapping blocks
let span = if let ExprKind::Block(block, _) = expr.kind {
match (&block.expr, block.stmts.len(), block.stmts.first()) {
(&Some(exp), 0, _) => exp.span,
(&None, 1, Some(stmt)) => stmt.span,
_ => return false,
}
} else {
expr.span
};

is_expn_of(span, "panic").is_some() && is_expn_of(span, "unreachable").is_none()
}

fn check_match_ref_pats<'a, 'b, I>(cx: &LateContext<'_>, ex: &Expr<'_>, pats: I, expr: &Expr<'_>)
Expand Down
4 changes: 1 addition & 3 deletions clippy_utils/src/higher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,9 +718,7 @@ impl PanicExpn<'tcx> {
/// Parses an expanded `panic!` invocation
pub fn parse(expr: &'tcx Expr<'tcx>) -> Option<Self> {
if_chain! {
if let ExprKind::Block(block, _) = expr.kind;
if let Some(init) = block.expr;
if let ExprKind::Call(_, [format_args]) = init.kind;
if let ExprKind::Call(_, [format_args]) = expr.kind;
let expn_data = expr.span.ctxt().outer_expn_data();
if let ExprKind::AddrOf(_, _, format_args) = format_args.kind;
if let Some(format_args) = FormatArgsExpn::parse(format_args);
Expand Down
42 changes: 42 additions & 0 deletions tests/ui/if_then_panic_core.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// run-rustfix
#![crate_type = "lib"]
#![no_std]
#![warn(clippy::if_then_panic)]

pub fn main() {
let a = &[1, 2, 3];
let c = Some(2);
if !a.is_empty()
&& a.len() == 3
&& c != None
&& !a.is_empty()
&& a.len() == 3
&& !a.is_empty()
&& a.len() == 3
&& !a.is_empty()
&& a.len() == 3
{
panic!("qaqaq{:?}", a);
}
assert!(a.is_empty(), "qaqaq{:?}", a);
assert!(a.is_empty(), "qwqwq");
if a.len() == 3 {
format_args!("qwq");
format_args!("qwq");
format_args!("qwq");
}
if let Some(b) = c {
panic!("orz {}", b);
}
if a.len() == 3 {
panic!("qaqaq");
} else {
format_args!("qwq");
}
let b = &[1, 2, 3];
assert!(!b.is_empty(), "panic1");
assert!(!(b.is_empty() && a.is_empty()), "panic2");
assert!(!(a.is_empty() && !b.is_empty()), "panic3");
assert!(!(b.is_empty() || a.is_empty()), "panic4");
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
}
56 changes: 56 additions & 0 deletions tests/ui/if_then_panic_core.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// run-rustfix
#![crate_type = "lib"]
#![no_std]
#![warn(clippy::if_then_panic)]

pub fn main() {
let a = &[1, 2, 3];
let c = Some(2);
if !a.is_empty()
&& a.len() == 3
&& c != None
&& !a.is_empty()
&& a.len() == 3
&& !a.is_empty()
&& a.len() == 3
&& !a.is_empty()
&& a.len() == 3
{
panic!("qaqaq{:?}", a);
}
if !a.is_empty() {
panic!("qaqaq{:?}", a);
}
if !a.is_empty() {
panic!("qwqwq");
}
if a.len() == 3 {
format_args!("qwq");
format_args!("qwq");
format_args!("qwq");
}
if let Some(b) = c {
panic!("orz {}", b);
}
if a.len() == 3 {
panic!("qaqaq");
} else {
format_args!("qwq");
}
let b = &[1, 2, 3];
if b.is_empty() {
panic!("panic1");
}
if b.is_empty() && a.is_empty() {
panic!("panic2");
}
if a.is_empty() && !b.is_empty() {
panic!("panic3");
}
if b.is_empty() || a.is_empty() {
panic!("panic4");
}
if a.is_empty() || !b.is_empty() {
panic!("panic5");
}
}
60 changes: 60 additions & 0 deletions tests/ui/if_then_panic_core.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
error: only a `panic!` in `if`-then statement
--> $DIR/if_then_panic_core.rs:21:5
|
LL | / if !a.is_empty() {
LL | | panic!("qaqaq{:?}", a);
LL | | }
| |_____^ help: try: `assert!(a.is_empty(), "qaqaq{:?}", a);`
|
= note: `-D clippy::if-then-panic` implied by `-D warnings`

error: only a `panic!` in `if`-then statement
--> $DIR/if_then_panic_core.rs:24:5
|
LL | / if !a.is_empty() {
LL | | panic!("qwqwq");
LL | | }
| |_____^ help: try: `assert!(a.is_empty(), "qwqwq");`

error: only a `panic!` in `if`-then statement
--> $DIR/if_then_panic_core.rs:41:5
|
LL | / if b.is_empty() {
LL | | panic!("panic1");
LL | | }
| |_____^ help: try: `assert!(!b.is_empty(), "panic1");`

error: only a `panic!` in `if`-then statement
--> $DIR/if_then_panic_core.rs:44:5
|
LL | / if b.is_empty() && a.is_empty() {
LL | | panic!("panic2");
LL | | }
| |_____^ help: try: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`

error: only a `panic!` in `if`-then statement
--> $DIR/if_then_panic_core.rs:47:5
|
LL | / if a.is_empty() && !b.is_empty() {
LL | | panic!("panic3");
LL | | }
| |_____^ help: try: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`

error: only a `panic!` in `if`-then statement
--> $DIR/if_then_panic_core.rs:50:5
|
LL | / if b.is_empty() || a.is_empty() {
LL | | panic!("panic4");
LL | | }
| |_____^ help: try: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`

error: only a `panic!` in `if`-then statement
--> $DIR/if_then_panic_core.rs:53:5
|
LL | / if a.is_empty() || !b.is_empty() {
LL | | panic!("panic5");
LL | | }
| |_____^ help: try: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`

error: aborting due to 7 previous errors

67 changes: 67 additions & 0 deletions tests/ui/match_wild_err_arm_core.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
#![crate_type = "lib"]
#![no_std]
#![feature(exclusive_range_pattern)]
#![allow(clippy::match_same_arms)]
#![warn(clippy::match_wild_err_arm)]

fn foo() {}

fn match_wild_err_arm() {
let x: Result<i32, &str> = Ok(3);

match x {
Ok(3) => (),
Ok(_) => (),
Err(_) => panic!("err"),
}

match x {
Ok(3) => (),
Ok(_) => (),
Err(_) => panic!(),
}

match x {
Ok(3) => (),
Ok(_) => (),
Err(_) => {
panic!();
},
}

match x {
Ok(3) => (),
Ok(_) => (),
Err(_e) => panic!(),
}

// Allowed when used in `panic!`.
match x {
Ok(3) => (),
Ok(_) => (),
Err(_e) => panic!("{}", _e),
}

// Allowed when not with `panic!` block.
match x {
Ok(3) => (),
Ok(_) => (),
Err(_) => foo(),
}

// Allowed when used with `unreachable!`.
match x {
Ok(3) => (),
Ok(_) => (),
Err(_) => unreachable!(),
}

// Allowed when used with `unreachable!`.
match x {
Ok(3) => (),
Ok(_) => (),
Err(_) => {
unreachable!();
},
}
}
35 changes: 35 additions & 0 deletions tests/ui/match_wild_err_arm_core.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error: `Err(_)` matches all errors
--> $DIR/match_wild_err_arm_core.rs:15:9
|
LL | Err(_) => panic!("err"),
| ^^^^^^
|
= note: `-D clippy::match-wild-err-arm` implied by `-D warnings`
= note: match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable

error: `Err(_)` matches all errors
--> $DIR/match_wild_err_arm_core.rs:21:9
|
LL | Err(_) => panic!(),
| ^^^^^^
|
= note: match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable

error: `Err(_)` matches all errors
--> $DIR/match_wild_err_arm_core.rs:27:9
|
LL | Err(_) => {
| ^^^^^^
|
= note: match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable

error: `Err(_e)` matches all errors
--> $DIR/match_wild_err_arm_core.rs:35:9
|
LL | Err(_e) => panic!(),
| ^^^^^^^
|
= note: match each error separately or use the error output, or use `.except(msg)` if the error case is unreachable

error: aborting due to 4 previous errors

0 comments on commit 001d071

Please sign in to comment.