diff --git a/clippy_lints/src/try_err.rs b/clippy_lints/src/try_err.rs index 7466221fb..3734b6098 100644 --- a/clippy_lints/src/try_err.rs +++ b/clippy_lints/src/try_err.rs @@ -1,10 +1,11 @@ -use crate::utils::{match_qpath, paths, snippet, span_lint_and_sugg}; +use crate::utils::{in_macro_or_desugar, match_qpath, paths, snippet, span_lint_and_sugg}; use if_chain::if_chain; use rustc::hir::*; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::ty::Ty; use rustc::{declare_lint_pass, declare_tool_lint}; use rustc_errors::Applicability; +use syntax::source_map::Span; declare_clippy_lint! { /// **What it does:** Checks for usages of `Err(x)?`. @@ -67,10 +68,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TryErr { then { let err_type = cx.tables.expr_ty(err_arg); - let suggestion = if err_type == return_type { - format!("return Err({})", snippet(cx, err_arg.span, "_")) + let span = if in_macro_or_desugar(err_arg.span) { + span_to_outer_expn(err_arg.span) } else { - format!("return Err({}.into())", snippet(cx, err_arg.span, "_")) + err_arg.span + }; + let suggestion = if err_type == return_type { + format!("return Err({})", snippet(cx, span, "_")) + } else { + format!("return Err({}.into())", snippet(cx, span, "_")) }; span_lint_and_sugg( @@ -87,6 +93,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TryErr { } } +fn span_to_outer_expn(span: Span) -> Span { + let mut span = span; + while let Some(expr) = span.ctxt().outer_expn_info() { + span = expr.call_site; + } + span +} + // In order to determine whether to suggest `.into()` or not, we need to find the error type the // function returns. To do that, we look for the From::from call (see tree above), and capture // its output type. diff --git a/tests/ui/try_err.fixed b/tests/ui/try_err.fixed index 117300e84..a2087316e 100644 --- a/tests/ui/try_err.fixed +++ b/tests/ui/try_err.fixed @@ -78,3 +78,22 @@ fn main() { closure_matches_test().unwrap(); closure_into_test().unwrap(); } + +macro_rules! bar { + () => { + String::from("aasdfasdfasdfa") + }; +} + +macro_rules! foo { + () => { + bar!() + }; +} + +pub fn macro_inside(fail: bool) -> Result { + if fail { + return Err(foo!()); + } + Ok(0) +} diff --git a/tests/ui/try_err.rs b/tests/ui/try_err.rs index 828cf639a..5ef1b615d 100644 --- a/tests/ui/try_err.rs +++ b/tests/ui/try_err.rs @@ -78,3 +78,22 @@ fn main() { closure_matches_test().unwrap(); closure_into_test().unwrap(); } + +macro_rules! bar { + () => { + String::from("aasdfasdfasdfa") + }; +} + +macro_rules! foo { + () => { + bar!() + }; +} + +pub fn macro_inside(fail: bool) -> Result { + if fail { + Err(foo!())?; + } + Ok(0) +} diff --git a/tests/ui/try_err.stderr b/tests/ui/try_err.stderr index dbe05ce51..b915d6b60 100644 --- a/tests/ui/try_err.stderr +++ b/tests/ui/try_err.stderr @@ -28,5 +28,11 @@ error: returning an `Err(_)` with the `?` operator LL | Err(err)?; | ^^^^^^^^^ help: try this: `return Err(err.into())` -error: aborting due to 4 previous errors +error: returning an `Err(_)` with the `?` operator + --> $DIR/try_err.rs:96:9 + | +LL | Err(foo!())?; + | ^^^^^^^^^^^^ help: try this: `return Err(foo!())` + +error: aborting due to 5 previous errors