From 9d84687a8fa430894ab1317e3db74d56266148af Mon Sep 17 00:00:00 2001 From: Camelid Date: Sun, 8 Nov 2020 14:22:44 -0800 Subject: [PATCH 1/7] Fix handling of panic calls --- clippy_lints/src/assertions_on_constants.rs | 5 ++--- clippy_lints/src/attrs.rs | 4 ++-- clippy_lints/src/fallible_impl_from.rs | 9 +++++---- clippy_lints/src/implicit_return.rs | 9 ++------- clippy_lints/src/panic_unimplemented.rs | 5 ++--- clippy_lints/src/utils/mod.rs | 20 +++++++++++++++++++- clippy_lints/src/utils/paths.rs | 4 ++++ 7 files changed, 36 insertions(+), 20 deletions(-) diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs index 982d5ecf8d02..a2ccb0369c4a 100644 --- a/clippy_lints/src/assertions_on_constants.rs +++ b/clippy_lints/src/assertions_on_constants.rs @@ -1,6 +1,5 @@ use crate::consts::{constant, Constant}; -use crate::utils::paths; -use crate::utils::{is_direct_expn_of, is_expn_of, match_function_call, snippet_opt, span_lint_and_help}; +use crate::utils::{is_direct_expn_of, is_expn_of, match_panic_call, snippet_opt, span_lint_and_help}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_hir::{Expr, ExprKind, PatKind, UnOp}; @@ -133,7 +132,7 @@ fn match_assert_with_message<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) if let ExprKind::Block(ref inner_block, _) = block_expr.kind; if let Some(begin_panic_call) = &inner_block.expr; // function call - if let Some(args) = match_function_call(cx, begin_panic_call, &paths::BEGIN_PANIC); + if let Some(args) = match_panic_call(cx, begin_panic_call); if args.len() == 1; // bind the second argument of the `assert!` macro if it exists if let panic_message = snippet_opt(cx, args[0].span); diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index a004abb58b83..537421bce758 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -1,7 +1,7 @@ //! checks for attributes use crate::utils::{ - first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_help, + first_line_of_span, is_present_in_source, match_panic_def_id, snippet_opt, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, without_block_comments, }; use if_chain::if_chain; @@ -513,7 +513,7 @@ fn is_relevant_expr(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_> typeck_results .qpath_res(qpath, path_expr.hir_id) .opt_def_id() - .map_or(true, |fun_id| !match_def_path(cx, fun_id, &paths::BEGIN_PANIC)) + .map_or(true, |fun_id| !match_panic_def_id(cx, fun_id)) } else { true } diff --git a/clippy_lints/src/fallible_impl_from.rs b/clippy_lints/src/fallible_impl_from.rs index fe817fe94f2e..509a4a4e15f6 100644 --- a/clippy_lints/src/fallible_impl_from.rs +++ b/clippy_lints/src/fallible_impl_from.rs @@ -1,5 +1,7 @@ -use crate::utils::paths::{BEGIN_PANIC, BEGIN_PANIC_FMT, FROM_TRAIT}; -use crate::utils::{is_expn_of, is_type_diagnostic_item, match_def_path, method_chain_args, span_lint_and_then}; +use crate::utils::paths::FROM_TRAIT; +use crate::utils::{ + is_expn_of, is_type_diagnostic_item, match_def_path, match_panic_def_id, method_chain_args, span_lint_and_then, +}; use if_chain::if_chain; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass}; @@ -84,8 +86,7 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_items: &[h if let ExprKind::Call(ref func_expr, _) = expr.kind; if let ExprKind::Path(QPath::Resolved(_, ref path)) = func_expr.kind; if let Some(path_def_id) = path.res.opt_def_id(); - if match_def_path(self.lcx, path_def_id, &BEGIN_PANIC) || - match_def_path(self.lcx, path_def_id, &BEGIN_PANIC_FMT); + if match_panic_def_id(self.lcx, path_def_id); if is_expn_of(expr.span, "unreachable").is_none(); then { self.result.push(expr.span); diff --git a/clippy_lints/src/implicit_return.rs b/clippy_lints/src/implicit_return.rs index 22c4fef32a32..ed7f3b9293db 100644 --- a/clippy_lints/src/implicit_return.rs +++ b/clippy_lints/src/implicit_return.rs @@ -1,8 +1,4 @@ -use crate::utils::{ - fn_has_unsatisfiable_preds, match_def_path, - paths::{BEGIN_PANIC, BEGIN_PANIC_FMT}, - snippet_opt, span_lint_and_then, -}; +use crate::utils::{fn_has_unsatisfiable_preds, match_panic_def_id, snippet_opt, span_lint_and_then}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; @@ -109,8 +105,7 @@ fn expr_match(cx: &LateContext<'_>, expr: &Expr<'_>) { if_chain! { if let ExprKind::Path(qpath) = &expr.kind; if let Some(path_def_id) = cx.qpath_res(qpath, expr.hir_id).opt_def_id(); - if match_def_path(cx, path_def_id, &BEGIN_PANIC) || - match_def_path(cx, path_def_id, &BEGIN_PANIC_FMT); + if match_panic_def_id(cx, path_def_id); then { } else { lint(cx, expr.span, expr.span, LINT_RETURN) diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs index 6379dffd22e3..26ca917ed44e 100644 --- a/clippy_lints/src/panic_unimplemented.rs +++ b/clippy_lints/src/panic_unimplemented.rs @@ -1,4 +1,4 @@ -use crate::utils::{is_direct_expn_of, is_expn_of, match_function_call, paths, span_lint}; +use crate::utils::{is_direct_expn_of, is_expn_of, match_panic_call, span_lint}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_hir::{Expr, ExprKind}; @@ -96,8 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented { if_chain! { if let ExprKind::Block(ref block, _) = expr.kind; if let Some(ref ex) = block.expr; - if let Some(params) = match_function_call(cx, ex, &paths::BEGIN_PANIC) - .or_else(|| match_function_call(cx, ex, &paths::BEGIN_PANIC_FMT)); + if let Some(params) = match_panic_call(cx, ex, ); then { let span = get_outer_span(expr); if is_expn_of(expr.span, "unimplemented").is_some() { diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index cba3a0502499..bcca02821b0f 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -1232,7 +1232,7 @@ pub fn has_iter_method(cx: &LateContext<'_>, probably_ref_ty: Ty<'_>) -> Option< /// Usage: /// /// ```rust,ignore -/// if let Some(args) = match_function_call(cx, begin_panic_call, &paths::BEGIN_PANIC); +/// if let Some(args) = match_function_call(cx, cmp_max_call, &paths::CMP_MAX); /// ``` pub fn match_function_call<'tcx>( cx: &LateContext<'tcx>, @@ -1267,6 +1267,24 @@ pub fn match_def_path<'tcx>(cx: &LateContext<'tcx>, did: DefId, syms: &[&str]) - cx.match_def_path(did, &syms) } +pub fn match_panic_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx [Expr<'tcx>]> { + match_function_call(cx, expr, &paths::BEGIN_PANIC) + .or_else(|| match_function_call(cx, expr, &paths::BEGIN_PANIC_FMT)) + .or_else(|| match_function_call(cx, expr, &paths::PANIC_ANY)) + .or_else(|| match_function_call(cx, expr, &paths::PANICKING_PANIC)) + .or_else(|| match_function_call(cx, expr, &paths::PANICKING_PANIC_FMT)) + .or_else(|| match_function_call(cx, expr, &paths::PANICKING_PANIC_STR)) +} + +pub fn match_panic_def_id(cx: &LateContext<'_>, did: DefId) -> bool { + match_def_path(cx, did, &paths::BEGIN_PANIC) + || match_def_path(cx, did, &paths::BEGIN_PANIC_FMT) + || match_def_path(cx, did, &paths::PANIC_ANY) + || match_def_path(cx, did, &paths::PANICKING_PANIC) + || match_def_path(cx, did, &paths::PANICKING_PANIC_FMT) + || match_def_path(cx, did, &paths::PANICKING_PANIC_STR) +} + /// Returns the list of condition expressions and the list of blocks in a /// sequence of `if/else`. /// E.g., this returns `([a, b], [c, d, e])` for the expression diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 2be5ff93f869..6324dbe20e60 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -78,6 +78,10 @@ pub const ORD: [&str; 3] = ["core", "cmp", "Ord"]; pub const OS_STRING: [&str; 4] = ["std", "ffi", "os_str", "OsString"]; pub const OS_STRING_AS_OS_STR: [&str; 5] = ["std", "ffi", "os_str", "OsString", "as_os_str"]; pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"]; +pub const PANICKING_PANIC: [&str; 3] = ["core", "panicking", "panic"]; +pub const PANICKING_PANIC_FMT: [&str; 3] = ["core", "panicking", "panic_fmt"]; +pub const PANICKING_PANIC_STR: [&str; 3] = ["core", "panicking", "panic_str"]; +pub const PANIC_ANY: [&str; 3] = ["std", "panicking", "panic_any"]; pub const PARKING_LOT_MUTEX_GUARD: [&str; 2] = ["parking_lot", "MutexGuard"]; pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 2] = ["parking_lot", "RwLockReadGuard"]; pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 2] = ["parking_lot", "RwLockWriteGuard"]; From 26a4d124d90a42b575c996126a317bd3585fc6c4 Mon Sep 17 00:00:00 2001 From: Camelid Date: Sun, 8 Nov 2020 16:09:32 -0800 Subject: [PATCH 2/7] Make PANIC path consts private to prevent misuse They should be used through the `match_panic_call` and `match_panic_def_id` functions to ensure consistency across Clippy. --- clippy_lints/src/utils/paths.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 6324dbe20e60..4ffee0dfc32f 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -8,8 +8,8 @@ pub const ANY_TRAIT: [&str; 3] = ["std", "any", "Any"]; pub const ARC_PTR_EQ: [&str; 4] = ["alloc", "sync", "Arc", "ptr_eq"]; pub const ASMUT_TRAIT: [&str; 3] = ["core", "convert", "AsMut"]; pub const ASREF_TRAIT: [&str; 3] = ["core", "convert", "AsRef"]; -pub const BEGIN_PANIC: [&str; 3] = ["std", "panicking", "begin_panic"]; -pub const BEGIN_PANIC_FMT: [&str; 3] = ["std", "panicking", "begin_panic_fmt"]; +pub(super) const BEGIN_PANIC: [&str; 3] = ["std", "panicking", "begin_panic"]; +pub(super) const BEGIN_PANIC_FMT: [&str; 3] = ["std", "panicking", "begin_panic_fmt"]; pub const BINARY_HEAP: [&str; 4] = ["alloc", "collections", "binary_heap", "BinaryHeap"]; pub const BORROW_TRAIT: [&str; 3] = ["core", "borrow", "Borrow"]; pub const BOX: [&str; 3] = ["alloc", "boxed", "Box"]; @@ -78,10 +78,10 @@ pub const ORD: [&str; 3] = ["core", "cmp", "Ord"]; pub const OS_STRING: [&str; 4] = ["std", "ffi", "os_str", "OsString"]; pub const OS_STRING_AS_OS_STR: [&str; 5] = ["std", "ffi", "os_str", "OsString", "as_os_str"]; pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"]; -pub const PANICKING_PANIC: [&str; 3] = ["core", "panicking", "panic"]; -pub const PANICKING_PANIC_FMT: [&str; 3] = ["core", "panicking", "panic_fmt"]; -pub const PANICKING_PANIC_STR: [&str; 3] = ["core", "panicking", "panic_str"]; -pub const PANIC_ANY: [&str; 3] = ["std", "panicking", "panic_any"]; +pub(super) const PANICKING_PANIC: [&str; 3] = ["core", "panicking", "panic"]; +pub(super) const PANICKING_PANIC_FMT: [&str; 3] = ["core", "panicking", "panic_fmt"]; +pub(super) const PANICKING_PANIC_STR: [&str; 3] = ["core", "panicking", "panic_str"]; +pub(super) const PANIC_ANY: [&str; 3] = ["std", "panicking", "panic_any"]; pub const PARKING_LOT_MUTEX_GUARD: [&str; 2] = ["parking_lot", "MutexGuard"]; pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 2] = ["parking_lot", "RwLockReadGuard"]; pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 2] = ["parking_lot", "RwLockWriteGuard"]; From e5b0c14554ca72010104f0d4e3536771d9c323fc Mon Sep 17 00:00:00 2001 From: Camelid Date: Sun, 8 Nov 2020 18:55:51 -0800 Subject: [PATCH 3/7] Fix mistake in `panic_any` path --- clippy_lints/src/utils/paths.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 4ffee0dfc32f..137f5d18b664 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -81,7 +81,7 @@ pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to pub(super) const PANICKING_PANIC: [&str; 3] = ["core", "panicking", "panic"]; pub(super) const PANICKING_PANIC_FMT: [&str; 3] = ["core", "panicking", "panic_fmt"]; pub(super) const PANICKING_PANIC_STR: [&str; 3] = ["core", "panicking", "panic_str"]; -pub(super) const PANIC_ANY: [&str; 3] = ["std", "panicking", "panic_any"]; +pub(super) const PANIC_ANY: [&str; 3] = ["std", "panic", "panic_any"]; pub const PARKING_LOT_MUTEX_GUARD: [&str; 2] = ["parking_lot", "MutexGuard"]; pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 2] = ["parking_lot", "RwLockReadGuard"]; pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 2] = ["parking_lot", "RwLockWriteGuard"]; From 11280fefa2cdbf10043818f0aa69565681eff5d5 Mon Sep 17 00:00:00 2001 From: Camelid Date: Sun, 8 Nov 2020 20:40:07 -0800 Subject: [PATCH 4/7] Add test for `core` versions of the panic macros --- clippy_lints/src/panic_unimplemented.rs | 2 +- tests/ui/panicking_macros.rs | 8 ++++++++ tests/ui/panicking_macros.stderr | 20 +++++++++++++++++++- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs index 26ca917ed44e..7d7f615c6ea0 100644 --- a/clippy_lints/src/panic_unimplemented.rs +++ b/clippy_lints/src/panic_unimplemented.rs @@ -96,7 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented { if_chain! { if let ExprKind::Block(ref block, _) = expr.kind; if let Some(ref ex) = block.expr; - if let Some(params) = match_panic_call(cx, ex, ); + if let Some(params) = match_panic_call(cx, ex); then { let span = get_outer_span(expr); if is_expn_of(expr.span, "unimplemented").is_some() { diff --git a/tests/ui/panicking_macros.rs b/tests/ui/panicking_macros.rs index f91ccfaed743..47d845e8474f 100644 --- a/tests/ui/panicking_macros.rs +++ b/tests/ui/panicking_macros.rs @@ -33,9 +33,17 @@ fn unreachable() { let b = a + 2; } +fn core_versions() { + core::panic!(); + core::todo!(); + core::unimplemented!(); + core::unreachable!(); +} + fn main() { panic(); todo(); unimplemented(); unreachable(); + core_versions(); } diff --git a/tests/ui/panicking_macros.stderr b/tests/ui/panicking_macros.stderr index 37c11d72a574..ddc46676a12d 100644 --- a/tests/ui/panicking_macros.stderr +++ b/tests/ui/panicking_macros.stderr @@ -84,5 +84,23 @@ error: `unreachable` should not be present in production code LL | unreachable!("{} {}", "panic with", "multiple arguments"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 12 previous errors +error: `panic` should not be present in production code + --> $DIR/panicking_macros.rs:38:5 + | +LL | core::todo!(); + | ^^^^^^^^^^^^^^ + +error: `panic` should not be present in production code + --> $DIR/panicking_macros.rs:39:5 + | +LL | core::unimplemented!(); + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error: `panic` should not be present in production code + --> $DIR/panicking_macros.rs:40:5 + | +LL | core::unreachable!(); + | ^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 15 previous errors From 0cbe92543feec4753a01c5f5e0ca5f61e202e480 Mon Sep 17 00:00:00 2001 From: Camelid Date: Wed, 11 Nov 2020 12:28:43 -0800 Subject: [PATCH 5/7] Sidestep bug with qualified macros in test --- tests/ui/panicking_macros.rs | 11 +++++--- tests/ui/panicking_macros.stderr | 46 +++++++++++++------------------- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/tests/ui/panicking_macros.rs b/tests/ui/panicking_macros.rs index 47d845e8474f..77fcb8dfd02f 100644 --- a/tests/ui/panicking_macros.rs +++ b/tests/ui/panicking_macros.rs @@ -1,6 +1,8 @@ #![warn(clippy::unimplemented, clippy::unreachable, clippy::todo, clippy::panic)] #![allow(clippy::assertions_on_constants)] +extern crate core; + fn panic() { let a = 2; panic!(); @@ -34,10 +36,11 @@ fn unreachable() { } fn core_versions() { - core::panic!(); - core::todo!(); - core::unimplemented!(); - core::unreachable!(); + use core::{panic, todo, unimplemented, unreachable}; + panic!(); + todo!(); + unimplemented!(); + unreachable!(); } fn main() { diff --git a/tests/ui/panicking_macros.stderr b/tests/ui/panicking_macros.stderr index ddc46676a12d..d6438c6bc986 100644 --- a/tests/ui/panicking_macros.stderr +++ b/tests/ui/panicking_macros.stderr @@ -1,5 +1,5 @@ error: `panic` should not be present in production code - --> $DIR/panicking_macros.rs:6:5 + --> $DIR/panicking_macros.rs:8:5 | LL | panic!(); | ^^^^^^^^^ @@ -7,7 +7,7 @@ LL | panic!(); = note: `-D clippy::panic` implied by `-D warnings` error: `panic` should not be present in production code - --> $DIR/panicking_macros.rs:7:5 + --> $DIR/panicking_macros.rs:9:5 | LL | panic!("message"); | ^^^^^^^^^^^^^^^^^^ @@ -15,7 +15,7 @@ LL | panic!("message"); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `panic` should not be present in production code - --> $DIR/panicking_macros.rs:8:5 + --> $DIR/panicking_macros.rs:10:5 | LL | panic!("{} {}", "panic with", "multiple arguments"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -23,7 +23,7 @@ LL | panic!("{} {}", "panic with", "multiple arguments"); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `todo` should not be present in production code - --> $DIR/panicking_macros.rs:14:5 + --> $DIR/panicking_macros.rs:16:5 | LL | todo!(); | ^^^^^^^^ @@ -31,19 +31,19 @@ LL | todo!(); = note: `-D clippy::todo` implied by `-D warnings` error: `todo` should not be present in production code - --> $DIR/panicking_macros.rs:15:5 + --> $DIR/panicking_macros.rs:17:5 | LL | todo!("message"); | ^^^^^^^^^^^^^^^^^ error: `todo` should not be present in production code - --> $DIR/panicking_macros.rs:16:5 + --> $DIR/panicking_macros.rs:18:5 | LL | todo!("{} {}", "panic with", "multiple arguments"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `unimplemented` should not be present in production code - --> $DIR/panicking_macros.rs:22:5 + --> $DIR/panicking_macros.rs:24:5 | LL | unimplemented!(); | ^^^^^^^^^^^^^^^^^ @@ -51,19 +51,19 @@ LL | unimplemented!(); = note: `-D clippy::unimplemented` implied by `-D warnings` error: `unimplemented` should not be present in production code - --> $DIR/panicking_macros.rs:23:5 + --> $DIR/panicking_macros.rs:25:5 | LL | unimplemented!("message"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `unimplemented` should not be present in production code - --> $DIR/panicking_macros.rs:24:5 + --> $DIR/panicking_macros.rs:26:5 | LL | unimplemented!("{} {}", "panic with", "multiple arguments"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `unreachable` should not be present in production code - --> $DIR/panicking_macros.rs:30:5 + --> $DIR/panicking_macros.rs:32:5 | LL | unreachable!(); | ^^^^^^^^^^^^^^^ @@ -71,7 +71,7 @@ LL | unreachable!(); = note: `-D clippy::unreachable` implied by `-D warnings` error: `unreachable` should not be present in production code - --> $DIR/panicking_macros.rs:31:5 + --> $DIR/panicking_macros.rs:33:5 | LL | unreachable!("message"); | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -79,28 +79,18 @@ LL | unreachable!("message"); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `unreachable` should not be present in production code - --> $DIR/panicking_macros.rs:32:5 + --> $DIR/panicking_macros.rs:34:5 | LL | unreachable!("{} {}", "panic with", "multiple arguments"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: `panic` should not be present in production code - --> $DIR/panicking_macros.rs:38:5 - | -LL | core::todo!(); - | ^^^^^^^^^^^^^^ - -error: `panic` should not be present in production code - --> $DIR/panicking_macros.rs:39:5 +error: `unreachable` should not be present in production code + --> $DIR/panicking_macros.rs:43:5 | -LL | core::unimplemented!(); - | ^^^^^^^^^^^^^^^^^^^^^^^ - -error: `panic` should not be present in production code - --> $DIR/panicking_macros.rs:40:5 +LL | unreachable!(); + | ^^^^^^^^^^^^^^^ | -LL | core::unreachable!(); - | ^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 15 previous errors +error: aborting due to 13 previous errors From b9f1290098a19fae8fea2448c5f58f373f589f01 Mon Sep 17 00:00:00 2001 From: Camelid Date: Thu, 12 Nov 2020 13:57:04 -0800 Subject: [PATCH 6/7] Fix pre-existing bug Co-authored-by: Mara Bos --- clippy_lints/src/panic_unimplemented.rs | 4 +--- tests/ui/panicking_macros.stderr | 22 +++++++++++++++++++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs index 7d7f615c6ea0..69d58af0dbab 100644 --- a/clippy_lints/src/panic_unimplemented.rs +++ b/clippy_lints/src/panic_unimplemented.rs @@ -94,9 +94,7 @@ declare_lint_pass!(PanicUnimplemented => [PANIC_PARAMS, UNIMPLEMENTED, UNREACHAB impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if_chain! { - if let ExprKind::Block(ref block, _) = expr.kind; - if let Some(ref ex) = block.expr; - if let Some(params) = match_panic_call(cx, ex); + if let Some(params) = match_panic_call(cx, expr); then { let span = get_outer_span(expr); if is_expn_of(expr.span, "unimplemented").is_some() { diff --git a/tests/ui/panicking_macros.stderr b/tests/ui/panicking_macros.stderr index d6438c6bc986..83234c0ed92c 100644 --- a/tests/ui/panicking_macros.stderr +++ b/tests/ui/panicking_macros.stderr @@ -84,13 +84,29 @@ error: `unreachable` should not be present in production code LL | unreachable!("{} {}", "panic with", "multiple arguments"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +error: `panic` should not be present in production code + --> $DIR/panicking_macros.rs:40:5 + | +LL | panic!(); + | ^^^^^^^^^ + +error: `todo` should not be present in production code + --> $DIR/panicking_macros.rs:41:5 + | +LL | todo!(); + | ^^^^^^^^ + +error: `unimplemented` should not be present in production code + --> $DIR/panicking_macros.rs:42:5 + | +LL | unimplemented!(); + | ^^^^^^^^^^^^^^^^^ + error: `unreachable` should not be present in production code --> $DIR/panicking_macros.rs:43:5 | LL | unreachable!(); | ^^^^^^^^^^^^^^^ - | - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 13 previous errors +error: aborting due to 16 previous errors From 761cdf2b553f1e13e8e0e076f15e83817f35e79c Mon Sep 17 00:00:00 2001 From: Camelid Date: Tue, 17 Nov 2020 12:10:49 -0800 Subject: [PATCH 7/7] Remove unnecessary `if_chain` --- clippy_lints/src/panic_unimplemented.rs | 39 +++++++++++++------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs index 69d58af0dbab..3d888fe73257 100644 --- a/clippy_lints/src/panic_unimplemented.rs +++ b/clippy_lints/src/panic_unimplemented.rs @@ -93,24 +93,27 @@ declare_lint_pass!(PanicUnimplemented => [PANIC_PARAMS, UNIMPLEMENTED, UNREACHAB impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if_chain! { - if let Some(params) = match_panic_call(cx, expr); - then { - let span = get_outer_span(expr); - if is_expn_of(expr.span, "unimplemented").is_some() { - span_lint(cx, UNIMPLEMENTED, span, - "`unimplemented` should not be present in production code"); - } else if is_expn_of(expr.span, "todo").is_some() { - span_lint(cx, TODO, span, - "`todo` should not be present in production code"); - } else if is_expn_of(expr.span, "unreachable").is_some() { - span_lint(cx, UNREACHABLE, span, - "`unreachable` should not be present in production code"); - } else if is_expn_of(expr.span, "panic").is_some() { - span_lint(cx, PANIC, span, - "`panic` should not be present in production code"); - match_panic(params, expr, cx); - } + if let Some(params) = match_panic_call(cx, expr) { + let span = get_outer_span(expr); + if is_expn_of(expr.span, "unimplemented").is_some() { + span_lint( + cx, + UNIMPLEMENTED, + span, + "`unimplemented` should not be present in production code", + ); + } else if is_expn_of(expr.span, "todo").is_some() { + span_lint(cx, TODO, span, "`todo` should not be present in production code"); + } else if is_expn_of(expr.span, "unreachable").is_some() { + span_lint( + cx, + UNREACHABLE, + span, + "`unreachable` should not be present in production code", + ); + } else if is_expn_of(expr.span, "panic").is_some() { + span_lint(cx, PANIC, span, "`panic` should not be present in production code"); + match_panic(params, expr, cx); } } }