Skip to content

Commit 0ab75c3

Browse files
borsbors
committed
Auto merge of rust-lang#5977 - xvschneider:AddLintPanicInResult, r=matthiaskrgr
Add lint panic in result ### Change Adding a new "restriction" lint that will emit a warning when using "panic", "unimplemented" or "unreachable" in a function of type option/result. ### Motivation Some codebases must avoid crashes at all costs, and hence functions of type option/result must return an error instead of crashing. ### Test plan Running: TESTNAME=panic_in_result cargo uitest --- changelog: none
2 parents df6d7bf + f90b1fc commit 0ab75c3

File tree

6 files changed

+278
-0
lines changed

6 files changed

+278
-0
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1755,6 +1755,7 @@ Released 2018-09-13
17551755
[`out_of_bounds_indexing`]: https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
17561756
[`overflow_check_conditional`]: https://rust-lang.github.io/rust-clippy/master/index.html#overflow_check_conditional
17571757
[`panic`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic
1758+
[`panic_in_result_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_in_result_fn
17581759
[`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params
17591760
[`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
17601761
[`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl

‎clippy_lints/src/lib.rs

+5
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ mod open_options;
269269
mod option_env_unwrap;
270270
mod option_if_let_else;
271271
mod overflow_check_conditional;
272+
mod panic_in_result_fn;
272273
mod panic_unimplemented;
273274
mod partialeq_ne_impl;
274275
mod path_buf_push_overwrite;
@@ -751,6 +752,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
751752
&option_env_unwrap::OPTION_ENV_UNWRAP,
752753
&option_if_let_else::OPTION_IF_LET_ELSE,
753754
&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,
755+
&panic_in_result_fn::PANIC_IN_RESULT_FN,
754756
&panic_unimplemented::PANIC,
755757
&panic_unimplemented::PANIC_PARAMS,
756758
&panic_unimplemented::TODO,
@@ -1091,6 +1093,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10911093
store.register_late_pass(|| box manual_async_fn::ManualAsyncFn);
10921094
store.register_early_pass(|| box redundant_field_names::RedundantFieldNames);
10931095
store.register_late_pass(|| box vec_resize_to_zero::VecResizeToZero);
1096+
store.register_late_pass(|| box panic_in_result_fn::PanicInResultFn);
1097+
10941098
let single_char_binding_names_threshold = conf.single_char_binding_names_threshold;
10951099
store.register_early_pass(move || box non_expressive_names::NonExpressiveNames {
10961100
single_char_binding_names_threshold,
@@ -1135,6 +1139,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
11351139
LintId::of(&missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS),
11361140
LintId::of(&missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS),
11371141
LintId::of(&modulo_arithmetic::MODULO_ARITHMETIC),
1142+
LintId::of(&panic_in_result_fn::PANIC_IN_RESULT_FN),
11381143
LintId::of(&panic_unimplemented::PANIC),
11391144
LintId::of(&panic_unimplemented::TODO),
11401145
LintId::of(&panic_unimplemented::UNIMPLEMENTED),

‎clippy_lints/src/panic_in_result_fn.rs

+90
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
use crate::utils::{is_expn_of, is_type_diagnostic_item, return_ty, span_lint_and_then};
2+
use rustc_hir as hir;
3+
use rustc_hir::intravisit::{self, FnKind, NestedVisitorMap, Visitor};
4+
use rustc_hir::Expr;
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_middle::hir::map::Map;
7+
use rustc_session::{declare_lint_pass, declare_tool_lint};
8+
use rustc_span::Span;
9+
10+
declare_clippy_lint! {
11+
/// **What it does:** Checks for usage of `panic!`, `unimplemented!`, `todo!` or `unreachable!` in a function of type result.
12+
///
13+
/// **Why is this bad?** For some codebases, it is desirable for functions of type result to return an error instead of crashing. Hence unimplemented, panic and unreachable should be avoided.
14+
///
15+
/// **Known problems:** None.
16+
///
17+
/// **Example:**
18+
///
19+
/// ```rust
20+
/// fn result_with_panic() -> Result<bool, String>
21+
/// {
22+
/// panic!("error");
23+
/// }
24+
/// ```
25+
pub PANIC_IN_RESULT_FN,
26+
restriction,
27+
"functions of type `Result<..>` that contain `panic!()`, `todo!()` or `unreachable()` or `unimplemented()` "
28+
}
29+
30+
declare_lint_pass!(PanicInResultFn => [PANIC_IN_RESULT_FN]);
31+
32+
impl<'tcx> LateLintPass<'tcx> for PanicInResultFn {
33+
fn check_fn(
34+
&mut self,
35+
cx: &LateContext<'tcx>,
36+
fn_kind: FnKind<'tcx>,
37+
_: &'tcx hir::FnDecl<'tcx>,
38+
body: &'tcx hir::Body<'tcx>,
39+
span: Span,
40+
hir_id: hir::HirId,
41+
) {
42+
if !matches!(fn_kind, FnKind::Closure(_))
43+
&& is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(result_type))
44+
{
45+
lint_impl_body(cx, span, body);
46+
}
47+
}
48+
}
49+
50+
struct FindPanicUnimplementedUnreachable {
51+
result: Vec<Span>,
52+
}
53+
54+
impl<'tcx> Visitor<'tcx> for FindPanicUnimplementedUnreachable {
55+
type Map = Map<'tcx>;
56+
57+
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
58+
if ["unimplemented", "unreachable", "panic", "todo"]
59+
.iter()
60+
.any(|fun| is_expn_of(expr.span, fun).is_some())
61+
{
62+
self.result.push(expr.span);
63+
}
64+
// and check sub-expressions
65+
intravisit::walk_expr(self, expr);
66+
}
67+
68+
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
69+
NestedVisitorMap::None
70+
}
71+
}
72+
73+
fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir::Body<'tcx>) {
74+
let mut panics = FindPanicUnimplementedUnreachable { result: Vec::new() };
75+
panics.visit_expr(&body.value);
76+
if !panics.result.is_empty() {
77+
span_lint_and_then(
78+
cx,
79+
PANIC_IN_RESULT_FN,
80+
impl_span,
81+
"used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`",
82+
move |diag| {
83+
diag.help(
84+
"`unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing",
85+
);
86+
diag.span_note(panics.result, "return Err() instead of panicking");
87+
},
88+
);
89+
}
90+
}

‎src/lintlist/mod.rs

+7
Original file line numberDiff line numberDiff line change
@@ -1718,6 +1718,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
17181718
deprecation: None,
17191719
module: "panic_unimplemented",
17201720
},
1721+
Lint {
1722+
name: "panic_in_result_fn",
1723+
group: "restriction",
1724+
desc: "functions of type `Result<..>` that contain `panic!()`, `todo!()` or `unreachable()` or `unimplemented()` ",
1725+
deprecation: None,
1726+
module: "panic_in_result_fn",
1727+
},
17211728
Lint {
17221729
name: "panic_params",
17231730
group: "style",

‎tests/ui/panic_in_result_fn.rs

+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
#![warn(clippy::panic_in_result_fn)]
2+
3+
struct A;
4+
5+
impl A {
6+
fn result_with_panic() -> Result<bool, String> // should emit lint
7+
{
8+
panic!("error");
9+
}
10+
11+
fn result_with_unimplemented() -> Result<bool, String> // should emit lint
12+
{
13+
unimplemented!();
14+
}
15+
16+
fn result_with_unreachable() -> Result<bool, String> // should emit lint
17+
{
18+
unreachable!();
19+
}
20+
21+
fn result_with_todo() -> Result<bool, String> // should emit lint
22+
{
23+
todo!("Finish this");
24+
}
25+
26+
fn other_with_panic() // should not emit lint
27+
{
28+
panic!("");
29+
}
30+
31+
fn other_with_unreachable() // should not emit lint
32+
{
33+
unreachable!();
34+
}
35+
36+
fn other_with_unimplemented() // should not emit lint
37+
{
38+
unimplemented!();
39+
}
40+
41+
fn other_with_todo() // should not emit lint
42+
{
43+
todo!("finish this")
44+
}
45+
46+
fn result_without_banned_functions() -> Result<bool, String> // should not emit lint
47+
{
48+
Ok(true)
49+
}
50+
}
51+
52+
fn function_result_with_panic() -> Result<bool, String> // should emit lint
53+
{
54+
panic!("error");
55+
}
56+
57+
fn todo() {
58+
println!("something");
59+
}
60+
61+
fn function_result_with_custom_todo() -> Result<bool, String> // should not emit lint
62+
{
63+
todo();
64+
Ok(true)
65+
}
66+
67+
fn main() -> Result<(), String> {
68+
todo!("finish main method");
69+
Ok(())
70+
}

‎tests/ui/panic_in_result_fn.stderr

+105
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`
2+
--> $DIR/panic_in_result_fn.rs:6:5
3+
|
4+
LL | / fn result_with_panic() -> Result<bool, String> // should emit lint
5+
LL | | {
6+
LL | | panic!("error");
7+
LL | | }
8+
| |_____^
9+
|
10+
= note: `-D clippy::panic-in-result-fn` implied by `-D warnings`
11+
= help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
12+
note: return Err() instead of panicking
13+
--> $DIR/panic_in_result_fn.rs:8:9
14+
|
15+
LL | panic!("error");
16+
| ^^^^^^^^^^^^^^^^
17+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
18+
19+
error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`
20+
--> $DIR/panic_in_result_fn.rs:11:5
21+
|
22+
LL | / fn result_with_unimplemented() -> Result<bool, String> // should emit lint
23+
LL | | {
24+
LL | | unimplemented!();
25+
LL | | }
26+
| |_____^
27+
|
28+
= help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
29+
note: return Err() instead of panicking
30+
--> $DIR/panic_in_result_fn.rs:13:9
31+
|
32+
LL | unimplemented!();
33+
| ^^^^^^^^^^^^^^^^^
34+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
35+
36+
error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`
37+
--> $DIR/panic_in_result_fn.rs:16:5
38+
|
39+
LL | / fn result_with_unreachable() -> Result<bool, String> // should emit lint
40+
LL | | {
41+
LL | | unreachable!();
42+
LL | | }
43+
| |_____^
44+
|
45+
= help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
46+
note: return Err() instead of panicking
47+
--> $DIR/panic_in_result_fn.rs:18:9
48+
|
49+
LL | unreachable!();
50+
| ^^^^^^^^^^^^^^^
51+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
52+
53+
error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`
54+
--> $DIR/panic_in_result_fn.rs:21:5
55+
|
56+
LL | / fn result_with_todo() -> Result<bool, String> // should emit lint
57+
LL | | {
58+
LL | | todo!("Finish this");
59+
LL | | }
60+
| |_____^
61+
|
62+
= help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
63+
note: return Err() instead of panicking
64+
--> $DIR/panic_in_result_fn.rs:23:9
65+
|
66+
LL | todo!("Finish this");
67+
| ^^^^^^^^^^^^^^^^^^^^^
68+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
69+
70+
error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`
71+
--> $DIR/panic_in_result_fn.rs:52:1
72+
|
73+
LL | / fn function_result_with_panic() -> Result<bool, String> // should emit lint
74+
LL | | {
75+
LL | | panic!("error");
76+
LL | | }
77+
| |_^
78+
|
79+
= help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
80+
note: return Err() instead of panicking
81+
--> $DIR/panic_in_result_fn.rs:54:5
82+
|
83+
LL | panic!("error");
84+
| ^^^^^^^^^^^^^^^^
85+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
86+
87+
error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`
88+
--> $DIR/panic_in_result_fn.rs:67:1
89+
|
90+
LL | / fn main() -> Result<(), String> {
91+
LL | | todo!("finish main method");
92+
LL | | Ok(())
93+
LL | | }
94+
| |_^
95+
|
96+
= help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
97+
note: return Err() instead of panicking
98+
--> $DIR/panic_in_result_fn.rs:68:5
99+
|
100+
LL | todo!("finish main method");
101+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
102+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
103+
104+
error: aborting due to 6 previous errors
105+

0 commit comments

Comments
 (0)