Skip to content

Commit ff0993c

Browse files
borsbors
committed
Auto merge of rust-lang#5692 - ebroto:5689_N_dotdot_N, r=yaahc
reversed_empty_ranges: avoid linting N..N except in for loop arguments changelog: [`reversed_empty_ranges`]: avoid linting N..N except in for loop arguments r? @yaahc Fixes rust-lang#5689
2 parents f947644 + a664ce7 commit ff0993c

6 files changed

+51
-66
lines changed

‎clippy_lints/src/ranges.rs

Lines changed: 29 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -241,14 +241,26 @@ fn check_inclusive_range_minus_one(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
241241
}
242242

243243
fn check_reversed_empty_range(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
244-
fn inside_indexing_expr<'a>(cx: &'a LateContext<'_, '_>, expr: &Expr<'_>) -> Option<&'a Expr<'a>> {
245-
match get_parent_expr(cx, expr) {
246-
parent_expr @ Some(Expr {
244+
fn inside_indexing_expr(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
245+
matches!(
246+
get_parent_expr(cx, expr),
247+
Some(Expr {
247248
kind: ExprKind::Index(..),
248249
..
249-
}) => parent_expr,
250-
_ => None,
250+
})
251+
)
252+
}
253+
254+
fn is_for_loop_arg(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
255+
let mut cur_expr = expr;
256+
while let Some(parent_expr) = get_parent_expr(cx, cur_expr) {
257+
match higher::for_loop(parent_expr) {
258+
Some((_, args, _)) if args.hir_id == expr.hir_id => return true,
259+
_ => cur_expr = parent_expr,
260+
}
251261
}
262+
263+
false
252264
}
253265

254266
fn is_empty_range(limits: RangeLimits, ordering: Ordering) -> bool {
@@ -267,34 +279,18 @@ fn check_reversed_empty_range(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
267279
if let Some(ordering) = Constant::partial_cmp(cx.tcx, ty, &start_idx, &end_idx);
268280
if is_empty_range(limits, ordering);
269281
then {
270-
if let Some(parent_expr) = inside_indexing_expr(cx, expr) {
271-
let (reason, outcome) = if ordering == Ordering::Equal {
272-
("empty", "always yield an empty slice")
273-
} else {
274-
("reversed", "panic at run-time")
275-
};
276-
277-
span_lint_and_then(
278-
cx,
279-
REVERSED_EMPTY_RANGES,
280-
expr.span,
281-
&format!("this range is {} and using it to index a slice will {}", reason, outcome),
282-
|diag| {
283-
if_chain! {
284-
if ordering == Ordering::Equal;
285-
if let ty::Slice(slice_ty) = cx.tables.expr_ty(parent_expr).kind;
286-
then {
287-
diag.span_suggestion(
288-
parent_expr.span,
289-
"if you want an empty slice, use",
290-
format!("[] as &[{}]", slice_ty),
291-
Applicability::MaybeIncorrect
292-
);
293-
}
294-
}
295-
}
296-
);
297-
} else {
282+
if inside_indexing_expr(cx, expr) {
283+
// Avoid linting `N..N` as it has proven to be useful, see #5689 and #5628 ...
284+
if ordering != Ordering::Equal {
285+
span_lint(
286+
cx,
287+
REVERSED_EMPTY_RANGES,
288+
expr.span,
289+
"this range is reversed and using it to index a slice will panic at run-time",
290+
);
291+
}
292+
// ... except in for loop arguments for backwards compatibility with `reverse_range_loop`
293+
} else if ordering != Ordering::Equal || is_for_loop_arg(cx, expr) {
298294
span_lint_and_then(
299295
cx,
300296
REVERSED_EMPTY_RANGES,

‎tests/ui/reversed_empty_ranges_fixable.fixed

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
const ANSWER: i32 = 42;
55

66
fn main() {
7-
let arr = [1, 2, 3, 4, 5];
8-
97
// These should be linted:
108

119
(21..=42).rev().for_each(|x| println!("{}", x));
@@ -14,16 +12,18 @@ fn main() {
1412
for _ in (-42..=-21).rev() {}
1513
for _ in (21u32..42u32).rev() {}
1614

17-
let _ = &[] as &[i32];
18-
1915
// These should be ignored as they are not empty ranges:
2016

2117
(21..=42).for_each(|x| println!("{}", x));
2218
(21..42).for_each(|x| println!("{}", x));
2319

20+
let arr = [1, 2, 3, 4, 5];
2421
let _ = &arr[1..=3];
2522
let _ = &arr[1..3];
2623

2724
for _ in 21..=42 {}
2825
for _ in 21..42 {}
26+
27+
// This range is empty but should be ignored, see issue #5689
28+
let _ = &arr[0..0];
2929
}

‎tests/ui/reversed_empty_ranges_fixable.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
const ANSWER: i32 = 42;
55

66
fn main() {
7-
let arr = [1, 2, 3, 4, 5];
8-
97
// These should be linted:
108

119
(42..=21).for_each(|x| println!("{}", x));
@@ -14,16 +12,18 @@ fn main() {
1412
for _ in -21..=-42 {}
1513
for _ in 42u32..21u32 {}
1614

17-
let _ = &arr[3..3];
18-
1915
// These should be ignored as they are not empty ranges:
2016

2117
(21..=42).for_each(|x| println!("{}", x));
2218
(21..42).for_each(|x| println!("{}", x));
2319

20+
let arr = [1, 2, 3, 4, 5];
2421
let _ = &arr[1..=3];
2522
let _ = &arr[1..3];
2623

2724
for _ in 21..=42 {}
2825
for _ in 21..42 {}
26+
27+
// This range is empty but should be ignored, see issue #5689
28+
let _ = &arr[0..0];
2929
}

‎tests/ui/reversed_empty_ranges_fixable.stderr

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this range is empty so it will yield no values
2-
--> $DIR/reversed_empty_ranges_fixable.rs:11:5
2+
--> $DIR/reversed_empty_ranges_fixable.rs:9:5
33
|
44
LL | (42..=21).for_each(|x| println!("{}", x));
55
| ^^^^^^^^^
@@ -11,7 +11,7 @@ LL | (21..=42).rev().for_each(|x| println!("{}", x));
1111
| ^^^^^^^^^^^^^^^
1212

1313
error: this range is empty so it will yield no values
14-
--> $DIR/reversed_empty_ranges_fixable.rs:12:13
14+
--> $DIR/reversed_empty_ranges_fixable.rs:10:13
1515
|
1616
LL | let _ = (ANSWER..21).filter(|x| x % 2 == 0).take(10).collect::<Vec<_>>();
1717
| ^^^^^^^^^^^^
@@ -22,7 +22,7 @@ LL | let _ = (21..ANSWER).rev().filter(|x| x % 2 == 0).take(10).collect::<Ve
2222
| ^^^^^^^^^^^^^^^^^^
2323

2424
error: this range is empty so it will yield no values
25-
--> $DIR/reversed_empty_ranges_fixable.rs:14:14
25+
--> $DIR/reversed_empty_ranges_fixable.rs:12:14
2626
|
2727
LL | for _ in -21..=-42 {}
2828
| ^^^^^^^^^
@@ -33,7 +33,7 @@ LL | for _ in (-42..=-21).rev() {}
3333
| ^^^^^^^^^^^^^^^^^
3434

3535
error: this range is empty so it will yield no values
36-
--> $DIR/reversed_empty_ranges_fixable.rs:15:14
36+
--> $DIR/reversed_empty_ranges_fixable.rs:13:14
3737
|
3838
LL | for _ in 42u32..21u32 {}
3939
| ^^^^^^^^^^^^
@@ -43,11 +43,5 @@ help: consider using the following if you are attempting to iterate over this ra
4343
LL | for _ in (21u32..42u32).rev() {}
4444
| ^^^^^^^^^^^^^^^^^^^^
4545

46-
error: this range is empty and using it to index a slice will always yield an empty slice
47-
--> $DIR/reversed_empty_ranges_fixable.rs:17:18
48-
|
49-
LL | let _ = &arr[3..3];
50-
| ----^^^^- help: if you want an empty slice, use: `[] as &[i32]`
51-
52-
error: aborting due to 5 previous errors
46+
error: aborting due to 4 previous errors
5347

‎tests/ui/reversed_empty_ranges_unfixable.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@ const ANSWER: i32 = 42;
44
const SOME_NUM: usize = 3;
55

66
fn main() {
7-
let _ = (42 + 10..42 + 10).map(|x| x / 2).find(|&x| x == 21);
8-
97
let arr = [1, 2, 3, 4, 5];
108
let _ = &arr[3usize..=1usize];
119
let _ = &arr[SOME_NUM..1];
1210

1311
for _ in ANSWER..ANSWER {}
12+
13+
// Should not be linted, see issue #5689
14+
let _ = (42 + 10..42 + 10).map(|x| x / 2).find(|&x| x == 21);
1415
}

‎tests/ui/reversed_empty_ranges_unfixable.stderr

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,22 @@
1-
error: this range is empty so it will yield no values
2-
--> $DIR/reversed_empty_ranges_unfixable.rs:7:13
3-
|
4-
LL | let _ = (42 + 10..42 + 10).map(|x| x / 2).find(|&x| x == 21);
5-
| ^^^^^^^^^^^^^^^^^^
6-
|
7-
= note: `-D clippy::reversed-empty-ranges` implied by `-D warnings`
8-
91
error: this range is reversed and using it to index a slice will panic at run-time
10-
--> $DIR/reversed_empty_ranges_unfixable.rs:10:18
2+
--> $DIR/reversed_empty_ranges_unfixable.rs:8:18
113
|
124
LL | let _ = &arr[3usize..=1usize];
135
| ^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::reversed-empty-ranges` implied by `-D warnings`
148

159
error: this range is reversed and using it to index a slice will panic at run-time
16-
--> $DIR/reversed_empty_ranges_unfixable.rs:11:18
10+
--> $DIR/reversed_empty_ranges_unfixable.rs:9:18
1711
|
1812
LL | let _ = &arr[SOME_NUM..1];
1913
| ^^^^^^^^^^^
2014

2115
error: this range is empty so it will yield no values
22-
--> $DIR/reversed_empty_ranges_unfixable.rs:13:14
16+
--> $DIR/reversed_empty_ranges_unfixable.rs:11:14
2317
|
2418
LL | for _ in ANSWER..ANSWER {}
2519
| ^^^^^^^^^^^^^^
2620

27-
error: aborting due to 4 previous errors
21+
error: aborting due to 3 previous errors
2822

0 commit comments

Comments
 (0)