about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-04-10 19:15:37 +0000
committerbors <bors@rust-lang.org>2021-04-10 19:15:37 +0000
commitfd5cf4e82d3422887585fd148369b592f95cd169 (patch)
treedacd684c99431f4c1fd03961a4ffdb98c5df0a62
parentf7c2c44e76c626afcd594ecc193ed446729152db (diff)
parent271c163ba319ce8518913057cef32978f38b2f6a (diff)
downloadrust-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.rs8
-rw-r--r--tests/ui/panic_in_result_fn_debug_assertions.rs23
-rw-r--r--tests/ui/panic_in_result_fn_debug_assertions.stderr57
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
-