Auto merge of #9967 - koka831:fix/9416, r=llogiq

Remove blank lines when needless_return returns no value

fix https://github.com/rust-lang/rust-clippy/issues/9416

changelog: [`needless_return`] improve result format

r? `@llogiq`

4 files changed, 106 insertions, 40 deletions

diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index 2b2a41d1601..81143d7799e 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -12,6 +12,7 @@ use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::subst::GenericArgKind;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
+use rustc_span::{BytePos, Pos};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -209,13 +210,14 @@ fn check_final_expr<'tcx>(
             if cx.tcx.hir().attrs(expr.hir_id).is_empty() {
                 let borrows = inner.map_or(false, |inner| last_statement_borrows(cx, inner));
                 if !borrows {
-                    emit_return_lint(
-                        cx,
-                        peeled_drop_expr.span,
-                        semi_spans,
-                        inner.as_ref().map(|i| i.span),
-                        replacement,
-                    );
+                    // check if expr return nothing
+                    let ret_span = if inner.is_none() && replacement == RetReplacement::Empty {
+                        extend_span_to_previous_non_ws(cx, peeled_drop_expr.span)
+                    } else {
+                        peeled_drop_expr.span
+                    };
+
+                    emit_return_lint(cx, ret_span, semi_spans, inner.as_ref().map(|i| i.span), replacement);
                 }
             }
         },
@@ -289,3 +291,16 @@ fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>)
     })
     .is_some()
 }
+
+// Go backwards while encountering whitespace and extend the given Span to that point.
+fn extend_span_to_previous_non_ws(cx: &LateContext<'_>, sp: Span) -> Span {
+    if let Ok(prev_source) = cx.sess().source_map().span_to_prev_source(sp) {
+        let ws = [' ', '\t', '\n'];
+        if let Some(non_ws_pos) = prev_source.rfind(|c| !ws.contains(&c)) {
+            let len = prev_source.len() - non_ws_pos - 1;
+            return sp.with_lo(sp.lo() - BytePos::from_usize(len));
+        }
+    }
+
+    sp
+}
diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed
index d2163b14fca..4386aaec49e 100644
--- a/tests/ui/needless_return.fixed
+++ b/tests/ui/needless_return.fixed
@@ -59,14 +59,11 @@ fn test_macro_call() -> i32 {
 }
 
 fn test_void_fun() {
-    
 }
 
 fn test_void_if_fun(b: bool) {
     if b {
-        
     } else {
-        
     }
 }
 
@@ -82,7 +79,6 @@ fn test_nested_match(x: u32) {
         0 => (),
         1 => {
             let _ = 42;
-            
         },
         _ => (),
     }
@@ -126,7 +122,6 @@ mod issue6501 {
 
     fn test_closure() {
         let _ = || {
-            
         };
         let _ = || {};
     }
@@ -179,14 +174,11 @@ async fn async_test_macro_call() -> i32 {
 }
 
 async fn async_test_void_fun() {
-    
 }
 
 async fn async_test_void_if_fun(b: bool) {
     if b {
-        
     } else {
-        
     }
 }
 
@@ -269,4 +261,15 @@ fn issue9503(x: usize) -> isize {
     }
 }
 
+mod issue9416 {
+    pub fn with_newline() {
+        let _ = 42;
+    }
+
+    #[rustfmt::skip]
+    pub fn oneline() {
+        let _ = 42;
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs
index 114414b5fac..666dc54b76b 100644
--- a/tests/ui/needless_return.rs
+++ b/tests/ui/needless_return.rs
@@ -269,4 +269,17 @@ fn issue9503(x: usize) -> isize {
     };
 }
 
+mod issue9416 {
+    pub fn with_newline() {
+        let _ = 42;
+
+        return;
+    }
+
+    #[rustfmt::skip]
+    pub fn oneline() {
+        let _ = 42; return;
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr
index 047fb6c2311..a8b5d86cd55 100644
--- a/tests/ui/needless_return.stderr
+++ b/tests/ui/needless_return.stderr
@@ -72,26 +72,32 @@ LL |     return the_answer!();
    = help: remove `return`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:62:5
+  --> $DIR/needless_return.rs:61:21
    |
-LL |     return;
-   |     ^^^^^^
+LL |   fn test_void_fun() {
+   |  _____________________^
+LL | |     return;
+   | |__________^
    |
    = help: remove `return`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:67:9
+  --> $DIR/needless_return.rs:66:11
    |
-LL |         return;
-   |         ^^^^^^
+LL |       if b {
+   |  ___________^
+LL | |         return;
+   | |______________^
    |
    = help: remove `return`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:69:9
+  --> $DIR/needless_return.rs:68:13
    |
-LL |         return;
-   |         ^^^^^^
+LL |       } else {
+   |  _____________^
+LL | |         return;
+   | |______________^
    |
    = help: remove `return`
 
@@ -104,10 +110,12 @@ LL |         _ => return,
    = help: replace `return` with a unit value
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:85:13
+  --> $DIR/needless_return.rs:84:24
    |
-LL |             return;
-   |             ^^^^^^
+LL |               let _ = 42;
+   |  ________________________^
+LL | |             return;
+   | |__________________^
    |
    = help: remove `return`
 
@@ -144,10 +152,12 @@ LL |         bar.unwrap_or_else(|_| return)
    = help: replace `return` with an empty block
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:129:13
+  --> $DIR/needless_return.rs:128:21
    |
-LL |             return;
-   |             ^^^^^^
+LL |           let _ = || {
+   |  _____________________^
+LL | |             return;
+   | |__________________^
    |
    = help: remove `return`
 
@@ -240,26 +250,32 @@ LL |     return the_answer!();
    = help: remove `return`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:182:5
+  --> $DIR/needless_return.rs:181:33
    |
-LL |     return;
-   |     ^^^^^^
+LL |   async fn async_test_void_fun() {
+   |  _________________________________^
+LL | |     return;
+   | |__________^
    |
    = help: remove `return`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:187:9
+  --> $DIR/needless_return.rs:186:11
    |
-LL |         return;
-   |         ^^^^^^
+LL |       if b {
+   |  ___________^
+LL | |         return;
+   | |______________^
    |
    = help: remove `return`
 
 error: unneeded `return` statement
-  --> $DIR/needless_return.rs:189:9
+  --> $DIR/needless_return.rs:188:13
    |
-LL |         return;
-   |         ^^^^^^
+LL |       } else {
+   |  _____________^
+LL | |         return;
+   | |______________^
    |
    = help: remove `return`
 
@@ -351,5 +367,24 @@ LL |             return !*(x as *const isize);
    |
    = help: remove `return`
 
-error: aborting due to 44 previous errors
+error: unneeded `return` statement
+  --> $DIR/needless_return.rs:274:20
+   |
+LL |           let _ = 42;
+   |  ____________________^
+LL | |
+LL | |         return;
+   | |______________^
+   |
+   = help: remove `return`
+
+error: unneeded `return` statement
+  --> $DIR/needless_return.rs:281:20
+   |
+LL |         let _ = 42; return;
+   |                    ^^^^^^^
+   |
+   = help: remove `return`
+
+error: aborting due to 46 previous errors