Auto merge of #8932 - dswij:pr-8879, r=giraffate

`needless_return` checks for macro expr in return stmts

closes #8879

Macro expressions in returns were not checked by `needless_return`. The test added in [this commit](https://github.com/rust-lang/rust-clippy/commit/6396a7a425293745b1810566851c17cf08d36985#diff-a869168cfafb7e6e5010feb76a16389d6c96d59e98113dee5c2b304a5160e43aR51-R55) seems to have regressed.

changelog: [`needless_return`] checks for macro exprs in return statements

5 files changed, 40 insertions, 14 deletions

diff --git a/clippy_lints/src/methods/option_map_or_none.rs b/clippy_lints/src/methods/option_map_or_none.rs
index 76bc9466ed8..8989db54f6c 100644
--- a/clippy_lints/src/methods/option_map_or_none.rs
+++ b/clippy_lints/src/methods/option_map_or_none.rs
@@ -97,7 +97,7 @@ pub(super) fn check<'tcx>(
         let func_snippet = snippet(cx, map_arg.span, "..");
         let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \
                        `and_then(..)` instead";
-        return span_lint_and_sugg(
+        span_lint_and_sugg(
             cx,
             OPTION_MAP_OR_NONE,
             expr.span,
@@ -110,7 +110,7 @@ pub(super) fn check<'tcx>(
         let msg = "called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling \
                        `ok()` instead";
         let self_snippet = snippet(cx, recv.span, "..");
-        return span_lint_and_sugg(
+        span_lint_and_sugg(
             cx,
             RESULT_MAP_OR_INTO_OPTION,
             expr.span,
diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index 8068fa22d9c..e525eba53e2 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -1,5 +1,5 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
-use clippy_utils::source::snippet_opt;
+use clippy_utils::source::{snippet_opt, snippet_with_context};
 use clippy_utils::{fn_def_id, path_to_local_id};
 use if_chain::if_chain;
 use rustc_ast::ast::Attribute;
@@ -226,14 +226,10 @@ fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, inner_span: Option<Spa
     }
     match inner_span {
         Some(inner_span) => {
-            if in_external_macro(cx.tcx.sess, inner_span) || inner_span.from_expansion() {
-                return;
-            }
-
+            let mut applicability = Applicability::MachineApplicable;
             span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded `return` statement", |diag| {
-                if let Some(snippet) = snippet_opt(cx, inner_span) {
-                    diag.span_suggestion(ret_span, "remove `return`", snippet, Applicability::MachineApplicable);
-                }
+                let (snippet, _) = snippet_with_context(cx, inner_span, ret_span.ctxt(), "..", &mut applicability);
+                diag.span_suggestion(ret_span, "remove `return`", snippet, applicability);
             });
         },
         None => match replacement {
@@ -287,7 +283,7 @@ struct BorrowVisitor<'a, 'tcx> {
 
 impl<'tcx> Visitor<'tcx> for BorrowVisitor<'_, 'tcx> {
     fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
-        if self.borrows {
+        if self.borrows || expr.span.from_expansion() {
             return;
         }
 
diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed
index 603d438d558..7c828430b78 100644
--- a/tests/ui/needless_return.fixed
+++ b/tests/ui/needless_return.fixed
@@ -53,7 +53,7 @@ fn test_closure() {
 }
 
 fn test_macro_call() -> i32 {
-    return the_answer!();
+    the_answer!()
 }
 
 fn test_void_fun() {
@@ -175,7 +175,7 @@ async fn async_test_closure() {
 }
 
 async fn async_test_macro_call() -> i32 {
-    return the_answer!();
+    the_answer!()
 }
 
 async fn async_test_void_fun() {
@@ -223,4 +223,10 @@ fn let_else() {
     let Some(1) = Some(1) else { return };
 }
 
+fn needless_return_macro() -> String {
+    let _ = "foo";
+    let _ = "bar";
+    format!("Hello {}", "world!")
+}
+
 fn main() {}
diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs
index c6c8cb9ec15..fe82af00e67 100644
--- a/tests/ui/needless_return.rs
+++ b/tests/ui/needless_return.rs
@@ -223,4 +223,10 @@ fn let_else() {
     let Some(1) = Some(1) else { return };
 }
 
+fn needless_return_macro() -> String {
+    let _ = "foo";
+    let _ = "bar";
+    return format!("Hello {}", "world!");
+}
+
 fn main() {}
diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr
index 5bc787c56a6..4c8be47b025 100644
--- a/tests/ui/needless_return.stderr
+++ b/tests/ui/needless_return.stderr
@@ -49,6 +49,12 @@ LL |     let _ = || return true;
    |                ^^^^^^^^^^^ help: remove `return`: `true`
 
 error: unneeded `return` statement
+  --> $DIR/needless_return.rs:56:5
+   |
+LL |     return the_answer!();
+   |     ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `the_answer!()`
+
+error: unneeded `return` statement
   --> $DIR/needless_return.rs:60:5
    |
 LL |     return;
@@ -169,6 +175,12 @@ LL |     let _ = || return true;
    |                ^^^^^^^^^^^ help: remove `return`: `true`
 
 error: unneeded `return` statement
+  --> $DIR/needless_return.rs:178:5
+   |
+LL |     return the_answer!();
+   |     ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `the_answer!()`
+
+error: unneeded `return` statement
   --> $DIR/needless_return.rs:182:5
    |
 LL |     return;
@@ -204,5 +216,11 @@ error: unneeded `return` statement
 LL |         return String::new();
    |         ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `String::new()`
 
-error: aborting due to 34 previous errors
+error: unneeded `return` statement
+  --> $DIR/needless_return.rs:229:5
+   |
+LL |     return format!("Hello {}", "world!");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `format!("Hello {}", "world!")`
+
+error: aborting due to 37 previous errors