Skip to content

Commit 0487b58

Browse files
committed
Fix macro expansion in try_err lint
1 parent b041511 commit 0487b58

File tree

4 files changed

+62
-4
lines changed

4 files changed

+62
-4
lines changed

clippy_lints/src/try_err.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
use crate::utils::{match_qpath, paths, snippet, span_lint_and_sugg};
1+
use crate::utils::{in_macro_or_desugar, match_qpath, paths, snippet, span_lint_and_sugg};
22
use if_chain::if_chain;
33
use rustc::hir::*;
44
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
55
use rustc::ty::Ty;
66
use rustc::{declare_lint_pass, declare_tool_lint};
77
use rustc_errors::Applicability;
8+
use syntax::source_map::Span;
89

910
declare_clippy_lint! {
1011
/// **What it does:** Checks for usages of `Err(x)?`.
@@ -67,10 +68,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TryErr {
6768

6869
then {
6970
let err_type = cx.tables.expr_ty(err_arg);
71+
let span = if in_macro_or_desugar(err_arg.span) {
72+
span_to_outer_expn(err_arg.span)
73+
} else {
74+
err_arg.span
75+
};
7076
let suggestion = if err_type == return_type {
71-
format!("return Err({})", snippet(cx, err_arg.span, "_"))
77+
format!("return Err({})", snippet(cx, span, "_"))
7278
} else {
73-
format!("return Err({}.into())", snippet(cx, err_arg.span, "_"))
79+
format!("return Err({}.into())", snippet(cx, span, "_"))
7480
};
7581

7682
span_lint_and_sugg(
@@ -87,6 +93,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TryErr {
8793
}
8894
}
8995

96+
fn span_to_outer_expn(span: Span) -> Span {
97+
let mut span = span;
98+
while let Some(expr) = span.ctxt().outer_expn_info() {
99+
span = expr.call_site;
100+
}
101+
span
102+
}
103+
90104
// In order to determine whether to suggest `.into()` or not, we need to find the error type the
91105
// function returns. To do that, we look for the From::from call (see tree above), and capture
92106
// its output type.

tests/ui/try_err.fixed

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,22 @@ fn main() {
7878
closure_matches_test().unwrap();
7979
closure_into_test().unwrap();
8080
}
81+
82+
macro_rules! bar {
83+
() => {
84+
String::from("aasdfasdfasdfa")
85+
};
86+
}
87+
88+
macro_rules! foo {
89+
() => {
90+
bar!()
91+
};
92+
}
93+
94+
pub fn macro_inside(fail: bool) -> Result<i32, String> {
95+
if fail {
96+
return Err(foo!());
97+
}
98+
Ok(0)
99+
}

tests/ui/try_err.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,22 @@ fn main() {
7878
closure_matches_test().unwrap();
7979
closure_into_test().unwrap();
8080
}
81+
82+
macro_rules! bar {
83+
() => {
84+
String::from("aasdfasdfasdfa")
85+
};
86+
}
87+
88+
macro_rules! foo {
89+
() => {
90+
bar!()
91+
};
92+
}
93+
94+
pub fn macro_inside(fail: bool) -> Result<i32, String> {
95+
if fail {
96+
Err(foo!())?;
97+
}
98+
Ok(0)
99+
}

tests/ui/try_err.stderr

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,11 @@ error: returning an `Err(_)` with the `?` operator
2828
LL | Err(err)?;
2929
| ^^^^^^^^^ help: try this: `return Err(err.into())`
3030

31-
error: aborting due to 4 previous errors
31+
error: returning an `Err(_)` with the `?` operator
32+
--> $DIR/try_err.rs:96:9
33+
|
34+
LL | Err(foo!())?;
35+
| ^^^^^^^^^^^^ help: try this: `return Err(foo!())`
36+
37+
error: aborting due to 5 previous errors
3238

0 commit comments

Comments
 (0)