add some comments

Author: y21

clippy_lints/src/missing_assert_for_indexing.rs

@@ -91,6 +91,9 @@ enum LengthComparison {
     IntLessThanOrEqualLength,
 }
 
+/// Extracts parts out of a length comparison expression.
+///
+/// E.g. for `v.len() > 5` this returns `Some((LengthComparison::IntLessThanLength, 5, `v.len()`))`
 fn len_comparison<'hir>(
     bin_op: BinOp,
     left: &'hir Expr<'hir>,
@@ -105,7 +108,7 @@ fn len_comparison<'hir>(
         };
     }
 
-    // normalize comparison, `v.len() > 4` becomes `4 <= v.len()`
+    // normalize comparison, `v.len() > 4` becomes `4 < v.len()`
     // this simplifies the logic a bit
     let (op, left, right) = normalize_comparison(bin_op.node, left, right)?;
     match (op, &left.kind, &right.kind) {
@@ -117,6 +120,8 @@ fn len_comparison<'hir>(
     }
 }
 
+/// Attempts to extract parts out of an `assert!`-like expression
+/// in the form `assert!(some_slice.len() > 5)`.
 fn assert_len_expr<'hir>(
     cx: &LateContext<'_>,
     expr: &'hir Expr<'hir>,
@@ -142,12 +147,17 @@ fn assert_len_expr<'hir>(
 
 #[derive(Debug)]
 enum IndexEntry {
+    /// `assert!` without any indexing (so far)
     StrayAssert {
         asserted_len: usize,
         cmp: LengthComparison,
         assert_span: Span,
         slice_span: Span,
     },
+    /// `assert!` with indexing
+    ///
+    /// We also store the highest index to be able to check
+    /// if the `assert!` asserts the right length.
     AssertWithIndex {
         highest_index: usize,
         asserted_len: usize,
@@ -156,7 +166,8 @@ enum IndexEntry {
         indexes: Vec<Span>,
         cmp: LengthComparison,
     },
-    AssertWithoutIndex {
+    /// Indexing without an `assert!`
+    IndexWithoutAssert {
         highest_index: usize,
         indexes: Vec<Span>,
         slice_span: Span,
@@ -167,13 +178,17 @@ impl IndexEntry {
     pub fn spans(&self) -> Option<&[Span]> {
         match self {
             IndexEntry::StrayAssert { .. } => None,
-            IndexEntry::AssertWithIndex { indexes, .. } | IndexEntry::AssertWithoutIndex { indexes, .. } => {
+            IndexEntry::AssertWithIndex { indexes, .. } | IndexEntry::IndexWithoutAssert { indexes, .. } => {
                 Some(indexes)
             },
         }
     }
 }
 
+/// Extracts the upper index of a slice indexing expression.
+///
+/// E.g. for `5` this returns `Some(5)`, for `..5` this returns `Some(4)`,
+/// for `..=5` this returns `Some(5)`
 fn upper_index_expr(expr: &Expr<'_>) -> Option<usize> {
     if let ExprKind::Lit(lit) = &expr.kind && let LitKind::Int(index, _) = lit.node {
         Some(index as usize)
@@ -190,35 +205,36 @@ fn upper_index_expr(expr: &Expr<'_>) -> Option<usize> {
     }
 }
 
+/// Checks if the expression is an index into a slice and adds it to the `indexes` map
 fn check_index(cx: &LateContext<'_>, expr: &Expr<'_>, indexes: &mut FxHashMap<u64, IndexEntry>) {
     if let ExprKind::Index(target, index_lit) = expr.kind
         && cx.typeck_results().expr_ty(target).peel_refs().is_slice()
         && let Some(index) = upper_index_expr(index_lit)
     {
         let hash = hash_expr(cx, target);
         let entry = indexes.entry(hash)
-            .or_insert_with(|| IndexEntry::AssertWithoutIndex {
+            .or_insert_with(|| IndexEntry::IndexWithoutAssert {
                 highest_index: index,
                 slice_span: target.span,
                 indexes: Vec::new(),
             });
 
         match entry {
-            IndexEntry::StrayAssert { asserted_len, cmp, assert_span, slice_span: receiver_span } => {
+            IndexEntry::StrayAssert { asserted_len, cmp, assert_span, slice_span } => {
                 let asserted_len = *asserted_len;
                 let cmp = *cmp;
                 let assert_span = *assert_span;
-                let receiver_span = *receiver_span;
+                let slice_span = *slice_span;
                 *entry = IndexEntry::AssertWithIndex {
                     highest_index: index,
                     asserted_len,
                     indexes: vec![expr.span],
                     assert_span,
                     cmp,
-                    slice_span: receiver_span
+                    slice_span
                 };
             }
-            IndexEntry::AssertWithoutIndex { highest_index, indexes, .. }
+            IndexEntry::IndexWithoutAssert { highest_index, indexes, .. }
             | IndexEntry::AssertWithIndex { highest_index, indexes, .. } => {
                 indexes.push(expr.span);
                 *highest_index = (*highest_index).max(index);
@@ -227,6 +243,7 @@ fn check_index(cx: &LateContext<'_>, expr: &Expr<'_>, indexes: &mut FxHashMap<u6
     }
 }
 
+/// Checks if the expression is an `assert!` expression and adds it to the `indexes` map
 fn check_assert(cx: &LateContext<'_>, expr: &Expr<'_>, indexes: &mut FxHashMap<u64, IndexEntry>) {
     if let Some((cmp, asserted_len, slice)) = assert_len_expr(cx, expr) {
         let hash = hash_expr(cx, slice);
@@ -240,7 +257,7 @@ fn check_assert(cx: &LateContext<'_>, expr: &Expr<'_>, indexes: &mut FxHashMap<u
                 });
             },
             Entry::Occupied(mut entry) => {
-                if let IndexEntry::AssertWithoutIndex {
+                if let IndexEntry::IndexWithoutAssert {
                     highest_index, indexes, ..
                 } = entry.get_mut()
                 {
@@ -261,6 +278,9 @@ fn check_assert(cx: &LateContext<'_>, expr: &Expr<'_>, indexes: &mut FxHashMap<u
     }
 }
 
+/// Inspects indexes and reports lints.
+///
+/// Called at the end of this lint after all indexing and `assert!` expressions have been collected.
 fn report_indexes(cx: &LateContext<'_>, indexes: &mut FxHashMap<u64, IndexEntry>) {
     for entry in indexes.values() {
         let Some(full_span) = entry
@@ -280,7 +300,7 @@ fn report_indexes(cx: &LateContext<'_>, indexes: &mut FxHashMap<u64, IndexEntry>
                 slice_span: receiver_span,
             } if indexes.len() > 1 => {
                 // if we have found an `assert!`, let's also check that it's actually right
-                // and convers the highest index and if not, suggest the correct length
+                // and if it convers the highest index and if not, suggest the correct length
                 let sugg = match cmp {
                     // `v.len() < 5` and `v.len() <= 5` does nothing in terms of bounds checks.
                     // The user probably meant `v.len() > 5`
@@ -318,11 +338,13 @@ fn report_indexes(cx: &LateContext<'_>, indexes: &mut FxHashMap<u64, IndexEntry>
                     );
                 }
             },
-            IndexEntry::AssertWithoutIndex {
+            IndexEntry::IndexWithoutAssert {
                 indexes,
                 highest_index,
                 slice_span: receiver_span,
             } if indexes.len() > 1 => {
+                // if there was no `assert!` but more than one index, suggest
+                // adding an `assert!` that covers the highest index
                 report_lint(
                     cx,
                     full_span,