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.
This commit is contained in:
Michael Wright 2022-09-08 20:04:43 +02:00
parent b30c5c0554
commit a6d8afd958
4 changed files with 50 additions and 17 deletions

View File

@ -350,6 +350,7 @@ fn check_range_bounds<'a>(cx: &'a LateContext<'_>, ex: &'a Expr<'_>) -> Option<R
// exclusive range plus one: `x..(y+1)`
fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
if_chain! {
if expr.span.can_be_used_for_suggestions();
if let Some(higher::Range {
start,
end: Some(end),
@ -357,14 +358,7 @@ fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
}) = higher::Range::hir(expr);
if let Some(y) = y_plus_one(cx, end);
then {
let span = if expr.span.from_expansion() {
expr.span
.ctxt()
.outer_expn_data()
.call_site
} else {
expr.span
};
let span = expr.span;
span_lint_and_then(
cx,
RANGE_PLUS_ONE,
@ -399,6 +393,7 @@ fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
// inclusive range minus one: `x..=(y-1)`
fn check_inclusive_range_minus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
if_chain! {
if expr.span.can_be_used_for_suggestions();
if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::Range::hir(expr);
if let Some(y) = y_minus_one(cx, end);
then {

View File

@ -6,6 +6,22 @@ fn f() -> usize {
42
}
macro_rules! macro_plus_one {
($m: literal) => {
for i in 0..$m + 1 {
println!("{}", i);
}
};
}
macro_rules! macro_minus_one {
($m: literal) => {
for i in 0..=$m - 1 {
println!("{}", i);
}
};
}
#[warn(clippy::range_plus_one)]
#[warn(clippy::range_minus_one)]
fn main() {
@ -39,4 +55,7 @@ fn main() {
let mut vec: Vec<()> = std::vec::Vec::new();
vec.drain(..);
macro_plus_one!(5);
macro_minus_one!(5);
}

View File

@ -6,6 +6,22 @@ fn f() -> usize {
42
}
macro_rules! macro_plus_one {
($m: literal) => {
for i in 0..$m + 1 {
println!("{}", i);
}
};
}
macro_rules! macro_minus_one {
($m: literal) => {
for i in 0..=$m - 1 {
println!("{}", i);
}
};
}
#[warn(clippy::range_plus_one)]
#[warn(clippy::range_minus_one)]
fn main() {
@ -39,4 +55,7 @@ fn main() {
let mut vec: Vec<()> = std::vec::Vec::new();
vec.drain(..);
macro_plus_one!(5);
macro_minus_one!(5);
}

View File

@ -1,5 +1,5 @@
error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:15:14
--> $DIR/range_plus_minus_one.rs:31:14
|
LL | for _ in 0..3 + 1 {}
| ^^^^^^^^ help: use: `0..=3`
@ -7,25 +7,25 @@ LL | for _ in 0..3 + 1 {}
= note: `-D clippy::range-plus-one` implied by `-D warnings`
error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:18:14
--> $DIR/range_plus_minus_one.rs:34:14
|
LL | for _ in 0..1 + 5 {}
| ^^^^^^^^ help: use: `0..=5`
error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:21:14
--> $DIR/range_plus_minus_one.rs:37:14
|
LL | for _ in 1..1 + 1 {}
| ^^^^^^^^ help: use: `1..=1`
error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:27:14
--> $DIR/range_plus_minus_one.rs:43:14
|
LL | for _ in 0..(1 + f()) {}
| ^^^^^^^^^^^^ help: use: `0..=f()`
error: an exclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:31:13
--> $DIR/range_plus_minus_one.rs:47:13
|
LL | let _ = ..=11 - 1;
| ^^^^^^^^^ help: use: `..11`
@ -33,25 +33,25 @@ LL | let _ = ..=11 - 1;
= note: `-D clippy::range-minus-one` implied by `-D warnings`
error: an exclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:32:13
--> $DIR/range_plus_minus_one.rs:48:13
|
LL | let _ = ..=(11 - 1);
| ^^^^^^^^^^^ help: use: `..11`
error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:33:13
--> $DIR/range_plus_minus_one.rs:49:13
|
LL | let _ = (1..11 + 1);
| ^^^^^^^^^^^ help: use: `(1..=11)`
error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:34:13
--> $DIR/range_plus_minus_one.rs:50:13
|
LL | let _ = (f() + 1)..(f() + 1);
| ^^^^^^^^^^^^^^^^^^^^ help: use: `((f() + 1)..=f())`
error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:38:14
--> $DIR/range_plus_minus_one.rs:54:14
|
LL | for _ in 1..ONE + ONE {}
| ^^^^^^^^^^^^ help: use: `1..=ONE`