reversed_empty_ranges: don't lint N..N except in for loop arg

This commit is contained in:
Eduardo Broto 2020-06-07 20:38:28 +02:00
parent 67ec96c8f9
commit ebfc1da07d
6 changed files with 52 additions and 67 deletions

View file

@ -241,14 +241,26 @@ fn check_inclusive_range_minus_one(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
} }
fn check_reversed_empty_range(cx: &LateContext<'_, '_>, expr: &Expr<'_>) { fn check_reversed_empty_range(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
fn inside_indexing_expr<'a>(cx: &'a LateContext<'_, '_>, expr: &Expr<'_>) -> Option<&'a Expr<'a>> { fn inside_indexing_expr<'a>(cx: &'a LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
match get_parent_expr(cx, expr) { matches!(
parent_expr @ Some(Expr { get_parent_expr(cx, expr),
Some(Expr {
kind: ExprKind::Index(..), kind: ExprKind::Index(..),
.. ..
}) => parent_expr, })
_ => None, )
}
fn is_for_loop_arg(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
let mut cur_expr = expr;
while let Some(parent_expr) = get_parent_expr(cx, cur_expr) {
match higher::for_loop(parent_expr) {
Some((_, args, _)) if args.hir_id == expr.hir_id => return true,
_ => cur_expr = parent_expr,
}
} }
false
} }
fn is_empty_range(limits: RangeLimits, ordering: Ordering) -> bool { fn is_empty_range(limits: RangeLimits, ordering: Ordering) -> bool {
@ -267,34 +279,18 @@ fn check_reversed_empty_range(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
if let Some(ordering) = Constant::partial_cmp(cx.tcx, ty, &start_idx, &end_idx); if let Some(ordering) = Constant::partial_cmp(cx.tcx, ty, &start_idx, &end_idx);
if is_empty_range(limits, ordering); if is_empty_range(limits, ordering);
then { then {
if let Some(parent_expr) = inside_indexing_expr(cx, expr) { if inside_indexing_expr(cx, expr) {
let (reason, outcome) = if ordering == Ordering::Equal { // Avoid linting `N..N` as it has proven to be useful, see #5689 and #5628 ...
("empty", "always yield an empty slice") if ordering != Ordering::Equal {
} else { span_lint(
("reversed", "panic at run-time") cx,
}; REVERSED_EMPTY_RANGES,
expr.span,
span_lint_and_then( "this range is reversed and using it to index a slice will panic at run-time",
cx, );
REVERSED_EMPTY_RANGES, }
expr.span, // ... except in for loop arguments for backwards compatibility with `reverse_range_loop`
&format!("this range is {} and using it to index a slice will {}", reason, outcome), } else if ordering != Ordering::Equal || is_for_loop_arg(cx, expr) {
|diag| {
if_chain! {
if ordering == Ordering::Equal;
if let ty::Slice(slice_ty) = cx.tables.expr_ty(parent_expr).kind;
then {
diag.span_suggestion(
parent_expr.span,
"if you want an empty slice, use",
format!("[] as &[{}]", slice_ty),
Applicability::MaybeIncorrect
);
}
}
}
);
} else {
span_lint_and_then( span_lint_and_then(
cx, cx,
REVERSED_EMPTY_RANGES, REVERSED_EMPTY_RANGES,

View file

@ -4,8 +4,6 @@
const ANSWER: i32 = 42; const ANSWER: i32 = 42;
fn main() { fn main() {
let arr = [1, 2, 3, 4, 5];
// These should be linted: // These should be linted:
(21..=42).rev().for_each(|x| println!("{}", x)); (21..=42).rev().for_each(|x| println!("{}", x));
@ -14,16 +12,18 @@ fn main() {
for _ in (-42..=-21).rev() {} for _ in (-42..=-21).rev() {}
for _ in (21u32..42u32).rev() {} for _ in (21u32..42u32).rev() {}
let _ = &[] as &[i32];
// These should be ignored as they are not empty ranges: // These should be ignored as they are not empty ranges:
(21..=42).for_each(|x| println!("{}", x)); (21..=42).for_each(|x| println!("{}", x));
(21..42).for_each(|x| println!("{}", x)); (21..42).for_each(|x| println!("{}", x));
let arr = [1, 2, 3, 4, 5];
let _ = &arr[1..=3]; let _ = &arr[1..=3];
let _ = &arr[1..3]; let _ = &arr[1..3];
for _ in 21..=42 {} for _ in 21..=42 {}
for _ in 21..42 {} for _ in 21..42 {}
// This range is empty but should be ignored, see issue #5689
let _ = &arr[0..0];
} }

View file

@ -4,8 +4,6 @@
const ANSWER: i32 = 42; const ANSWER: i32 = 42;
fn main() { fn main() {
let arr = [1, 2, 3, 4, 5];
// These should be linted: // These should be linted:
(42..=21).for_each(|x| println!("{}", x)); (42..=21).for_each(|x| println!("{}", x));
@ -14,16 +12,18 @@ fn main() {
for _ in -21..=-42 {} for _ in -21..=-42 {}
for _ in 42u32..21u32 {} for _ in 42u32..21u32 {}
let _ = &arr[3..3];
// These should be ignored as they are not empty ranges: // These should be ignored as they are not empty ranges:
(21..=42).for_each(|x| println!("{}", x)); (21..=42).for_each(|x| println!("{}", x));
(21..42).for_each(|x| println!("{}", x)); (21..42).for_each(|x| println!("{}", x));
let arr = [1, 2, 3, 4, 5];
let _ = &arr[1..=3]; let _ = &arr[1..=3];
let _ = &arr[1..3]; let _ = &arr[1..3];
for _ in 21..=42 {} for _ in 21..=42 {}
for _ in 21..42 {} for _ in 21..42 {}
// This range is empty but should be ignored, see issue #5689
let _ = &arr[0..0];
} }

View file

@ -1,5 +1,5 @@
error: this range is empty so it will yield no values error: this range is empty so it will yield no values
--> $DIR/reversed_empty_ranges_fixable.rs:11:5 --> $DIR/reversed_empty_ranges_fixable.rs:9:5
| |
LL | (42..=21).for_each(|x| println!("{}", x)); LL | (42..=21).for_each(|x| println!("{}", x));
| ^^^^^^^^^ | ^^^^^^^^^
@ -11,7 +11,7 @@ LL | (21..=42).rev().for_each(|x| println!("{}", x));
| ^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^
error: this range is empty so it will yield no values error: this range is empty so it will yield no values
--> $DIR/reversed_empty_ranges_fixable.rs:12:13 --> $DIR/reversed_empty_ranges_fixable.rs:10:13
| |
LL | let _ = (ANSWER..21).filter(|x| x % 2 == 0).take(10).collect::<Vec<_>>(); LL | let _ = (ANSWER..21).filter(|x| x % 2 == 0).take(10).collect::<Vec<_>>();
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^
@ -22,7 +22,7 @@ LL | let _ = (21..ANSWER).rev().filter(|x| x % 2 == 0).take(10).collect::<Ve
| ^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^
error: this range is empty so it will yield no values error: this range is empty so it will yield no values
--> $DIR/reversed_empty_ranges_fixable.rs:14:14 --> $DIR/reversed_empty_ranges_fixable.rs:12:14
| |
LL | for _ in -21..=-42 {} LL | for _ in -21..=-42 {}
| ^^^^^^^^^ | ^^^^^^^^^
@ -33,7 +33,7 @@ LL | for _ in (-42..=-21).rev() {}
| ^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^
error: this range is empty so it will yield no values error: this range is empty so it will yield no values
--> $DIR/reversed_empty_ranges_fixable.rs:15:14 --> $DIR/reversed_empty_ranges_fixable.rs:13:14
| |
LL | for _ in 42u32..21u32 {} LL | for _ in 42u32..21u32 {}
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^
@ -43,11 +43,5 @@ help: consider using the following if you are attempting to iterate over this ra
LL | for _ in (21u32..42u32).rev() {} LL | for _ in (21u32..42u32).rev() {}
| ^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^
error: this range is empty and using it to index a slice will always yield an empty slice error: aborting due to 4 previous errors
--> $DIR/reversed_empty_ranges_fixable.rs:17:18
|
LL | let _ = &arr[3..3];
| ----^^^^- help: if you want an empty slice, use: `[] as &[i32]`
error: aborting due to 5 previous errors

View file

@ -4,11 +4,12 @@ const ANSWER: i32 = 42;
const SOME_NUM: usize = 3; const SOME_NUM: usize = 3;
fn main() { fn main() {
let _ = (42 + 10..42 + 10).map(|x| x / 2).find(|&x| x == 21);
let arr = [1, 2, 3, 4, 5]; let arr = [1, 2, 3, 4, 5];
let _ = &arr[3usize..=1usize]; let _ = &arr[3usize..=1usize];
let _ = &arr[SOME_NUM..1]; let _ = &arr[SOME_NUM..1];
for _ in ANSWER..ANSWER {} for _ in ANSWER..ANSWER {}
// Should not be linted, see issue #5689
let _ = (42 + 10..42 + 10).map(|x| x / 2).find(|&x| x == 21);
} }

View file

@ -1,28 +1,22 @@
error: this range is empty so it will yield no values error: this range is reversed and using it to index a slice will panic at run-time
--> $DIR/reversed_empty_ranges_unfixable.rs:7:13 --> $DIR/reversed_empty_ranges_unfixable.rs:8:18
| |
LL | let _ = (42 + 10..42 + 10).map(|x| x / 2).find(|&x| x == 21); LL | let _ = &arr[3usize..=1usize];
| ^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^
| |
= note: `-D clippy::reversed-empty-ranges` implied by `-D warnings` = note: `-D clippy::reversed-empty-ranges` implied by `-D warnings`
error: this range is reversed and using it to index a slice will panic at run-time error: this range is reversed and using it to index a slice will panic at run-time
--> $DIR/reversed_empty_ranges_unfixable.rs:10:18 --> $DIR/reversed_empty_ranges_unfixable.rs:9:18
|
LL | let _ = &arr[3usize..=1usize];
| ^^^^^^^^^^^^^^^
error: this range is reversed and using it to index a slice will panic at run-time
--> $DIR/reversed_empty_ranges_unfixable.rs:11:18
| |
LL | let _ = &arr[SOME_NUM..1]; LL | let _ = &arr[SOME_NUM..1];
| ^^^^^^^^^^^ | ^^^^^^^^^^^
error: this range is empty so it will yield no values error: this range is empty so it will yield no values
--> $DIR/reversed_empty_ranges_unfixable.rs:13:14 --> $DIR/reversed_empty_ranges_unfixable.rs:11:14
| |
LL | for _ in ANSWER..ANSWER {} LL | for _ in ANSWER..ANSWER {}
| ^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^
error: aborting due to 4 previous errors error: aborting due to 3 previous errors