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 rust-lang#11025 by removing checks for `todo!`,
`unimplemented!` and `unreachable!`.

Signed-off-by: Panagiotis Foliadis <[email protected]>
  • Loading branch information
panosfol committed Jul 7, 2023
1 parent 26edd5a commit 07dd5ae
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 127 deletions.
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
45 changes: 0 additions & 45 deletions tests/ui/panic_in_result_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,41 +8,11 @@ impl A {
panic!("error");
}

fn result_with_unimplemented() -> Result<bool, String> // should emit lint
{
unimplemented!();
}

fn result_with_unreachable() -> Result<bool, String> // should emit lint
{
unreachable!();
}

fn result_with_todo() -> Result<bool, String> // should emit lint
{
todo!("Finish this");
}

fn other_with_panic() // should not emit lint
{
panic!("");
}

fn other_with_unreachable() // should not emit lint
{
unreachable!();
}

fn other_with_unimplemented() // should not emit lint
{
unimplemented!();
}

fn other_with_todo() // should not emit lint
{
todo!("finish this")
}

fn result_without_banned_functions() -> Result<bool, String> // should not emit lint
{
Ok(true)
Expand All @@ -53,18 +23,3 @@ fn function_result_with_panic() -> Result<bool, String> // should emit lint
{
panic!("error");
}

fn todo() {
println!("something");
}

fn function_result_with_custom_todo() -> Result<bool, String> // should not emit lint
{
todo();
Ok(true)
}

fn main() -> Result<(), String> {
todo!("finish main method");
Ok(())
}
78 changes: 7 additions & 71 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,93 +7,29 @@ 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`
--> $DIR/panic_in_result_fn.rs:52:1
error: used `panic!()` or assertion in a function that returns `Result`
--> $DIR/panic_in_result_fn.rs:22:1
|
LL | / fn function_result_with_panic() -> Result<bool, String> // should emit lint
LL | | {
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
--> $DIR/panic_in_result_fn.rs:24: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

0 comments on commit 07dd5ae

Please sign in to comment.