Auto merge of #11837 - y21:issue11835, r=dswij

[`missing_asserts_for_indexing`]: accept length equality checks

Fixes #11835

The lint now allows indexing with indices 0 and 1 when an `assert!(x.len() == 2);` is found.
(Also fixed a typo in the doc example)

changelog: [`missing_asserts_for_indexing`]: accept len equality checks as a valid assertion
This commit is contained in:
bors 2023-12-01 19:02:59 +00:00
commit ee8376075d
4 changed files with 99 additions and 6 deletions

View file

@ -52,7 +52,7 @@ declare_clippy_lint! {
/// Use instead: /// Use instead:
/// ```no_run /// ```no_run
/// fn sum(v: &[u8]) -> u8 { /// fn sum(v: &[u8]) -> u8 {
/// assert!(v.len() > 4); /// assert!(v.len() > 3);
/// // no bounds checks /// // no bounds checks
/// v[0] + v[1] + v[2] + v[3] /// v[0] + v[1] + v[2] + v[3]
/// } /// }
@ -87,6 +87,9 @@ enum LengthComparison {
LengthLessThanOrEqualInt, LengthLessThanOrEqualInt,
/// `5 <= v.len()` /// `5 <= v.len()`
IntLessThanOrEqualLength, IntLessThanOrEqualLength,
/// `5 == v.len()`
/// `v.len() == 5`
LengthEqualInt,
} }
/// Extracts parts out of a length comparison expression. /// Extracts parts out of a length comparison expression.
@ -114,6 +117,8 @@ fn len_comparison<'hir>(
(Rel::Lt, _, int_lit_pat!(right)) => Some((LengthComparison::LengthLessThanInt, *right as usize, left)), (Rel::Lt, _, int_lit_pat!(right)) => Some((LengthComparison::LengthLessThanInt, *right as usize, left)),
(Rel::Le, int_lit_pat!(left), _) => Some((LengthComparison::IntLessThanOrEqualLength, *left as usize, right)), (Rel::Le, int_lit_pat!(left), _) => Some((LengthComparison::IntLessThanOrEqualLength, *left as usize, right)),
(Rel::Le, _, int_lit_pat!(right)) => Some((LengthComparison::LengthLessThanOrEqualInt, *right as usize, left)), (Rel::Le, _, int_lit_pat!(right)) => Some((LengthComparison::LengthLessThanOrEqualInt, *right as usize, left)),
(Rel::Eq, int_lit_pat!(left), _) => Some((LengthComparison::LengthEqualInt, *left as usize, right)),
(Rel::Eq, _, int_lit_pat!(right)) => Some((LengthComparison::LengthEqualInt, *right as usize, left)),
_ => None, _ => None,
} }
} }
@ -316,11 +321,11 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnhashMap<u64, Vec<IndexEntry<'_>>
continue; continue;
}; };
match entry { match *entry {
IndexEntry::AssertWithIndex { IndexEntry::AssertWithIndex {
highest_index, highest_index,
asserted_len, asserted_len,
indexes, ref indexes,
comparison, comparison,
assert_span, assert_span,
slice, slice,
@ -343,6 +348,12 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnhashMap<u64, Vec<IndexEntry<'_>>
"assert!({}.len() > {highest_index})", "assert!({}.len() > {highest_index})",
snippet(cx, slice.span, "..") snippet(cx, slice.span, "..")
)), )),
// `highest_index` here is rather a length, so we need to add 1 to it
LengthComparison::LengthEqualInt if asserted_len < highest_index + 1 => Some(format!(
"assert!({}.len() == {})",
snippet(cx, slice.span, ".."),
highest_index + 1
)),
_ => None, _ => None,
}; };
@ -354,7 +365,7 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnhashMap<u64, Vec<IndexEntry<'_>>
indexes, indexes,
|diag| { |diag| {
diag.span_suggestion( diag.span_suggestion(
*assert_span, assert_span,
"provide the highest index that is indexed with", "provide the highest index that is indexed with",
sugg, sugg,
Applicability::MachineApplicable, Applicability::MachineApplicable,
@ -364,7 +375,7 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnhashMap<u64, Vec<IndexEntry<'_>>
} }
}, },
IndexEntry::IndexWithoutAssert { IndexEntry::IndexWithoutAssert {
indexes, ref indexes,
highest_index, highest_index,
slice, slice,
} if indexes.len() > 1 => { } if indexes.len() > 1 => {

View file

@ -118,4 +118,19 @@ fn index_different_slice_in_same_expr(v1: &[u8], v2: &[u8]) {
let _ = v1[0] + v2[1]; let _ = v1[0] + v2[1];
} }
fn issue11835(v1: &[u8], v2: &[u8], v3: &[u8], v4: &[u8]) {
assert!(v1.len() == 3);
assert!(v2.len() == 4);
assert!(v3.len() == 3);
assert!(4 == v4.len());
let _ = v1[0] + v1[1] + v1[2];
//~^ ERROR: indexing into a slice multiple times with an `assert` that does not cover the
let _ = v2[0] + v2[1] + v2[2];
let _ = v3[0] + v3[1] + v3[2];
//~^ ERROR: indexing into a slice multiple times with an `assert` that does not cover the
let _ = v4[0] + v4[1] + v4[2];
}
fn main() {} fn main() {}

View file

@ -118,4 +118,19 @@ fn index_different_slice_in_same_expr(v1: &[u8], v2: &[u8]) {
let _ = v1[0] + v2[1]; let _ = v1[0] + v2[1];
} }
fn issue11835(v1: &[u8], v2: &[u8], v3: &[u8], v4: &[u8]) {
assert!(v1.len() == 2);
assert!(v2.len() == 4);
assert!(2 == v3.len());
assert!(4 == v4.len());
let _ = v1[0] + v1[1] + v1[2];
//~^ ERROR: indexing into a slice multiple times with an `assert` that does not cover the
let _ = v2[0] + v2[1] + v2[2];
let _ = v3[0] + v3[1] + v3[2];
//~^ ERROR: indexing into a slice multiple times with an `assert` that does not cover the
let _ = v4[0] + v4[1] + v4[2];
}
fn main() {} fn main() {}

View file

@ -249,5 +249,57 @@ LL | let _ = v1[0] + v1[12];
| ^^^^^^ | ^^^^^^
= note: asserting the length before indexing will elide bounds checks = note: asserting the length before indexing will elide bounds checks
error: aborting due to 9 previous errors error: indexing into a slice multiple times with an `assert` that does not cover the highest index
--> $DIR/missing_asserts_for_indexing.rs:127:13
|
LL | assert!(v1.len() == 2);
| ---------------------- help: provide the highest index that is indexed with: `assert!(v1.len() == 3)`
...
LL | let _ = v1[0] + v1[1] + v1[2];
| ^^^^^^^^^^^^^^^^^^^^^
|
note: slice indexed here
--> $DIR/missing_asserts_for_indexing.rs:127:13
|
LL | let _ = v1[0] + v1[1] + v1[2];
| ^^^^^
note: slice indexed here
--> $DIR/missing_asserts_for_indexing.rs:127:21
|
LL | let _ = v1[0] + v1[1] + v1[2];
| ^^^^^
note: slice indexed here
--> $DIR/missing_asserts_for_indexing.rs:127:29
|
LL | let _ = v1[0] + v1[1] + v1[2];
| ^^^^^
= note: asserting the length before indexing will elide bounds checks
error: indexing into a slice multiple times with an `assert` that does not cover the highest index
--> $DIR/missing_asserts_for_indexing.rs:131:13
|
LL | assert!(2 == v3.len());
| ---------------------- help: provide the highest index that is indexed with: `assert!(v3.len() == 3)`
...
LL | let _ = v3[0] + v3[1] + v3[2];
| ^^^^^^^^^^^^^^^^^^^^^
|
note: slice indexed here
--> $DIR/missing_asserts_for_indexing.rs:131:13
|
LL | let _ = v3[0] + v3[1] + v3[2];
| ^^^^^
note: slice indexed here
--> $DIR/missing_asserts_for_indexing.rs:131:21
|
LL | let _ = v3[0] + v3[1] + v3[2];
| ^^^^^
note: slice indexed here
--> $DIR/missing_asserts_for_indexing.rs:131:29
|
LL | let _ = v3[0] + v3[1] + v3[2];
| ^^^^^
= note: asserting the length before indexing will elide bounds checks
error: aborting due to 11 previous errors