Skip to content

Commit fc1a9ef

Browse files
committed
fix FP with MBEs
1 parent 114d4fa commit fc1a9ef

File tree

4 files changed

+111
-4
lines changed

4 files changed

+111
-4
lines changed

clippy_lints/src/needless_parens_on_range_literals.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::higher;
3-
use clippy_utils::source::{SpanRangeExt, snippet_with_applicability};
3+
use clippy_utils::source::{SpanRangeExt, snippet_with_context};
44

55
use rustc_ast::ast;
66
use rustc_errors::Applicability;
@@ -41,7 +41,8 @@ declare_clippy_lint! {
4141
declare_lint_pass!(NeedlessParensOnRangeLiterals => [NEEDLESS_PARENS_ON_RANGE_LITERALS]);
4242

4343
fn check_for_parens(cx: &LateContext<'_>, e: &Expr<'_>, is_start: bool) {
44-
if let ExprKind::Lit(literal) = e.kind
44+
if !e.span.from_expansion()
45+
&& let ExprKind::Lit(literal) = e.kind
4546
// the indicator that parenthesis surround the literal is that the span of the expression and the literal differ
4647
&& literal.span != e.span
4748
// inspect the source code of the expression for parenthesis
@@ -56,7 +57,7 @@ fn check_for_parens(cx: &LateContext<'_>, e: &Expr<'_>, is_start: bool) {
5657
}
5758

5859
let mut applicability = Applicability::MachineApplicable;
59-
let suggestion = snippet_with_applicability(cx, literal.span, "_", &mut applicability);
60+
let suggestion = snippet_with_context(cx, literal.span, e.span.ctxt(), "_", &mut applicability).0;
6061
span_lint_and_sugg(
6162
cx,
6263
NEEDLESS_PARENS_ON_RANGE_LITERALS,

tests/ui/needless_parens_on_range_literals.fixed

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,51 @@ fn main() {
1818
//~^ needless_parens_on_range_literals
1919
let _ = ..'z';
2020
//~^ needless_parens_on_range_literals
21+
22+
macro_rules! verbatim {
23+
($e:expr) => {
24+
$e
25+
};
26+
}
27+
macro_rules! add_paren {
28+
($e:expr) => {
29+
($e)
30+
};
31+
}
32+
33+
// lint: the paren was added by the user
34+
let _ = verbatim!(0)..1;
35+
//~^ needless_parens_on_range_literals
36+
let _ = 0..verbatim!(1);
37+
//~^ needless_parens_on_range_literals
38+
39+
// lint: the macro doesn't have anything to do with the paren
40+
// NOTE: This doesn't actually work currently.
41+
//
42+
// If we have a case like this:
43+
// ```
44+
// macro_rules! zero {
45+
// () => {
46+
// 0
47+
// };
48+
// }
49+
// let _ = (zero!())..1;
50+
// ```
51+
// then the lint doesn't trigger, because the span comes from expansion.
52+
// "But surely the outer `()` don't come from an expansion!", I hear you say. But the thing is that
53+
// those parens aren't present in the HIR in the first place, and so the expression we actually
54+
// end up looking at is `zero!()`, which is in fact an expansion.
55+
//
56+
// And if we have a case like this:
57+
// ```
58+
// let _ = (verbatim!(0))..1;
59+
// ```
60+
// then the parens get ignored as described above, but in addition to that, the whole `verbatim!(0)`
61+
// gets replaced with just `0`, which is something that we've written, so its span now gets
62+
// doubly-incorrectly identified as not coming from an expansion, and so the lint fires.
63+
// But the problem arises when we get to creating the suggestion -- as it _also_ doesn't see the
64+
// `verbatim!`, our suggestion ends up being just `let _ = 0..1;`, which is of course incorrect.
65+
66+
// don't lint: the paren was added by the macro
67+
let _ = add_paren!(0)..1;
2168
}

tests/ui/needless_parens_on_range_literals.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,51 @@ fn main() {
1818
//~^ needless_parens_on_range_literals
1919
let _ = ..('z');
2020
//~^ needless_parens_on_range_literals
21+
22+
macro_rules! verbatim {
23+
($e:expr) => {
24+
$e
25+
};
26+
}
27+
macro_rules! add_paren {
28+
($e:expr) => {
29+
($e)
30+
};
31+
}
32+
33+
// lint: the paren was added by the user
34+
let _ = verbatim!((0))..1;
35+
//~^ needless_parens_on_range_literals
36+
let _ = 0..verbatim!((1));
37+
//~^ needless_parens_on_range_literals
38+
39+
// lint: the macro doesn't have anything to do with the paren
40+
// NOTE: This doesn't actually work currently.
41+
//
42+
// If we have a case like this:
43+
// ```
44+
// macro_rules! zero {
45+
// () => {
46+
// 0
47+
// };
48+
// }
49+
// let _ = (zero!())..1;
50+
// ```
51+
// then the lint doesn't trigger, because the span comes from expansion.
52+
// "But surely the outer `()` don't come from an expansion!", I hear you say. But the thing is that
53+
// those parens aren't present in the HIR in the first place, and so the expression we actually
54+
// end up looking at is `zero!()`, which is in fact an expansion.
55+
//
56+
// And if we have a case like this:
57+
// ```
58+
// let _ = (verbatim!(0))..1;
59+
// ```
60+
// then the parens get ignored as described above, but in addition to that, the whole `verbatim!(0)`
61+
// gets replaced with just `0`, which is something that we've written, so its span now gets
62+
// doubly-incorrectly identified as not coming from an expansion, and so the lint fires.
63+
// But the problem arises when we get to creating the suggestion -- as it _also_ doesn't see the
64+
// `verbatim!`, our suggestion ends up being just `let _ = 0..1;`, which is of course incorrect.
65+
66+
// don't lint: the paren was added by the macro
67+
let _ = add_paren!(0)..1;
2168
}

tests/ui/needless_parens_on_range_literals.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,17 @@ error: needless parenthesis on range literals can be removed
4343
LL | let _ = ..('z');
4444
| ^^^^^ help: try: `'z'`
4545

46-
error: aborting due to 7 previous errors
46+
error: needless parenthesis on range literals can be removed
47+
--> tests/ui/needless_parens_on_range_literals.rs:34:23
48+
|
49+
LL | let _ = verbatim!((0))..1;
50+
| ^^^ help: try: `0`
51+
52+
error: needless parenthesis on range literals can be removed
53+
--> tests/ui/needless_parens_on_range_literals.rs:36:26
54+
|
55+
LL | let _ = 0..verbatim!((1));
56+
| ^^^ help: try: `1`
57+
58+
error: aborting due to 9 previous errors
4759

0 commit comments

Comments
 (0)