Fix FP with empty return for `needless_return` lint

4 files changed, 55 insertions, 2 deletions

diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index 63548d8fdb4..e438f92b136 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -131,7 +131,16 @@ impl<'tcx> LateLintPass<'tcx> for Return {
         _: HirId,
     ) {
         match kind {
-            FnKind::Closure(_) => check_final_expr(cx, &body.value, Some(body.value.span), RetReplacement::Empty),
+            FnKind::Closure(_) => {
+                // when returning without value in closure, replace this `return`
+                // with an empty block to prevent invalid suggestion (see #6501)
+                let replacement = if let ExprKind::Ret(None) = &body.value.kind {
+                    RetReplacement::Block
+                } else {
+                    RetReplacement::Empty
+                };
+                check_final_expr(cx, &body.value, Some(body.value.span), replacement)
+            },
             FnKind::ItemFn(..) | FnKind::Method(..) => {
                 if let ExprKind::Block(ref block, _) = body.value.kind {
                     check_block_return(cx, block);
diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed
index 86bfc5b4bb2..f137e8ecae9 100644
--- a/tests/ui/needless_return.fixed
+++ b/tests/ui/needless_return.fixed
@@ -101,6 +101,19 @@ fn test_return_in_macro() {
     needed_return!(0);
 }
 
+mod issue6501 {
+    fn foo(bar: Result<(), ()>) {
+        bar.unwrap_or_else(|_| {})
+    }
+
+    fn test_closure() {
+        let _ = || {
+            
+        };
+        let _ = || {};
+    }
+}
+
 fn main() {
     let _ = test_end_of_fn();
     let _ = test_no_semicolon();
diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs
index 51061370dfe..d754e4d37a8 100644
--- a/tests/ui/needless_return.rs
+++ b/tests/ui/needless_return.rs
@@ -101,6 +101,19 @@ fn test_return_in_macro() {
     needed_return!(0);
 }
 
+mod issue6501 {
+    fn foo(bar: Result<(), ()>) {
+        bar.unwrap_or_else(|_| return)
+    }
+
+    fn test_closure() {
+        let _ = || {
+            return;
+        };
+        let _ = || return;
+    }
+}
+
 fn main() {
     let _ = test_end_of_fn();
     let _ = test_no_semicolon();
diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr
index f73c833a801..12d94e892ed 100644
--- a/tests/ui/needless_return.stderr
+++ b/tests/ui/needless_return.stderr
@@ -84,5 +84,23 @@ error: unneeded `return` statement
 LL |         return String::new();
    |         ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()`
 
-error: aborting due to 14 previous errors
+error: unneeded `return` statement
+  --> $DIR/needless_return.rs:91:32
+   |
+LL |         bar.unwrap_or_else(|_| return)
+   |                                ^^^^^^ help: replace `return` with an empty block: `{}`
+
+error: unneeded `return` statement
+  --> $DIR/needless_return.rs:96:13
+   |
+LL |             return;
+   |             ^^^^^^^ help: remove `return`
+
+error: unneeded `return` statement
+  --> $DIR/needless_return.rs:98:20
+   |
+LL |         let _ = || return;
+   |                    ^^^^^^ help: replace `return` with an empty block: `{}`
+
+error: aborting due to 17 previous errors