mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #5692 - ebroto:5689_N_dotdot_N, r=yaahc

Browse Source 
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 #5689
This commit is contained in:
bors 2020-06-08 17:15:45 +00:00
commit ff0993c5e9
6 changed files with 52 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
clippy_lints/src/ranges.rs
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 inside_indexing_expr<'a>(cx: &'a LateContext<'_, '_>, expr: &Expr<'_>) -> Option<&'a Expr<'a>> {
match get_parent_expr(cx, expr) {
parent_expr @ Some(Expr {
fn inside_indexing_expr(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
matches!(
get_parent_expr(cx, expr),
Some(Expr {
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 {
@ -267,34 +279,18 @@ fn is_empty_range(limits: RangeLimits, ordering: Ordering) -> bool {
if let Some(ordering) = Constant::partial_cmp(cx.tcx, ty, &start_idx, &end_idx);
if is_empty_range(limits, ordering);
then {
if let Some(parent_expr) = inside_indexing_expr(cx, expr) {
let (reason, outcome) = if ordering == Ordering::Equal {
("empty", "always yield an empty slice")
} else {
("reversed", "panic at run-time")
};
span_lint_and_then(
cx,
REVERSED_EMPTY_RANGES,
expr.span,
&format!("this range is {} and using it to index a slice will {}", reason, outcome),
|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 {
if inside_indexing_expr(cx, expr) {
// Avoid linting `N..N` as it has proven to be useful, see #5689 and #5628 ...
if ordering != Ordering::Equal {
span_lint(
cx,
REVERSED_EMPTY_RANGES,
expr.span,
"this range is reversed and using it to index a slice will panic at run-time",
);
}
// ... except in for loop arguments for backwards compatibility with `reverse_range_loop`
} else if ordering != Ordering::Equal || is_for_loop_arg(cx, expr) {
span_lint_and_then(
cx,
REVERSED_EMPTY_RANGES,

8
tests/ui/reversed_empty_ranges_fixable.fixed
View File

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

8
tests/ui/reversed_empty_ranges_fixable.rs
View File

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

16
tests/ui/reversed_empty_ranges_fixable.stderr
View File

@ -1,5 +1,5 @@
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));
| ^^^^^^^^^
@ -11,7 +11,7 @@ LL | (21..=42).rev().for_each(|x| println!("{}", x));
| ^^^^^^^^^^^^^^^
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<_>>();
| ^^^^^^^^^^^^
@ -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
--> $DIR/reversed_empty_ranges_fixable.rs:14:14
--> $DIR/reversed_empty_ranges_fixable.rs:12:14
|
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
--> $DIR/reversed_empty_ranges_fixable.rs:15:14
--> $DIR/reversed_empty_ranges_fixable.rs:13:14
|
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() {}
| ^^^^^^^^^^^^^^^^^^^^
error: this range is empty and using it to index a slice will always yield an empty slice
--> $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
error: aborting due to 4 previous errors

5
tests/ui/reversed_empty_ranges_unfixable.rs
View File

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

20
tests/ui/reversed_empty_ranges_unfixable.stderr
View File

@ -1,28 +1,22 @@
error: this range is empty so it will yield no values
--> $DIR/reversed_empty_ranges_unfixable.rs:7:13
error: this range is reversed and using it to index a slice will panic at run-time
--> $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`
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
|
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
--> $DIR/reversed_empty_ranges_unfixable.rs:9:18
|
LL | let _ = &arr[SOME_NUM..1];
| ^^^^^^^^^^^
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 {}
| ^^^^^^^^^^^^^^
error: aborting due to 4 previous errors
error: aborting due to 3 previous errors