Skip to content

Commit ce98468

Browse files
committed
Fix incorrect suggestion when from expansion in try_err lint
1 parent 343bdb3 commit ce98468

File tree

4 files changed

+56
-7
lines changed

4 files changed

+56
-7
lines changed

clippy_lints/src/try_err.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::utils::{
2-
is_type_diagnostic_item, match_def_path, match_qpath, paths, snippet, snippet_with_macro_callsite,
2+
in_macro, is_type_diagnostic_item, match_def_path, match_qpath, paths, snippet, snippet_with_macro_callsite,
33
span_lint_and_sugg,
44
};
55
use if_chain::if_chain;
@@ -92,16 +92,22 @@ impl<'tcx> LateLintPass<'tcx> for TryErr {
9292

9393
let expr_err_ty = cx.typeck_results().expr_ty(err_arg);
9494

95-
let origin_snippet = if err_arg.span.from_expansion() {
95+
// println!("\n\n{:?}", in_macro(expr.span));
96+
// println!("{:#?}", snippet(cx, err_arg.span, "_"));
97+
let origin_snippet = if err_arg.span.from_expansion() && !in_macro(expr.span) {
98+
// println!("from expansion");
9699
snippet_with_macro_callsite(cx, err_arg.span, "_")
97100
} else {
101+
// println!("just a snippet");
98102
snippet(cx, err_arg.span, "_")
99103
};
100104
let suggestion = if err_ty == expr_err_ty {
101105
format!("return {}{}{}", prefix, origin_snippet, suffix)
102106
} else {
103107
format!("return {}{}.into(){}", prefix, origin_snippet, suffix)
104108
};
109+
// println!("origin_snippet: {:#?}", origin_snippet);
110+
// println!("suggestion: {:#?}", suggestion);
105111

106112
span_lint_and_sugg(
107113
cx,

tests/ui/try_err.fixed

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,28 @@ fn nested_error() -> Result<i32, i32> {
7878
Ok(1)
7979
}
8080

81+
// Bad suggestion when in macro (see #6242)
82+
macro_rules! try_validation {
83+
($e: expr) => {{
84+
match $e {
85+
Ok(_) => 0,
86+
Err(_) => return Err(1),
87+
}
88+
}};
89+
}
90+
91+
fn calling_macro() -> Result<i32, i32> {
92+
try_validation!(Ok::<_, i32>(5));
93+
Ok(5)
94+
}
95+
8196
fn main() {
8297
basic_test().unwrap();
8398
into_test().unwrap();
8499
negative_test().unwrap();
85100
closure_matches_test().unwrap();
86101
closure_into_test().unwrap();
102+
calling_macro().unwrap();
87103

88104
// We don't want to lint in external macros
89105
try_err!();

tests/ui/try_err.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,28 @@ fn nested_error() -> Result<i32, i32> {
7878
Ok(1)
7979
}
8080

81+
// Bad suggestion when in macro (see #6242)
82+
macro_rules! try_validation {
83+
($e: expr) => {{
84+
match $e {
85+
Ok(_) => 0,
86+
Err(_) => Err(1)?,
87+
}
88+
}};
89+
}
90+
91+
fn calling_macro() -> Result<i32, i32> {
92+
try_validation!(Ok::<_, i32>(5));
93+
Ok(5)
94+
}
95+
8196
fn main() {
8297
basic_test().unwrap();
8398
into_test().unwrap();
8499
negative_test().unwrap();
85100
closure_matches_test().unwrap();
86101
closure_into_test().unwrap();
102+
calling_macro().unwrap();
87103

88104
// We don't want to lint in external macros
89105
try_err!();

tests/ui/try_err.stderr

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,28 +29,39 @@ LL | Err(err)?;
2929
| ^^^^^^^^^ help: try this: `return Err(err.into())`
3030

3131
error: returning an `Err(_)` with the `?` operator
32-
--> $DIR/try_err.rs:106:9
32+
--> $DIR/try_err.rs:86:23
33+
|
34+
LL | Err(_) => Err(1)?,
35+
| ^^^^^^^ help: try this: `return Err(1)`
36+
...
37+
LL | try_validation!(Ok::<_, i32>(5));
38+
| --------------------------------- in this macro invocation
39+
|
40+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
41+
42+
error: returning an `Err(_)` with the `?` operator
43+
--> $DIR/try_err.rs:122:9
3344
|
3445
LL | Err(foo!())?;
3546
| ^^^^^^^^^^^^ help: try this: `return Err(foo!())`
3647

3748
error: returning an `Err(_)` with the `?` operator
38-
--> $DIR/try_err.rs:113:9
49+
--> $DIR/try_err.rs:129:9
3950
|
4051
LL | Err(io::ErrorKind::WriteZero)?
4152
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `return Poll::Ready(Err(io::ErrorKind::WriteZero.into()))`
4253

4354
error: returning an `Err(_)` with the `?` operator
44-
--> $DIR/try_err.rs:115:9
55+
--> $DIR/try_err.rs:131:9
4556
|
4657
LL | Err(io::Error::new(io::ErrorKind::InvalidInput, "error"))?
4758
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `return Poll::Ready(Err(io::Error::new(io::ErrorKind::InvalidInput, "error")))`
4859

4960
error: returning an `Err(_)` with the `?` operator
50-
--> $DIR/try_err.rs:123:9
61+
--> $DIR/try_err.rs:139:9
5162
|
5263
LL | Err(io::ErrorKind::NotFound)?
5364
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `return Poll::Ready(Some(Err(io::ErrorKind::NotFound.into())))`
5465

55-
error: aborting due to 8 previous errors
66+
error: aborting due to 9 previous errors
5667

0 commit comments

Comments
 (0)