Fix incorrect suggestion for try_err lint when Err arg is itself …

…a macro
flip1995 · Nov 5, 2020 · 83e75f9 · 83e75f9
1 parent f83762b
commit 83e75f9
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 8 deletions.
diff --git a/clippy_lints/src/try_err.rs b/clippy_lints/src/try_err.rs
@@ -1,6 +1,6 @@
 use crate::utils::{
-    in_macro, is_type_diagnostic_item, match_def_path, match_qpath, paths, snippet, snippet_with_macro_callsite,
-    span_lint_and_sugg,
+    differing_macro_contexts, 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;
 use rustc_errors::Applicability;
@@ -91,8 +91,11 @@ impl<'tcx> LateLintPass<'tcx> for TryErr {
                 };
 
                 let expr_err_ty = cx.typeck_results().expr_ty(err_arg);
+                let differing_contexts = differing_macro_contexts(expr.span, err_arg.span);
 
-                let origin_snippet = if err_arg.span.from_expansion() && !in_macro(expr.span) {
+                let origin_snippet = if in_macro(expr.span) && in_macro(err_arg.span) && differing_contexts {
+                    snippet(cx, err_arg.span.ctxt().outer_expn_data().call_site, "_")
+                } else if err_arg.span.from_expansion() && !in_macro(expr.span) {
                     snippet_with_macro_callsite(cx, err_arg.span, "_")
                 } else {
                     snippet(cx, err_arg.span, "_")

diff --git a/tests/ui/try_err.fixed b/tests/ui/try_err.fixed
@@ -88,8 +88,26 @@ macro_rules! try_validation {
     }};
 }
 
+macro_rules! ret_one {
+    () => {
+        1
+    };
+}
+
+macro_rules! try_validation_in_macro {
+    ($e: expr) => {{
+        match $e {
+            Ok(_) => 0,
+            Err(_) => return Err(ret_one!()),
+        }
+    }};
+}
+
 fn calling_macro() -> Result<i32, i32> {
+    // macro
     try_validation!(Ok::<_, i32>(5));
+    // `Err` arg is another macro
+    try_validation_in_macro!(Ok::<_, i32>(5));
     Ok(5)
 }
 

diff --git a/tests/ui/try_err.rs b/tests/ui/try_err.rs
@@ -88,8 +88,26 @@ macro_rules! try_validation {
     }};
 }
 
+macro_rules! ret_one {
+    () => {
+        1
+    };
+}
+
+macro_rules! try_validation_in_macro {
+    ($e: expr) => {{
+        match $e {
+            Ok(_) => 0,
+            Err(_) => Err(ret_one!())?,
+        }
+    }};
+}
+
 fn calling_macro() -> Result<i32, i32> {
+    // macro
     try_validation!(Ok::<_, i32>(5));
+    // `Err` arg is another macro
+    try_validation_in_macro!(Ok::<_, i32>(5));
     Ok(5)
 }
 

diff --git a/tests/ui/try_err.stderr b/tests/ui/try_err.stderr
@@ -40,28 +40,39 @@ LL |     try_validation!(Ok::<_, i32>(5));
    = 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
+  --> $DIR/try_err.rs:101:23
+   |
+LL |             Err(_) => Err(ret_one!())?,
+   |                       ^^^^^^^^^^^^^^^^ help: try this: `return Err(ret_one!())`
+...
+LL |     try_validation_in_macro!(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:140:9
    |
 LL |         Err(foo!())?;
    |         ^^^^^^^^^^^^ help: try this: `return Err(foo!())`
 
 error: returning an `Err(_)` with the `?` operator
-  --> $DIR/try_err.rs:129:9
+  --> $DIR/try_err.rs:147: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:131:9
+  --> $DIR/try_err.rs:149: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:139:9
+  --> $DIR/try_err.rs:157:9
    |
 LL |         Err(io::ErrorKind::NotFound)?
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `return Poll::Ready(Some(Err(io::ErrorKind::NotFound.into())))`
 
-error: aborting due to 9 previous errors
+error: aborting due to 10 previous errors