Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions clippy_lints/src/missing_asserts_for_indexing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ declare_clippy_lint! {
/// Use instead:
/// ```no_run
/// fn sum(v: &[u8]) -> u8 {
/// assert!(v.len() > 4);
/// assert!(v.len() > 3);
/// // no bounds checks
/// v[0] + v[1] + v[2] + v[3]
/// }
Expand Down Expand Up @@ -87,6 +87,9 @@ enum LengthComparison {
LengthLessThanOrEqualInt,
/// `5 <= v.len()`
IntLessThanOrEqualLength,
/// `5 == v.len()`
/// `v.len() == 5`
LengthEqualInt,
}

/// Extracts parts out of a length comparison expression.
Expand Down Expand Up @@ -114,6 +117,8 @@ fn len_comparison<'hir>(
(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!(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,
}
}
Expand Down Expand Up @@ -316,11 +321,11 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnhashMap<u64, Vec<IndexEntry<'_>>
continue;
};

match entry {
match *entry {
IndexEntry::AssertWithIndex {
highest_index,
asserted_len,
indexes,
ref indexes,
comparison,
assert_span,
slice,
Expand All @@ -343,6 +348,12 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnhashMap<u64, Vec<IndexEntry<'_>>
"assert!({}.len() > {highest_index})",
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,
};

Expand All @@ -354,7 +365,7 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnhashMap<u64, Vec<IndexEntry<'_>>
indexes,
|diag| {
diag.span_suggestion(
*assert_span,
assert_span,
"provide the highest index that is indexed with",
sugg,
Applicability::MachineApplicable,
Expand All @@ -364,7 +375,7 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnhashMap<u64, Vec<IndexEntry<'_>>
}
},
IndexEntry::IndexWithoutAssert {
indexes,
ref indexes,
highest_index,
slice,
} if indexes.len() > 1 => {
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/missing_asserts_for_indexing.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,19 @@ fn index_different_slice_in_same_expr(v1: &[u8], v2: &[u8]) {
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() {}
15 changes: 15 additions & 0 deletions tests/ui/missing_asserts_for_indexing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,19 @@ fn index_different_slice_in_same_expr(v1: &[u8], v2: &[u8]) {
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() {}
54 changes: 53 additions & 1 deletion tests/ui/missing_asserts_for_indexing.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -249,5 +249,57 @@ LL | let _ = v1[0] + v1[12];
| ^^^^^^
= 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