Skip to content

Commit 577ebc8

Browse files
m-ou-sem-ou-se
authored
Rollup merge of rust-lang#79145 - camelid:clippy-fix-panics, r=flip1995
Fix handling of panic calls This should make Clippy more resilient and will unblock rust-lang#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. r? ``@flip1995`` cc ``@m-ou-se``
2 parents a467c51 + 4e4c4fb commit 577ebc8

9 files changed

+106
-54
lines changed

‎clippy_lints/src/assertions_on_constants.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use crate::consts::{constant, Constant};
2-
use crate::utils::paths;
3-
use crate::utils::{is_direct_expn_of, is_expn_of, match_function_call, snippet_opt, span_lint_and_help};
2+
use crate::utils::{is_direct_expn_of, is_expn_of, match_panic_call, snippet_opt, span_lint_and_help};
43
use if_chain::if_chain;
54
use rustc_ast::ast::LitKind;
65
use rustc_hir::{Expr, ExprKind, PatKind, UnOp};
@@ -133,7 +132,7 @@ fn match_assert_with_message<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>)
133132
if let ExprKind::Block(ref inner_block, _) = block_expr.kind;
134133
if let Some(begin_panic_call) = &inner_block.expr;
135134
// function call
136-
if let Some(args) = match_function_call(cx, begin_panic_call, &paths::BEGIN_PANIC);
135+
if let Some(args) = match_panic_call(cx, begin_panic_call);
137136
if args.len() == 1;
138137
// bind the second argument of the `assert!` macro if it exists
139138
if let panic_message = snippet_opt(cx, args[0].span);

‎clippy_lints/src/attrs.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! checks for attributes
22
33
use crate::utils::{
4-
first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_help,
4+
first_line_of_span, is_present_in_source, match_panic_def_id, snippet_opt, span_lint, span_lint_and_help,
55
span_lint_and_sugg, span_lint_and_then, without_block_comments,
66
};
77
use if_chain::if_chain;
@@ -513,7 +513,7 @@ fn is_relevant_expr(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_>
513513
typeck_results
514514
.qpath_res(qpath, path_expr.hir_id)
515515
.opt_def_id()
516-
.map_or(true, |fun_id| !match_def_path(cx, fun_id, &paths::BEGIN_PANIC))
516+
.map_or(true, |fun_id| !match_panic_def_id(cx, fun_id))
517517
} else {
518518
true
519519
}

‎clippy_lints/src/fallible_impl_from.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
use crate::utils::paths::{BEGIN_PANIC, BEGIN_PANIC_FMT, FROM_TRAIT};
2-
use crate::utils::{is_expn_of, is_type_diagnostic_item, match_def_path, method_chain_args, span_lint_and_then};
1+
use crate::utils::paths::FROM_TRAIT;
2+
use crate::utils::{
3+
is_expn_of, is_type_diagnostic_item, match_def_path, match_panic_def_id, method_chain_args, span_lint_and_then,
4+
};
35
use if_chain::if_chain;
46
use rustc_hir as hir;
57
use rustc_lint::{LateContext, LateLintPass};
@@ -84,8 +86,7 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_items: &[h
8486
if let ExprKind::Call(ref func_expr, _) = expr.kind;
8587
if let ExprKind::Path(QPath::Resolved(_, ref path)) = func_expr.kind;
8688
if let Some(path_def_id) = path.res.opt_def_id();
87-
if match_def_path(self.lcx, path_def_id, &BEGIN_PANIC) ||
88-
match_def_path(self.lcx, path_def_id, &BEGIN_PANIC_FMT);
89+
if match_panic_def_id(self.lcx, path_def_id);
8990
if is_expn_of(expr.span, "unreachable").is_none();
9091
then {
9192
self.result.push(expr.span);

‎clippy_lints/src/implicit_return.rs

+2-7
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
use crate::utils::{
2-
fn_has_unsatisfiable_preds, match_def_path,
3-
paths::{BEGIN_PANIC, BEGIN_PANIC_FMT},
4-
snippet_opt, span_lint_and_then,
5-
};
1+
use crate::utils::{fn_has_unsatisfiable_preds, match_panic_def_id, snippet_opt, span_lint_and_then};
62
use if_chain::if_chain;
73
use rustc_errors::Applicability;
84
use rustc_hir::intravisit::FnKind;
@@ -109,8 +105,7 @@ fn expr_match(cx: &LateContext<'_>, expr: &Expr<'_>) {
109105
if_chain! {
110106
if let ExprKind::Path(qpath) = &expr.kind;
111107
if let Some(path_def_id) = cx.qpath_res(qpath, expr.hir_id).opt_def_id();
112-
if match_def_path(cx, path_def_id, &BEGIN_PANIC) ||
113-
match_def_path(cx, path_def_id, &BEGIN_PANIC_FMT);
108+
if match_panic_def_id(cx, path_def_id);
114109
then { }
115110
else {
116111
lint(cx, expr.span, expr.span, LINT_RETURN)

‎clippy_lints/src/panic_unimplemented.rs

+22-22
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::utils::{is_direct_expn_of, is_expn_of, match_function_call, paths, span_lint};
1+
use crate::utils::{is_direct_expn_of, is_expn_of, match_panic_call, span_lint};
22
use if_chain::if_chain;
33
use rustc_ast::ast::LitKind;
44
use rustc_hir::{Expr, ExprKind};
@@ -93,27 +93,27 @@ declare_lint_pass!(PanicUnimplemented => [PANIC_PARAMS, UNIMPLEMENTED, UNREACHAB
9393

9494
impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented {
9595
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
96-
if_chain! {
97-
if let ExprKind::Block(ref block, _) = expr.kind;
98-
if let Some(ref ex) = block.expr;
99-
if let Some(params) = match_function_call(cx, ex, &paths::BEGIN_PANIC)
100-
.or_else(|| match_function_call(cx, ex, &paths::BEGIN_PANIC_FMT));
101-
then {
102-
let span = get_outer_span(expr);
103-
if is_expn_of(expr.span, "unimplemented").is_some() {
104-
span_lint(cx, UNIMPLEMENTED, span,
105-
"`unimplemented` should not be present in production code");
106-
} else if is_expn_of(expr.span, "todo").is_some() {
107-
span_lint(cx, TODO, span,
108-
"`todo` should not be present in production code");
109-
} else if is_expn_of(expr.span, "unreachable").is_some() {
110-
span_lint(cx, UNREACHABLE, span,
111-
"`unreachable` should not be present in production code");
112-
} else if is_expn_of(expr.span, "panic").is_some() {
113-
span_lint(cx, PANIC, span,
114-
"`panic` should not be present in production code");
115-
match_panic(params, expr, cx);
116-
}
96+
if let Some(params) = match_panic_call(cx, expr) {
97+
let span = get_outer_span(expr);
98+
if is_expn_of(expr.span, "unimplemented").is_some() {
99+
span_lint(
100+
cx,
101+
UNIMPLEMENTED,
102+
span,
103+
"`unimplemented` should not be present in production code",
104+
);
105+
} else if is_expn_of(expr.span, "todo").is_some() {
106+
span_lint(cx, TODO, span, "`todo` should not be present in production code");
107+
} else if is_expn_of(expr.span, "unreachable").is_some() {
108+
span_lint(
109+
cx,
110+
UNREACHABLE,
111+
span,
112+
"`unreachable` should not be present in production code",
113+
);
114+
} else if is_expn_of(expr.span, "panic").is_some() {
115+
span_lint(cx, PANIC, span, "`panic` should not be present in production code");
116+
match_panic(params, expr, cx);
117117
}
118118
}
119119
}

‎clippy_lints/src/utils/mod.rs

+19-1
Original file line numberDiff line numberDiff line change
@@ -1196,7 +1196,7 @@ pub fn has_iter_method(cx: &LateContext<'_>, probably_ref_ty: Ty<'_>) -> Option<
11961196
/// Usage:
11971197
///
11981198
/// ```rust,ignore
1199-
/// if let Some(args) = match_function_call(cx, begin_panic_call, &paths::BEGIN_PANIC);
1199+
/// if let Some(args) = match_function_call(cx, cmp_max_call, &paths::CMP_MAX);
12001200
/// ```
12011201
pub fn match_function_call<'tcx>(
12021202
cx: &LateContext<'tcx>,
@@ -1231,6 +1231,24 @@ pub fn match_def_path<'tcx>(cx: &LateContext<'tcx>, did: DefId, syms: &[&str]) -
12311231
cx.match_def_path(did, &syms)
12321232
}
12331233

1234+
pub fn match_panic_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx [Expr<'tcx>]> {
1235+
match_function_call(cx, expr, &paths::BEGIN_PANIC)
1236+
.or_else(|| match_function_call(cx, expr, &paths::BEGIN_PANIC_FMT))
1237+
.or_else(|| match_function_call(cx, expr, &paths::PANIC_ANY))
1238+
.or_else(|| match_function_call(cx, expr, &paths::PANICKING_PANIC))
1239+
.or_else(|| match_function_call(cx, expr, &paths::PANICKING_PANIC_FMT))
1240+
.or_else(|| match_function_call(cx, expr, &paths::PANICKING_PANIC_STR))
1241+
}
1242+
1243+
pub fn match_panic_def_id(cx: &LateContext<'_>, did: DefId) -> bool {
1244+
match_def_path(cx, did, &paths::BEGIN_PANIC)
1245+
|| match_def_path(cx, did, &paths::BEGIN_PANIC_FMT)
1246+
|| match_def_path(cx, did, &paths::PANIC_ANY)
1247+
|| match_def_path(cx, did, &paths::PANICKING_PANIC)
1248+
|| match_def_path(cx, did, &paths::PANICKING_PANIC_FMT)
1249+
|| match_def_path(cx, did, &paths::PANICKING_PANIC_STR)
1250+
}
1251+
12341252
/// Returns the list of condition expressions and the list of blocks in a
12351253
/// sequence of `if/else`.
12361254
/// E.g., this returns `([a, b], [c, d, e])` for the expression

‎clippy_lints/src/utils/paths.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ pub const ANY_TRAIT: [&str; 3] = ["std", "any", "Any"];
88
pub const ARC_PTR_EQ: [&str; 4] = ["alloc", "sync", "Arc", "ptr_eq"];
99
pub const ASMUT_TRAIT: [&str; 3] = ["core", "convert", "AsMut"];
1010
pub const ASREF_TRAIT: [&str; 3] = ["core", "convert", "AsRef"];
11-
pub const BEGIN_PANIC: [&str; 3] = ["std", "panicking", "begin_panic"];
12-
pub const BEGIN_PANIC_FMT: [&str; 3] = ["std", "panicking", "begin_panic_fmt"];
11+
pub(super) const BEGIN_PANIC: [&str; 3] = ["std", "panicking", "begin_panic"];
12+
pub(super) const BEGIN_PANIC_FMT: [&str; 3] = ["std", "panicking", "begin_panic_fmt"];
1313
pub const BINARY_HEAP: [&str; 4] = ["alloc", "collections", "binary_heap", "BinaryHeap"];
1414
pub const BORROW_TRAIT: [&str; 3] = ["core", "borrow", "Borrow"];
1515
pub const BOX: [&str; 3] = ["alloc", "boxed", "Box"];
@@ -78,6 +78,10 @@ pub const ORD: [&str; 3] = ["core", "cmp", "Ord"];
7878
pub const OS_STRING: [&str; 4] = ["std", "ffi", "os_str", "OsString"];
7979
pub const OS_STRING_AS_OS_STR: [&str; 5] = ["std", "ffi", "os_str", "OsString", "as_os_str"];
8080
pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"];
81+
pub(super) const PANICKING_PANIC: [&str; 3] = ["core", "panicking", "panic"];
82+
pub(super) const PANICKING_PANIC_FMT: [&str; 3] = ["core", "panicking", "panic_fmt"];
83+
pub(super) const PANICKING_PANIC_STR: [&str; 3] = ["core", "panicking", "panic_str"];
84+
pub(super) const PANIC_ANY: [&str; 3] = ["std", "panic", "panic_any"];
8185
pub const PARKING_LOT_MUTEX_GUARD: [&str; 2] = ["parking_lot", "MutexGuard"];
8286
pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 2] = ["parking_lot", "RwLockReadGuard"];
8387
pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 2] = ["parking_lot", "RwLockWriteGuard"];

‎tests/ui/panicking_macros.rs

+11
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#![warn(clippy::unimplemented, clippy::unreachable, clippy::todo, clippy::panic)]
22
#![allow(clippy::assertions_on_constants)]
33

4+
extern crate core;
5+
46
fn panic() {
57
let a = 2;
68
panic!();
@@ -33,9 +35,18 @@ fn unreachable() {
3335
let b = a + 2;
3436
}
3537

38+
fn core_versions() {
39+
use core::{panic, todo, unimplemented, unreachable};
40+
panic!();
41+
todo!();
42+
unimplemented!();
43+
unreachable!();
44+
}
45+
3646
fn main() {
3747
panic();
3848
todo();
3949
unimplemented();
4050
unreachable();
51+
core_versions();
4152
}

‎tests/ui/panicking_macros.stderr

+37-13
Original file line numberDiff line numberDiff line change
@@ -1,88 +1,112 @@
11
error: `panic` should not be present in production code
2-
--> $DIR/panicking_macros.rs:6:5
2+
--> $DIR/panicking_macros.rs:8:5
33
|
44
LL | panic!();
55
| ^^^^^^^^^
66
|
77
= note: `-D clippy::panic` implied by `-D warnings`
88

99
error: `panic` should not be present in production code
10-
--> $DIR/panicking_macros.rs:7:5
10+
--> $DIR/panicking_macros.rs:9:5
1111
|
1212
LL | panic!("message");
1313
| ^^^^^^^^^^^^^^^^^^
1414
|
1515
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
1616

1717
error: `panic` should not be present in production code
18-
--> $DIR/panicking_macros.rs:8:5
18+
--> $DIR/panicking_macros.rs:10:5
1919
|
2020
LL | panic!("{} {}", "panic with", "multiple arguments");
2121
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2222
|
2323
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
2424

2525
error: `todo` should not be present in production code
26-
--> $DIR/panicking_macros.rs:14:5
26+
--> $DIR/panicking_macros.rs:16:5
2727
|
2828
LL | todo!();
2929
| ^^^^^^^^
3030
|
3131
= note: `-D clippy::todo` implied by `-D warnings`
3232

3333
error: `todo` should not be present in production code
34-
--> $DIR/panicking_macros.rs:15:5
34+
--> $DIR/panicking_macros.rs:17:5
3535
|
3636
LL | todo!("message");
3737
| ^^^^^^^^^^^^^^^^^
3838

3939
error: `todo` should not be present in production code
40-
--> $DIR/panicking_macros.rs:16:5
40+
--> $DIR/panicking_macros.rs:18:5
4141
|
4242
LL | todo!("{} {}", "panic with", "multiple arguments");
4343
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4444

4545
error: `unimplemented` should not be present in production code
46-
--> $DIR/panicking_macros.rs:22:5
46+
--> $DIR/panicking_macros.rs:24:5
4747
|
4848
LL | unimplemented!();
4949
| ^^^^^^^^^^^^^^^^^
5050
|
5151
= note: `-D clippy::unimplemented` implied by `-D warnings`
5252

5353
error: `unimplemented` should not be present in production code
54-
--> $DIR/panicking_macros.rs:23:5
54+
--> $DIR/panicking_macros.rs:25:5
5555
|
5656
LL | unimplemented!("message");
5757
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
5858

5959
error: `unimplemented` should not be present in production code
60-
--> $DIR/panicking_macros.rs:24:5
60+
--> $DIR/panicking_macros.rs:26:5
6161
|
6262
LL | unimplemented!("{} {}", "panic with", "multiple arguments");
6363
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6464

6565
error: `unreachable` should not be present in production code
66-
--> $DIR/panicking_macros.rs:30:5
66+
--> $DIR/panicking_macros.rs:32:5
6767
|
6868
LL | unreachable!();
6969
| ^^^^^^^^^^^^^^^
7070
|
7171
= note: `-D clippy::unreachable` implied by `-D warnings`
7272

7373
error: `unreachable` should not be present in production code
74-
--> $DIR/panicking_macros.rs:31:5
74+
--> $DIR/panicking_macros.rs:33:5
7575
|
7676
LL | unreachable!("message");
7777
| ^^^^^^^^^^^^^^^^^^^^^^^^
7878
|
7979
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
8080

8181
error: `unreachable` should not be present in production code
82-
--> $DIR/panicking_macros.rs:32:5
82+
--> $DIR/panicking_macros.rs:34:5
8383
|
8484
LL | unreachable!("{} {}", "panic with", "multiple arguments");
8585
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
8686

87-
error: aborting due to 12 previous errors
87+
error: `panic` should not be present in production code
88+
--> $DIR/panicking_macros.rs:40:5
89+
|
90+
LL | panic!();
91+
| ^^^^^^^^^
92+
93+
error: `todo` should not be present in production code
94+
--> $DIR/panicking_macros.rs:41:5
95+
|
96+
LL | todo!();
97+
| ^^^^^^^^
98+
99+
error: `unimplemented` should not be present in production code
100+
--> $DIR/panicking_macros.rs:42:5
101+
|
102+
LL | unimplemented!();
103+
| ^^^^^^^^^^^^^^^^^
104+
105+
error: `unreachable` should not be present in production code
106+
--> $DIR/panicking_macros.rs:43:5
107+
|
108+
LL | unreachable!();
109+
| ^^^^^^^^^^^^^^^
110+
111+
error: aborting due to 16 previous errors
88112

0 commit comments

Comments
 (0)