Auto merge of #5638 - ebroto:issue_5628_add_suggestion_for_reversed_empty_ranges, r=phansch

reversed_empty_ranges: add suggestion for &slice[N..N]

As discussed in the issue thread, the user accepted this solution. Let me know if this is what we want, or if changing the way we lint the N..N case is prefered.

changelog: reversed_empty_ranges: add suggestion for &slice[N..N]

Closes #5628
This commit is contained in:
bors 2020-05-26 11:56:16 +00:00
commit a00025ab21
6 changed files with 47 additions and 24 deletions

View File

@ -241,14 +241,14 @@ fn check_inclusive_range_minus_one(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
}
fn check_reversed_empty_range(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
fn inside_indexing_expr(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
matches!(
get_parent_expr(cx, expr),
Some(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 {
kind: ExprKind::Index(..),
..
})
)
}) => parent_expr,
_ => None,
}
}
fn is_empty_range(limits: RangeLimits, ordering: Ordering) -> bool {
@ -267,18 +267,32 @@ fn check_reversed_empty_range(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
if let Some(ordering) = Constant::partial_cmp(cx.tcx, ty, &start_idx, &end_idx);
if is_empty_range(limits, ordering);
then {
if inside_indexing_expr(cx, expr) {
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(
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 {
span_lint_and_then(

View File

@ -4,18 +4,23 @@
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));
let _ = (21..ANSWER).rev().filter(|x| x % 2 == 0).take(10).collect::<Vec<_>>();
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];

View File

@ -4,18 +4,23 @@
const ANSWER: i32 = 42;
fn main() {
let arr = [1, 2, 3, 4, 5];
// These should be linted:
(42..=21).for_each(|x| println!("{}", x));
let _ = (ANSWER..21).filter(|x| x % 2 == 0).take(10).collect::<Vec<_>>();
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];

View File

@ -1,5 +1,5 @@
error: this range is empty so it will yield no values
--> $DIR/reversed_empty_ranges_fixable.rs:7:5
--> $DIR/reversed_empty_ranges_fixable.rs:11: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:8:13
--> $DIR/reversed_empty_ranges_fixable.rs:12: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:10:14
--> $DIR/reversed_empty_ranges_fixable.rs:14: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:11:14
--> $DIR/reversed_empty_ranges_fixable.rs:15:14
|
LL | for _ in 42u32..21u32 {}
| ^^^^^^^^^^^^
@ -43,5 +43,11 @@ help: consider using the following if you are attempting to iterate over this ra
LL | for _ in (21u32..42u32).rev() {}
| ^^^^^^^^^^^^^^^^^^^^
error: aborting due to 4 previous errors
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

View File

@ -9,7 +9,6 @@ fn main() {
let arr = [1, 2, 3, 4, 5];
let _ = &arr[3usize..=1usize];
let _ = &arr[SOME_NUM..1];
let _ = &arr[3..3];
for _ in ANSWER..ANSWER {}
}

View File

@ -18,17 +18,11 @@ error: this range is reversed and using it to index a slice will panic at run-ti
LL | let _ = &arr[SOME_NUM..1];
| ^^^^^^^^^^^
error: this range is empty and using it to index a slice will always yield an empty slice
--> $DIR/reversed_empty_ranges_unfixable.rs:12:18
|
LL | let _ = &arr[3..3];
| ^^^^
error: this range is empty so it will yield no values
--> $DIR/reversed_empty_ranges_unfixable.rs:14:14
--> $DIR/reversed_empty_ranges_unfixable.rs:13:14
|
LL | for _ in ANSWER..ANSWER {}
| ^^^^^^^^^^^^^^
error: aborting due to 5 previous errors
error: aborting due to 4 previous errors