From 0487b58f9afc249e850dc1bf2c8ae6d660237244 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Thu, 8 Aug 2019 19:33:34 +0700 Subject: [PATCH 1/2] Fix macro expansion in try_err lint --- clippy_lints/src/try_err.rs | 22 ++++++++++++++++++---- tests/ui/try_err.fixed | 19 +++++++++++++++++++ tests/ui/try_err.rs | 19 +++++++++++++++++++ tests/ui/try_err.stderr | 8 +++++++- 4 files changed, 63 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/try_err.rs b/clippy_lints/src/try_err.rs index 7466221fb11..3734b609861 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 117300e84a4..a2087316e37 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 828cf639a1b..5ef1b615dc7 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 dbe05ce5178..b915d6b601d 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 From 90a7b6041319085634666d1ca1a28d79bd7ba6cd Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Thu, 8 Aug 2019 19:33:34 +0700 Subject: [PATCH 2/2] Use snippet_with_macro_callsite suggested by flip1995 --- clippy_lints/src/try_err.rs | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/try_err.rs b/clippy_lints/src/try_err.rs index 3734b609861..7eba331ae0f 100644 --- a/clippy_lints/src/try_err.rs +++ b/clippy_lints/src/try_err.rs @@ -1,11 +1,10 @@ -use crate::utils::{in_macro_or_desugar, match_qpath, paths, snippet, span_lint_and_sugg}; +use crate::utils::{in_macro_or_desugar, match_qpath, paths, snippet, snippet_with_macro_callsite, 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)?`. @@ -68,15 +67,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TryErr { then { let err_type = cx.tables.expr_ty(err_arg); - let span = if in_macro_or_desugar(err_arg.span) { - span_to_outer_expn(err_arg.span) + let origin_snippet = if in_macro_or_desugar(err_arg.span) { + snippet_with_macro_callsite(cx, err_arg.span, "_") } else { - err_arg.span + snippet(cx, err_arg.span, "_") }; let suggestion = if err_type == return_type { - format!("return Err({})", snippet(cx, span, "_")) + format!("return Err({})", origin_snippet) } else { - format!("return Err({}.into())", snippet(cx, span, "_")) + format!("return Err({}.into())", origin_snippet) }; span_lint_and_sugg( @@ -93,14 +92,6 @@ 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.