Fixes for missing_asserts_for_indexing (#14108)

This PR fixes issues with the `missing_asserts_for_indexing` lint.
- false positive when the first index is the highest(or equal) value in
a list of indexes:
```rust
pub fn foo(slice: &[i32]) -> i32{
	slice[1] * slice[0]
}
```
- false negative when an assert statement if found after the indexing
operation.
```rust
pub fn bar(slice: &[i32]) -> i32 {
	let product = slice[0] * slice[1];
	assert!(slice.len() > 1);
	product
}
```

examples: https://godbolt.org/z/s7Y47vKdE

closes: #14079

changelog: [`missing_asserts_for_indexing`]: ignore asserts found after
indexing, and do not require asserts if the first index is highest.
This commit is contained in:
Samuel Tardieu 2025-04-06 10:42:29 +00:00 committed by GitHub
commit cf9cffa114
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 78 additions and 13 deletions

View file

@ -168,6 +168,7 @@ enum IndexEntry<'hir> {
/// if the `assert!` asserts the right length.
AssertWithIndex {
highest_index: usize,
is_first_highest: bool,
asserted_len: usize,
assert_span: Span,
slice: &'hir Expr<'hir>,
@ -177,6 +178,7 @@ enum IndexEntry<'hir> {
/// Indexing without an `assert!`
IndexWithoutAssert {
highest_index: usize,
is_first_highest: bool,
indexes: Vec<Span>,
slice: &'hir Expr<'hir>,
},
@ -244,28 +246,41 @@ fn check_index<'hir>(cx: &LateContext<'_>, expr: &'hir Expr<'hir>, map: &mut Uni
assert_span,
slice,
} => {
*entry = IndexEntry::AssertWithIndex {
highest_index: index,
asserted_len: *asserted_len,
assert_span: *assert_span,
slice,
indexes: vec![expr.span],
comparison: *comparison,
};
if slice.span.lo() > assert_span.lo() {
*entry = IndexEntry::AssertWithIndex {
highest_index: index,
is_first_highest: true,
asserted_len: *asserted_len,
assert_span: *assert_span,
slice,
indexes: vec![expr.span],
comparison: *comparison,
};
}
},
IndexEntry::IndexWithoutAssert {
highest_index, indexes, ..
highest_index,
indexes,
is_first_highest,
..
}
| IndexEntry::AssertWithIndex {
highest_index, indexes, ..
highest_index,
indexes,
is_first_highest,
..
} => {
indexes.push(expr.span);
if *is_first_highest {
(*is_first_highest) = *highest_index >= index;
}
*highest_index = (*highest_index).max(index);
},
}
} else {
indexes.push(IndexEntry::IndexWithoutAssert {
highest_index: index,
is_first_highest: true,
indexes: vec![expr.span],
slice,
});
@ -284,13 +299,16 @@ fn check_assert<'hir>(cx: &LateContext<'_>, expr: &'hir Expr<'hir>, map: &mut Un
if let Some(entry) = entry {
if let IndexEntry::IndexWithoutAssert {
highest_index,
is_first_highest,
indexes,
slice,
} = entry
&& expr.span.lo() <= slice.span.lo()
{
*entry = IndexEntry::AssertWithIndex {
highest_index: *highest_index,
indexes: mem::take(indexes),
is_first_highest: *is_first_highest,
slice,
assert_span: expr.span,
comparison,
@ -325,12 +343,13 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnindexMap<u64, Vec<IndexEntry<'_>
match *entry {
IndexEntry::AssertWithIndex {
highest_index,
is_first_highest,
asserted_len,
ref indexes,
comparison,
assert_span,
slice,
} if indexes.len() > 1 => {
} if indexes.len() > 1 && !is_first_highest => {
// if we have found an `assert!`, let's also check that it's actually right
// and if it covers the highest index and if not, suggest the correct length
let sugg = match comparison {
@ -378,8 +397,9 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnindexMap<u64, Vec<IndexEntry<'_>
IndexEntry::IndexWithoutAssert {
ref indexes,
highest_index,
is_first_highest,
slice,
} if indexes.len() > 1 => {
} if indexes.len() > 1 && !is_first_highest => {
// if there was no `assert!` but more than one index, suggest
// adding an `assert!` that covers the highest index
report_lint(

View file

@ -139,4 +139,14 @@ fn issue11835(v1: &[u8], v2: &[u8], v3: &[u8], v4: &[u8]) {
let _ = v4[0] + v4[1] + v4[2];
}
// ok
fn same_index_multiple_times(v1: &[u8]) {
let _ = v1[0] + v1[0];
}
// ok
fn highest_index_first(v1: &[u8]) {
let _ = v1[2] + v1[1] + v1[0];
}
fn main() {}

View file

@ -139,4 +139,14 @@ fn issue11835(v1: &[u8], v2: &[u8], v3: &[u8], v4: &[u8]) {
let _ = v4[0] + v4[1] + v4[2];
}
// ok
fn same_index_multiple_times(v1: &[u8]) {
let _ = v1[0] + v1[0];
}
// ok
fn highest_index_first(v1: &[u8]) {
let _ = v1[2] + v1[1] + v1[0];
}
fn main() {}

View file

@ -73,4 +73,10 @@ pub fn issue11856(values: &[i32]) -> usize {
ascending.len()
}
fn assert_after_indexing(v1: &[u8]) {
let _ = v1[1] + v1[2];
//~^ ERROR: indexing into a slice multiple times without an `assert`
assert!(v1.len() > 2);
}
fn main() {}

View file

@ -180,5 +180,24 @@ LL | let _ = x[0] + x[1];
| ^^^^
= note: asserting the length before indexing will elide bounds checks
error: aborting due to 8 previous errors
error: indexing into a slice multiple times without an `assert`
--> tests/ui/missing_asserts_for_indexing_unfixable.rs:77:13
|
LL | let _ = v1[1] + v1[2];
| ^^^^^^^^^^^^^
|
= help: consider asserting the length before indexing: `assert!(v1.len() > 2);`
note: slice indexed here
--> tests/ui/missing_asserts_for_indexing_unfixable.rs:77:13
|
LL | let _ = v1[1] + v1[2];
| ^^^^^
note: slice indexed here
--> tests/ui/missing_asserts_for_indexing_unfixable.rs:77:21
|
LL | let _ = v1[1] + v1[2];
| ^^^^^
= note: asserting the length before indexing will elide bounds checks
error: aborting due to 9 previous errors