Skip to content

[panic_in_result_fn] remove todo!, unimplemented!, unreachable! #11123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions clippy_lints/src/panic_in_result_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_span::{sym, Span};

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `panic!`, `unimplemented!`, `todo!`, `unreachable!` or assertions in a function of type result.
/// Checks for usage of `panic!` or assertions in a function of type result.
///
/// ### Why is this bad?
/// For some codebases, it is desirable for functions of type result to return an error instead of crashing. Hence panicking macros should be avoided.
Expand All @@ -37,7 +37,7 @@ declare_clippy_lint! {
#[clippy::version = "1.48.0"]
pub PANIC_IN_RESULT_FN,
restriction,
"functions of type `Result<..>` that contain `panic!()`, `todo!()`, `unreachable()`, `unimplemented()` or assertion"
"functions of type `Result<..>` that contain `panic!()` or assertion"
}

declare_lint_pass!(PanicInResultFn => [PANIC_IN_RESULT_FN]);
Expand Down Expand Up @@ -70,7 +70,7 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir
};
if matches!(
cx.tcx.item_name(macro_call.def_id).as_str(),
"unimplemented" | "unreachable" | "panic" | "todo" | "assert" | "assert_eq" | "assert_ne"
"panic" | "assert" | "assert_eq" | "assert_ne"
) {
panics.push(macro_call.span);
ControlFlow::Continue(Descend::No)
Expand All @@ -83,10 +83,10 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir
cx,
PANIC_IN_RESULT_FN,
impl_span,
"used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`",
"used `panic!()` or assertion in a function that returns `Result`",
move |diag| {
diag.help(
"`unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing",
"`panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing",
);
diag.span_note(panics, "return Err() instead of panicking");
},
Expand Down
74 changes: 5 additions & 69 deletions tests/ui/panic_in_result_fn.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`
error: used `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn.rs:6:5
|
LL | / fn result_with_panic() -> Result<bool, String> // should emit lint
Expand All @@ -7,63 +7,15 @@ LL | | panic!("error");
LL | | }
| |_____^
|
= help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
= help: `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> $DIR/panic_in_result_fn.rs:8:9
|
LL | panic!("error");
| ^^^^^^^^^^^^^^^
= note: `-D clippy::panic-in-result-fn` implied by `-D warnings`

error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn.rs:11:5
|
LL | / fn result_with_unimplemented() -> Result<bool, String> // should emit lint
LL | | {
LL | | unimplemented!();
LL | | }
| |_____^
|
= help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> $DIR/panic_in_result_fn.rs:13:9
|
LL | unimplemented!();
| ^^^^^^^^^^^^^^^^

error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn.rs:16:5
|
LL | / fn result_with_unreachable() -> Result<bool, String> // should emit lint
LL | | {
LL | | unreachable!();
LL | | }
| |_____^
|
= help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> $DIR/panic_in_result_fn.rs:18:9
|
LL | unreachable!();
| ^^^^^^^^^^^^^^

error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn.rs:21:5
|
LL | / fn result_with_todo() -> Result<bool, String> // should emit lint
LL | | {
LL | | todo!("Finish this");
LL | | }
| |_____^
|
= help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> $DIR/panic_in_result_fn.rs:23:9
|
LL | todo!("Finish this");
| ^^^^^^^^^^^^^^^^^^^^

error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`
error: used `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn.rs:52:1
|
LL | / fn function_result_with_panic() -> Result<bool, String> // should emit lint
Expand All @@ -72,28 +24,12 @@ LL | | panic!("error");
LL | | }
| |_^
|
= help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
= help: `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> $DIR/panic_in_result_fn.rs:54:5
|
LL | panic!("error");
| ^^^^^^^^^^^^^^^

error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn.rs:67:1
|
LL | / fn main() -> Result<(), String> {
LL | | todo!("finish main method");
LL | | Ok(())
LL | | }
| |_^
|
= help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> $DIR/panic_in_result_fn.rs:68:5
|
LL | todo!("finish main method");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 6 previous errors
error: aborting due to 2 previous errors

12 changes: 6 additions & 6 deletions tests/ui/panic_in_result_fn_assertions.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`
error: used `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn_assertions.rs:7:5
|
LL | / fn result_with_assert_with_message(x: i32) -> Result<bool, String> // should emit lint
Expand All @@ -8,15 +8,15 @@ LL | | Ok(true)
LL | | }
| |_____^
|
= help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
= help: `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> $DIR/panic_in_result_fn_assertions.rs:9:9
|
LL | assert!(x == 5, "wrong argument");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: `-D clippy::panic-in-result-fn` implied by `-D warnings`

error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`
error: used `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn_assertions.rs:13:5
|
LL | / fn result_with_assert_eq(x: i32) -> Result<bool, String> // should emit lint
Expand All @@ -26,14 +26,14 @@ LL | | Ok(true)
LL | | }
| |_____^
|
= help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
= help: `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> $DIR/panic_in_result_fn_assertions.rs:15:9
|
LL | assert_eq!(x, 5);
| ^^^^^^^^^^^^^^^^

error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`
error: used `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn_assertions.rs:19:5
|
LL | / fn result_with_assert_ne(x: i32) -> Result<bool, String> // should emit lint
Expand All @@ -43,7 +43,7 @@ LL | | Ok(true)
LL | | }
| |_____^
|
= help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
= help: `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
note: return Err() instead of panicking
--> $DIR/panic_in_result_fn_assertions.rs:21:9
|
Expand Down