Skip to content

Commit b52fb52

Browse files
committed
Auto merge of #9446 - mikerite:fix-9431-2, r=giraffate
Fix `range_{plus,minus}_one` bad suggestions Fixes #9431. The current `range_plus_one` and `range_minus_one` suggestions are completely incorrect when macros are involved. This commit resolves this by disabling the lints for any range expression that is expanded from a macro. The reasons for this are that it is very difficult to create a correct suggestion in this case and that false negatives are less important for pedantic lints. changelog: Fix `range_{plus,minus}_one` bad suggestions
2 parents 32fa80d + a6d8afd commit b52fb52

File tree

4 files changed

+50
-17
lines changed

4 files changed

+50
-17
lines changed

clippy_lints/src/ranges.rs

+3-8
Original file line numberDiff line numberDiff line change
@@ -350,21 +350,15 @@ fn check_range_bounds<'a>(cx: &'a LateContext<'_>, ex: &'a Expr<'_>) -> Option<R
350350
// exclusive range plus one: `x..(y+1)`
351351
fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
352352
if_chain! {
353+
if expr.span.can_be_used_for_suggestions();
353354
if let Some(higher::Range {
354355
start,
355356
end: Some(end),
356357
limits: RangeLimits::HalfOpen
357358
}) = higher::Range::hir(expr);
358359
if let Some(y) = y_plus_one(cx, end);
359360
then {
360-
let span = if expr.span.from_expansion() {
361-
expr.span
362-
.ctxt()
363-
.outer_expn_data()
364-
.call_site
365-
} else {
366-
expr.span
367-
};
361+
let span = expr.span;
368362
span_lint_and_then(
369363
cx,
370364
RANGE_PLUS_ONE,
@@ -399,6 +393,7 @@ fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
399393
// inclusive range minus one: `x..=(y-1)`
400394
fn check_inclusive_range_minus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
401395
if_chain! {
396+
if expr.span.can_be_used_for_suggestions();
402397
if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::Range::hir(expr);
403398
if let Some(y) = y_minus_one(cx, end);
404399
then {

tests/ui/range_plus_minus_one.fixed

+19
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,22 @@ fn f() -> usize {
66
42
77
}
88

9+
macro_rules! macro_plus_one {
10+
($m: literal) => {
11+
for i in 0..$m + 1 {
12+
println!("{}", i);
13+
}
14+
};
15+
}
16+
17+
macro_rules! macro_minus_one {
18+
($m: literal) => {
19+
for i in 0..=$m - 1 {
20+
println!("{}", i);
21+
}
22+
};
23+
}
24+
925
#[warn(clippy::range_plus_one)]
1026
#[warn(clippy::range_minus_one)]
1127
fn main() {
@@ -39,4 +55,7 @@ fn main() {
3955

4056
let mut vec: Vec<()> = std::vec::Vec::new();
4157
vec.drain(..);
58+
59+
macro_plus_one!(5);
60+
macro_minus_one!(5);
4261
}

tests/ui/range_plus_minus_one.rs

+19
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,22 @@ fn f() -> usize {
66
42
77
}
88

9+
macro_rules! macro_plus_one {
10+
($m: literal) => {
11+
for i in 0..$m + 1 {
12+
println!("{}", i);
13+
}
14+
};
15+
}
16+
17+
macro_rules! macro_minus_one {
18+
($m: literal) => {
19+
for i in 0..=$m - 1 {
20+
println!("{}", i);
21+
}
22+
};
23+
}
24+
925
#[warn(clippy::range_plus_one)]
1026
#[warn(clippy::range_minus_one)]
1127
fn main() {
@@ -39,4 +55,7 @@ fn main() {
3955

4056
let mut vec: Vec<()> = std::vec::Vec::new();
4157
vec.drain(..);
58+
59+
macro_plus_one!(5);
60+
macro_minus_one!(5);
4261
}

tests/ui/range_plus_minus_one.stderr

+9-9
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,57 @@
11
error: an inclusive range would be more readable
2-
--> $DIR/range_plus_minus_one.rs:15:14
2+
--> $DIR/range_plus_minus_one.rs:31:14
33
|
44
LL | for _ in 0..3 + 1 {}
55
| ^^^^^^^^ help: use: `0..=3`
66
|
77
= note: `-D clippy::range-plus-one` implied by `-D warnings`
88

99
error: an inclusive range would be more readable
10-
--> $DIR/range_plus_minus_one.rs:18:14
10+
--> $DIR/range_plus_minus_one.rs:34:14
1111
|
1212
LL | for _ in 0..1 + 5 {}
1313
| ^^^^^^^^ help: use: `0..=5`
1414

1515
error: an inclusive range would be more readable
16-
--> $DIR/range_plus_minus_one.rs:21:14
16+
--> $DIR/range_plus_minus_one.rs:37:14
1717
|
1818
LL | for _ in 1..1 + 1 {}
1919
| ^^^^^^^^ help: use: `1..=1`
2020

2121
error: an inclusive range would be more readable
22-
--> $DIR/range_plus_minus_one.rs:27:14
22+
--> $DIR/range_plus_minus_one.rs:43:14
2323
|
2424
LL | for _ in 0..(1 + f()) {}
2525
| ^^^^^^^^^^^^ help: use: `0..=f()`
2626

2727
error: an exclusive range would be more readable
28-
--> $DIR/range_plus_minus_one.rs:31:13
28+
--> $DIR/range_plus_minus_one.rs:47:13
2929
|
3030
LL | let _ = ..=11 - 1;
3131
| ^^^^^^^^^ help: use: `..11`
3232
|
3333
= note: `-D clippy::range-minus-one` implied by `-D warnings`
3434

3535
error: an exclusive range would be more readable
36-
--> $DIR/range_plus_minus_one.rs:32:13
36+
--> $DIR/range_plus_minus_one.rs:48:13
3737
|
3838
LL | let _ = ..=(11 - 1);
3939
| ^^^^^^^^^^^ help: use: `..11`
4040

4141
error: an inclusive range would be more readable
42-
--> $DIR/range_plus_minus_one.rs:33:13
42+
--> $DIR/range_plus_minus_one.rs:49:13
4343
|
4444
LL | let _ = (1..11 + 1);
4545
| ^^^^^^^^^^^ help: use: `(1..=11)`
4646

4747
error: an inclusive range would be more readable
48-
--> $DIR/range_plus_minus_one.rs:34:13
48+
--> $DIR/range_plus_minus_one.rs:50:13
4949
|
5050
LL | let _ = (f() + 1)..(f() + 1);
5151
| ^^^^^^^^^^^^^^^^^^^^ help: use: `((f() + 1)..=f())`
5252

5353
error: an inclusive range would be more readable
54-
--> $DIR/range_plus_minus_one.rs:38:14
54+
--> $DIR/range_plus_minus_one.rs:54:14
5555
|
5656
LL | for _ in 1..ONE + ONE {}
5757
| ^^^^^^^^^^^^ help: use: `1..=ONE`

0 commit comments

Comments
 (0)