From 43e6c656ba0f221caace290b8a8deace2abd89cd Mon Sep 17 00:00:00 2001 From: daxpedda Date: Sat, 10 Apr 2021 17:38:04 +0200 Subject: [PATCH 1/3] Remove `debug_assert` from `panic_in_result_fn` --- clippy_lints/src/panic_in_result_fn.rs | 3 - .../ui/panic_in_result_fn_debug_assertions.rs | 16 +++--- ...panic_in_result_fn_debug_assertions.stderr | 57 ------------------- 3 files changed, 9 insertions(+), 67 deletions(-) delete mode 100644 tests/ui/panic_in_result_fn_debug_assertions.stderr diff --git a/clippy_lints/src/panic_in_result_fn.rs b/clippy_lints/src/panic_in_result_fn.rs index d32b937b209c..791a2bf07984 100644 --- a/clippy_lints/src/panic_in_result_fn.rs +++ b/clippy_lints/src/panic_in_result_fn.rs @@ -61,9 +61,6 @@ 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, ); diff --git a/tests/ui/panic_in_result_fn_debug_assertions.rs b/tests/ui/panic_in_result_fn_debug_assertions.rs index b60c79f97c86..1f8485b8cd84 100644 --- a/tests/ui/panic_in_result_fn_debug_assertions.rs +++ b/tests/ui/panic_in_result_fn_debug_assertions.rs @@ -1,43 +1,45 @@ #![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 // should emit lint + fn result_with_debug_assert_with_message(x: i32) -> Result { debug_assert!(x == 5, "wrong argument"); Ok(true) } - fn result_with_debug_assert_eq(x: i32) -> Result // should emit lint + fn result_with_debug_assert_eq(x: i32) -> Result { debug_assert_eq!(x, 5); Ok(true) } - fn result_with_debug_assert_ne(x: i32) -> Result // should emit lint + fn result_with_debug_assert_ne(x: i32) -> Result { 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 // should not emit lint + fn result_without_banned_functions() -> Result { let debug_assert = "debug_assert!"; println!("No {}", debug_assert); 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 ec18e89698c5..000000000000 --- 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 // 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 // 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 // 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 - From 8f4417faf24e18359a344945ff1072b54f354f77 Mon Sep 17 00:00:00 2001 From: daxpedda Date: Sat, 10 Apr 2021 17:45:55 +0200 Subject: [PATCH 2/3] Fix rustfmt --- .../ui/panic_in_result_fn_debug_assertions.rs | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/tests/ui/panic_in_result_fn_debug_assertions.rs b/tests/ui/panic_in_result_fn_debug_assertions.rs index 1f8485b8cd84..c4fcd7e70944 100644 --- a/tests/ui/panic_in_result_fn_debug_assertions.rs +++ b/tests/ui/panic_in_result_fn_debug_assertions.rs @@ -6,41 +6,34 @@ struct A; impl A { - fn result_with_debug_assert_with_message(x: i32) -> Result - { + fn result_with_debug_assert_with_message(x: i32) -> Result { debug_assert!(x == 5, "wrong argument"); Ok(true) } - fn result_with_debug_assert_eq(x: i32) -> Result - { + fn result_with_debug_assert_eq(x: i32) -> Result { debug_assert_eq!(x, 5); Ok(true) } - fn result_with_debug_assert_ne(x: i32) -> Result - { + fn result_with_debug_assert_ne(x: i32) -> Result { debug_assert_ne!(x, 1); Ok(true) } - fn other_with_debug_assert_with_message(x: i32) - { + fn other_with_debug_assert_with_message(x: i32) { debug_assert!(x == 5, "wrong argument"); } - fn other_with_debug_assert_eq(x: i32) - { + fn other_with_debug_assert_eq(x: i32) { debug_assert_eq!(x, 5); } - fn other_with_debug_assert_ne(x: i32) - { + fn other_with_debug_assert_ne(x: i32) { debug_assert_ne!(x, 1); } - fn result_without_banned_functions() -> Result - { + fn result_without_banned_functions() -> Result { let debug_assert = "debug_assert!"; println!("No {}", debug_assert); Ok(true) From 271c163ba319ce8518913057cef32978f38b2f6a Mon Sep 17 00:00:00 2001 From: daxpedda Date: Sat, 10 Apr 2021 20:54:40 +0200 Subject: [PATCH 3/3] Fix false-positive `debug_assert` --- clippy_lints/src/panic_in_result_fn.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/panic_in_result_fn.rs b/clippy_lints/src/panic_in_result_fn.rs index 791a2bf07984..cef74d87e7c0 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", @@ -64,6 +64,7 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir ], body, ); + panics.retain(|span| is_expn_of(*span, "debug_assert").is_none()); if !panics.is_empty() { span_lint_and_then( cx,