mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-23 13:13:34 +00:00
Fix handling of panic calls
This should make Clippy more resilient and will unblock #78343. This PR is made against rust-lang/rust to avoid the need for a subtree sync at @flip1995's suggestion in rust-lang/rust-clippy#6310.
This commit is contained in:
parent
27a15721c5
commit
4e4c4fb8aa
9 changed files with 106 additions and 54 deletions
|
@ -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);
|
||||
|
|
|
@ -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
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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};
|
||||
|
@ -93,27 +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 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));
|
||||
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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1196,7 +1196,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>,
|
||||
|
@ -1231,6 +1231,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
|
||||
|
|
|
@ -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,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(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", "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"];
|
||||
|
|
|
@ -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!();
|
||||
|
@ -33,9 +35,18 @@ fn unreachable() {
|
|||
let b = a + 2;
|
||||
}
|
||||
|
||||
fn core_versions() {
|
||||
use core::{panic, todo, unimplemented, unreachable};
|
||||
panic!();
|
||||
todo!();
|
||||
unimplemented!();
|
||||
unreachable!();
|
||||
}
|
||||
|
||||
fn main() {
|
||||
panic();
|
||||
todo();
|
||||
unimplemented();
|
||||
unreachable();
|
||||
core_versions();
|
||||
}
|
||||
|
|
|
@ -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,10 +79,34 @@ 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: aborting due to 12 previous errors
|
||||
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!();
|
||||
| ^^^^^^^^^^^^^^^
|
||||
|
||||
error: aborting due to 16 previous errors
|
||||
|
||||
|
|
Loading…
Reference in a new issue