Skip to content

Commit

Permalink
[panic_in_result_fn] remove todo!, unimplemented!, unreachable!
Browse files Browse the repository at this point in the history 
This commit fixes #11025 by removing checks for `todo!`,
`unimplemented!` and `unreachable!`.

Signed-off-by: Panagiotis Foliadis <pfoliadis@hotmail.com>
  • Loading branch information
@panosfol
panosfol committed Jul 12, 2023
1 parent 26edd5a commit c49c177
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 80 deletions.
10 changes: 5 additions & 5 deletions clippy_lints/src/panic_in_result_fn.rs
View file
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
View file
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
View file
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

0 comments on commit c49c177

Please sign in to comment.