From 001d0713376ac9e8757a0b0b969259f3116b0ab2 Mon Sep 17 00:00:00 2001 From: Gary Guo <gary@garyguo.net> Date: Thu, 21 Oct 2021 00:08:18 +0100 Subject: [PATCH] Fix if_then_panic for `#![no_std]` and Rust 2021 --- clippy_lints/src/if_then_panic.rs | 15 ++++-- clippy_lints/src/matches.rs | 24 +++++---- clippy_utils/src/higher.rs | 4 +- tests/ui/if_then_panic_core.fixed | 42 ++++++++++++++++ tests/ui/if_then_panic_core.rs | 56 +++++++++++++++++++++ tests/ui/if_then_panic_core.stderr | 60 ++++++++++++++++++++++ tests/ui/match_wild_err_arm_core.rs | 67 +++++++++++++++++++++++++ tests/ui/match_wild_err_arm_core.stderr | 35 +++++++++++++ 8 files changed, 286 insertions(+), 17 deletions(-) create mode 100644 tests/ui/if_then_panic_core.fixed create mode 100644 tests/ui/if_then_panic_core.rs create mode 100644 tests/ui/if_then_panic_core.stderr create mode 100644 tests/ui/match_wild_err_arm_core.rs create mode 100644 tests/ui/match_wild_err_arm_core.stderr diff --git a/clippy_lints/src/if_then_panic.rs b/clippy_lints/src/if_then_panic.rs index e8cea5529e88..8eab850b91d3 100644 --- a/clippy_lints/src/if_then_panic.rs +++ b/clippy_lints/src/if_then_panic.rs @@ -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 diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index b643fba5d328..b16f6d6c0588 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -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, @@ -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<'_>) diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index fb4d53e2845d..9a7c2eb6ce40 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -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); diff --git a/tests/ui/if_then_panic_core.fixed b/tests/ui/if_then_panic_core.fixed new file mode 100644 index 000000000000..2322fba39803 --- /dev/null +++ b/tests/ui/if_then_panic_core.fixed @@ -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"); +} diff --git a/tests/ui/if_then_panic_core.rs b/tests/ui/if_then_panic_core.rs new file mode 100644 index 000000000000..fb7be9a25c19 --- /dev/null +++ b/tests/ui/if_then_panic_core.rs @@ -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"); + } +} diff --git a/tests/ui/if_then_panic_core.stderr b/tests/ui/if_then_panic_core.stderr new file mode 100644 index 000000000000..688ed0b98a4a --- /dev/null +++ b/tests/ui/if_then_panic_core.stderr @@ -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 + diff --git a/tests/ui/match_wild_err_arm_core.rs b/tests/ui/match_wild_err_arm_core.rs new file mode 100644 index 000000000000..8f998ae34bf2 --- /dev/null +++ b/tests/ui/match_wild_err_arm_core.rs @@ -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!(); + }, + } +} diff --git a/tests/ui/match_wild_err_arm_core.stderr b/tests/ui/match_wild_err_arm_core.stderr new file mode 100644 index 000000000000..e39454de9c44 --- /dev/null +++ b/tests/ui/match_wild_err_arm_core.stderr @@ -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 +