diff options
| author | bors <bors@rust-lang.org> | 2023-12-01 19:02:59 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-12-01 19:02:59 +0000 |
| commit | ee8376075d8fcfa0bacc316e0417981cd458044c (patch) | |
| tree | 6bded40173f127c15fd8c97d525eaa2bb0388012 | |
| parent | 5ac76ac54f5616f8d49c2faea81c6b734a316bce (diff) | |
| parent | 4de845e375c7679b919f624227a8004476e2c153 (diff) | |
| download | rust-ee8376075d8fcfa0bacc316e0417981cd458044c.tar.gz rust-ee8376075d8fcfa0bacc316e0417981cd458044c.zip | |
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
| -rw-r--r-- | clippy_lints/src/missing_asserts_for_indexing.rs | 21 | ||||
| -rw-r--r-- | tests/ui/missing_asserts_for_indexing.fixed | 15 | ||||
| -rw-r--r-- | tests/ui/missing_asserts_for_indexing.rs | 15 | ||||
| -rw-r--r-- | tests/ui/missing_asserts_for_indexing.stderr | 54 |
4 files changed, 99 insertions, 6 deletions
diff --git a/clippy_lints/src/missing_asserts_for_indexing.rs b/clippy_lints/src/missing_asserts_for_indexing.rs index 8f2a5390781..71470ea2dc6 100644 --- a/clippy_lints/src/missing_asserts_for_indexing.rs +++ b/clippy_lints/src/missing_asserts_for_indexing.rs @@ -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] /// } @@ -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. @@ -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, } } @@ -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, @@ -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, }; @@ -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, @@ -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 => { diff --git a/tests/ui/missing_asserts_for_indexing.fixed b/tests/ui/missing_asserts_for_indexing.fixed index a96827259f5..ac44a6f3fdb 100644 --- a/tests/ui/missing_asserts_for_indexing.fixed +++ b/tests/ui/missing_asserts_for_indexing.fixed @@ -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() {} diff --git a/tests/ui/missing_asserts_for_indexing.rs b/tests/ui/missing_asserts_for_indexing.rs index 0b4b883acf8..f05d5fea57d 100644 --- a/tests/ui/missing_asserts_for_indexing.rs +++ b/tests/ui/missing_asserts_for_indexing.rs @@ -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() {} diff --git a/tests/ui/missing_asserts_for_indexing.stderr b/tests/ui/missing_asserts_for_indexing.stderr index a3e66d7958e..61dce6ccc6c 100644 --- a/tests/ui/missing_asserts_for_indexing.stderr +++ b/tests/ui/missing_asserts_for_indexing.stderr @@ -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 |
