Fix incorrect suggestion when from expansion in `try_err` lint

4 files changed, 56 insertions, 7 deletions

diff --git a/clippy_lints/src/try_err.rs b/clippy_lints/src/try_err.rs
index 3e747ec4ad9..e6d8e7521a8 100644
--- a/clippy_lints/src/try_err.rs
+++ b/clippy_lints/src/try_err.rs
@@ -1,5 +1,5 @@
 use crate::utils::{
-    is_type_diagnostic_item, match_def_path, match_qpath, paths, snippet, snippet_with_macro_callsite,
+    in_macro, is_type_diagnostic_item, match_def_path, match_qpath, paths, snippet, snippet_with_macro_callsite,
     span_lint_and_sugg,
 };
 use if_chain::if_chain;
@@ -92,9 +92,13 @@ impl<'tcx> LateLintPass<'tcx> for TryErr {
 
                 let expr_err_ty = cx.typeck_results().expr_ty(err_arg);
 
-                let origin_snippet = if err_arg.span.from_expansion() {
+                // println!("\n\n{:?}", in_macro(expr.span));
+                // println!("{:#?}", snippet(cx, err_arg.span, "_"));
+                let origin_snippet = if err_arg.span.from_expansion() && !in_macro(expr.span) {
+                    // println!("from expansion");
                     snippet_with_macro_callsite(cx, err_arg.span, "_")
                 } else {
+                    // println!("just a snippet");
                     snippet(cx, err_arg.span, "_")
                 };
                 let suggestion = if err_ty == expr_err_ty {
@@ -102,6 +106,8 @@ impl<'tcx> LateLintPass<'tcx> for TryErr {
                 } else {
                     format!("return {}{}.into(){}", prefix, origin_snippet, suffix)
                 };
+                // println!("origin_snippet: {:#?}", origin_snippet);
+                // println!("suggestion: {:#?}", suggestion);
 
                 span_lint_and_sugg(
                     cx,
diff --git a/tests/ui/try_err.fixed b/tests/ui/try_err.fixed
index 9e77dcd8731..053dd45f23e 100644
--- a/tests/ui/try_err.fixed
+++ b/tests/ui/try_err.fixed
@@ -78,12 +78,28 @@ fn nested_error() -> Result<i32, i32> {
     Ok(1)
 }
 
+// Bad suggestion when in macro (see #6242)
+macro_rules! try_validation {
+    ($e: expr) => {{
+        match $e {
+            Ok(_) => 0,
+            Err(_) => return Err(1),
+        }
+    }};
+}
+
+fn calling_macro() -> Result<i32, i32> {
+    try_validation!(Ok::<_, i32>(5));
+    Ok(5)
+}
+
 fn main() {
     basic_test().unwrap();
     into_test().unwrap();
     negative_test().unwrap();
     closure_matches_test().unwrap();
     closure_into_test().unwrap();
+    calling_macro().unwrap();
 
     // We don't want to lint in external macros
     try_err!();
diff --git a/tests/ui/try_err.rs b/tests/ui/try_err.rs
index 41bcb0a189e..215ca6a07e6 100644
--- a/tests/ui/try_err.rs
+++ b/tests/ui/try_err.rs
@@ -78,12 +78,28 @@ fn nested_error() -> Result<i32, i32> {
     Ok(1)
 }
 
+// Bad suggestion when in macro (see #6242)
+macro_rules! try_validation {
+    ($e: expr) => {{
+        match $e {
+            Ok(_) => 0,
+            Err(_) => Err(1)?,
+        }
+    }};
+}
+
+fn calling_macro() -> Result<i32, i32> {
+    try_validation!(Ok::<_, i32>(5));
+    Ok(5)
+}
+
 fn main() {
     basic_test().unwrap();
     into_test().unwrap();
     negative_test().unwrap();
     closure_matches_test().unwrap();
     closure_into_test().unwrap();
+    calling_macro().unwrap();
 
     // We don't want to lint in external macros
     try_err!();
diff --git a/tests/ui/try_err.stderr b/tests/ui/try_err.stderr
index 3f1cbc17e72..443c8d08472 100644
--- a/tests/ui/try_err.stderr
+++ b/tests/ui/try_err.stderr
@@ -29,28 +29,39 @@ LL |                 Err(err)?;
    |                 ^^^^^^^^^ help: try this: `return Err(err.into())`
 
 error: returning an `Err(_)` with the `?` operator
-  --> $DIR/try_err.rs:106:9
+  --> $DIR/try_err.rs:86:23
+   |
+LL |             Err(_) => Err(1)?,
+   |                       ^^^^^^^ help: try this: `return Err(1)`
+...
+LL |     try_validation!(Ok::<_, i32>(5));
+   |     --------------------------------- in this macro invocation
+   |
+   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: returning an `Err(_)` with the `?` operator
+  --> $DIR/try_err.rs:122:9
    |
 LL |         Err(foo!())?;
    |         ^^^^^^^^^^^^ help: try this: `return Err(foo!())`
 
 error: returning an `Err(_)` with the `?` operator
-  --> $DIR/try_err.rs:113:9
+  --> $DIR/try_err.rs:129:9
    |
 LL |         Err(io::ErrorKind::WriteZero)?
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `return Poll::Ready(Err(io::ErrorKind::WriteZero.into()))`
 
 error: returning an `Err(_)` with the `?` operator
-  --> $DIR/try_err.rs:115:9
+  --> $DIR/try_err.rs:131:9
    |
 LL |         Err(io::Error::new(io::ErrorKind::InvalidInput, "error"))?
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `return Poll::Ready(Err(io::Error::new(io::ErrorKind::InvalidInput, "error")))`
 
 error: returning an `Err(_)` with the `?` operator
-  --> $DIR/try_err.rs:123:9
+  --> $DIR/try_err.rs:139:9
    |
 LL |         Err(io::ErrorKind::NotFound)?
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `return Poll::Ready(Some(Err(io::ErrorKind::NotFound.into())))`
 
-error: aborting due to 8 previous errors
+error: aborting due to 9 previous errors