diff options
| author | bors <bors@rust-lang.org> | 2021-04-10 19:15:37 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2021-04-10 19:15:37 +0000 |
| commit | fd5cf4e82d3422887585fd148369b592f95cd169 (patch) | |
| tree | dacd684c99431f4c1fd03961a4ffdb98c5df0a62 | |
| parent | f7c2c44e76c626afcd594ecc193ed446729152db (diff) | |
| parent | 271c163ba319ce8518913057cef32978f38b2f6a (diff) | |
| download | rust-fd5cf4e82d3422887585fd148369b592f95cd169.tar.gz rust-fd5cf4e82d3422887585fd148369b592f95cd169.zip | |
Auto merge of #7060 - daxpedda:debug-assert-panic-in-result-fn, r=flip1995
Remove `debug_assert` from `panic_in_result_fn` I couldn't find any documentation on `debug_assert` that should be remove. In my humble opinion, I would also like to argue that `todo` and `unreachable` shouldn't trigger this lint? Related: https://github.com/rust-lang/rust-clippy/issues/6082 r? `@flip1995` changelog: Change `panic_in_result_fn` to ignore `debug_assert` and co macros
| -rw-r--r-- | clippy_lints/src/panic_in_result_fn.rs | 8 | ||||
| -rw-r--r-- | tests/ui/panic_in_result_fn_debug_assertions.rs | 23 | ||||
| -rw-r--r-- | tests/ui/panic_in_result_fn_debug_assertions.stderr | 57 |
3 files changed, 12 insertions, 76 deletions
diff --git a/clippy_lints/src/panic_in_result_fn.rs b/clippy_lints/src/panic_in_result_fn.rs index d32b937b209..cef74d87e7c 100644 --- a/clippy_lints/src/panic_in_result_fn.rs +++ b/clippy_lints/src/panic_in_result_fn.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{find_macro_calls, return_ty}; +use clippy_utils::{find_macro_calls, is_expn_of, return_ty}; use rustc_hir as hir; use rustc_hir::intravisit::FnKind; use rustc_lint::{LateContext, LateLintPass}; @@ -52,7 +52,7 @@ impl<'tcx> LateLintPass<'tcx> for PanicInResultFn { } fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir::Body<'tcx>) { - let panics = find_macro_calls( + let mut panics = find_macro_calls( &[ "unimplemented", "unreachable", @@ -61,12 +61,10 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir "assert", "assert_eq", "assert_ne", - "debug_assert", - "debug_assert_eq", - "debug_assert_ne", ], body, ); + panics.retain(|span| is_expn_of(*span, "debug_assert").is_none()); if !panics.is_empty() { span_lint_and_then( cx, diff --git a/tests/ui/panic_in_result_fn_debug_assertions.rs b/tests/ui/panic_in_result_fn_debug_assertions.rs index b60c79f97c8..c4fcd7e7094 100644 --- a/tests/ui/panic_in_result_fn_debug_assertions.rs +++ b/tests/ui/panic_in_result_fn_debug_assertions.rs @@ -1,44 +1,39 @@ #![warn(clippy::panic_in_result_fn)] #![allow(clippy::unnecessary_wraps)] +// debug_assert should never trigger the `panic_in_result_fn` lint + struct A; impl A { - fn result_with_debug_assert_with_message(x: i32) -> Result<bool, String> // should emit lint - { + fn result_with_debug_assert_with_message(x: i32) -> Result<bool, String> { debug_assert!(x == 5, "wrong argument"); Ok(true) } - fn result_with_debug_assert_eq(x: i32) -> Result<bool, String> // should emit lint - { + fn result_with_debug_assert_eq(x: i32) -> Result<bool, String> { debug_assert_eq!(x, 5); Ok(true) } - fn result_with_debug_assert_ne(x: i32) -> Result<bool, String> // should emit lint - { + fn result_with_debug_assert_ne(x: i32) -> Result<bool, String> { debug_assert_ne!(x, 1); Ok(true) } - fn other_with_debug_assert_with_message(x: i32) // should not emit lint - { + fn other_with_debug_assert_with_message(x: i32) { debug_assert!(x == 5, "wrong argument"); } - fn other_with_debug_assert_eq(x: i32) // should not emit lint - { + fn other_with_debug_assert_eq(x: i32) { debug_assert_eq!(x, 5); } - fn other_with_debug_assert_ne(x: i32) // should not emit lint - { + fn other_with_debug_assert_ne(x: i32) { debug_assert_ne!(x, 1); } - fn result_without_banned_functions() -> Result<bool, String> // should not emit lint - { + fn result_without_banned_functions() -> Result<bool, String> { let debug_assert = "debug_assert!"; println!("No {}", debug_assert); Ok(true) diff --git a/tests/ui/panic_in_result_fn_debug_assertions.stderr b/tests/ui/panic_in_result_fn_debug_assertions.stderr deleted file mode 100644 index ec18e89698c..00000000000 --- a/tests/ui/panic_in_result_fn_debug_assertions.stderr +++ /dev/null @@ -1,57 +0,0 @@ -error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` - --> $DIR/panic_in_result_fn_debug_assertions.rs:7:5 - | -LL | / fn result_with_debug_assert_with_message(x: i32) -> Result<bool, String> // should emit lint -LL | | { -LL | | debug_assert!(x == 5, "wrong argument"); -LL | | Ok(true) -LL | | } - | |_____^ - | - = note: `-D clippy::panic-in-result-fn` implied by `-D warnings` - = 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_debug_assertions.rs:9:9 - | -LL | debug_assert!(x == 5, "wrong argument"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) - -error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` - --> $DIR/panic_in_result_fn_debug_assertions.rs:13:5 - | -LL | / fn result_with_debug_assert_eq(x: i32) -> Result<bool, String> // should emit lint -LL | | { -LL | | debug_assert_eq!(x, 5); -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 -note: return Err() instead of panicking - --> $DIR/panic_in_result_fn_debug_assertions.rs:15:9 - | -LL | debug_assert_eq!(x, 5); - | ^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) - -error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` - --> $DIR/panic_in_result_fn_debug_assertions.rs:19:5 - | -LL | / fn result_with_debug_assert_ne(x: i32) -> Result<bool, String> // should emit lint -LL | | { -LL | | debug_assert_ne!(x, 1); -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 -note: return Err() instead of panicking - --> $DIR/panic_in_result_fn_debug_assertions.rs:21:9 - | -LL | debug_assert_ne!(x, 1); - | ^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) - -error: aborting due to 3 previous errors - |
