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
rust-lang · Dec 1, 2023 · ee83760 · ee83760
2 parents 5ac76ac + 4de845e
commit ee83760
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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