missing_asserts_for_indexing: consider assert_eq!() as well

`assert_eq!()` and `assert_ne!()` are not expanded the same way as
`assert!()` (they use a `match` instead of a `if`). This makes them
being recognized as well.

Author: samueltardieu

clippy_lints/src/missing_asserts_for_indexing.rs

@@ -3,14 +3,15 @@ use std::ops::ControlFlow;
 
 use clippy_utils::comparisons::{Rel, normalize_comparison};
 use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::macros::{find_assert_eq_args, first_node_macro_backtrace};
 use clippy_utils::source::snippet;
 use clippy_utils::visitors::for_each_expr_without_closures;
 use clippy_utils::{eq_expr_value, hash_expr, higher};
-use rustc_ast::{LitKind, RangeLimits};
+use rustc_ast::{BinOpKind, LitKind, RangeLimits};
 use rustc_data_structures::packed::Pu128;
 use rustc_data_structures::unhash::UnindexMap;
 use rustc_errors::{Applicability, Diag};
-use rustc_hir::{BinOp, Block, Body, Expr, ExprKind, UnOp};
+use rustc_hir::{Block, Body, Expr, ExprKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
 use rustc_span::source_map::Spanned;
@@ -97,7 +98,7 @@ enum LengthComparison {
 ///
 /// E.g. for `v.len() > 5` this returns `Some((LengthComparison::IntLessThanLength, 5, v.len()))`
 fn len_comparison<'hir>(
-    bin_op: BinOp,
+    bin_op: BinOpKind,
     left: &'hir Expr<'hir>,
     right: &'hir Expr<'hir>,
 ) -> Option<(LengthComparison, usize, &'hir Expr<'hir>)> {
@@ -112,7 +113,7 @@ fn len_comparison<'hir>(
 
     // 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)?;
+    let (op, left, right) = normalize_comparison(bin_op, left, right)?;
     match (op, left.kind, right.kind) {
         (Rel::Lt, int_lit_pat!(left), _) => Some((LengthComparison::IntLessThanLength, left as usize, right)),
         (Rel::Lt, _, int_lit_pat!(right)) => Some((LengthComparison::LengthLessThanInt, right as usize, left)),
@@ -134,18 +135,30 @@ fn assert_len_expr<'hir>(
     cx: &LateContext<'_>,
     expr: &'hir Expr<'hir>,
 ) -> Option<(LengthComparison, usize, &'hir Expr<'hir>)> {
-    if let Some(higher::If { cond, then, .. }) = higher::If::hir(expr)
+    let (cmp, asserted_len, slice_len) = if let Some(higher::If { cond, then, .. }) = higher::If::hir(expr)
         && let ExprKind::Unary(UnOp::Not, condition) = &cond.kind
         && let ExprKind::Binary(bin_op, left, right) = &condition.kind
-
-        && let Some((cmp, asserted_len, slice_len)) = len_comparison(*bin_op, left, right)
-        && let ExprKind::MethodCall(method, recv, [], _) = &slice_len.kind
-        && cx.typeck_results().expr_ty_adjusted(recv).peel_refs().is_slice()
-        && method.ident.name == sym::len
-
         // check if `then` block has a never type expression
         && let ExprKind::Block(Block { expr: Some(then_expr), .. }, _) = then.kind
         && cx.typeck_results().expr_ty(then_expr).is_never()
+    {
+        len_comparison(bin_op.node, left, right)?
+    } else if let Some((macro_call, bin_op)) = first_node_macro_backtrace(cx, expr).find_map(|macro_call| {
+        match cx.tcx.get_diagnostic_name(macro_call.def_id) {
+            Some(sym::assert_eq_macro) => Some((macro_call, BinOpKind::Eq)),
+            Some(sym::assert_ne_macro) => Some((macro_call, BinOpKind::Ne)),
+            _ => None,
+        }
+    }) && let Some((left, right, _)) = find_assert_eq_args(cx, expr, macro_call.expn)
+    {
+        len_comparison(bin_op, left, right)?
+    } else {
+        return None;
+    };
+
+    if let ExprKind::MethodCall(method, recv, [], _) = &slice_len.kind
+        && cx.typeck_results().expr_ty_adjusted(recv).peel_refs().is_slice()
+        && method.ident.name == sym::len
     {
         Some((cmp, asserted_len, recv))
     } else {
@@ -310,7 +323,7 @@ fn check_assert<'hir>(cx: &LateContext<'_>, expr: &'hir Expr<'hir>, map: &mut Un
                     indexes: mem::take(indexes),
                     is_first_highest: *is_first_highest,
                     slice,
-                    assert_span: expr.span,
+                    assert_span: expr.span.source_callsite(),
                     comparison,
                     asserted_len,
                 };
@@ -319,7 +332,7 @@ fn check_assert<'hir>(cx: &LateContext<'_>, expr: &'hir Expr<'hir>, map: &mut Un
             indexes.push(IndexEntry::StrayAssert {
                 asserted_len,
                 comparison,
-                assert_span: expr.span,
+                assert_span: expr.span.source_callsite(),
                 slice,
             });
         }

tests/ui/missing_asserts_for_indexing.fixed

@@ -149,4 +149,21 @@ fn highest_index_first(v1: &[u8]) {
     let _ = v1[2] + v1[1] + v1[0];
 }
 
+fn issue14255(v1: &[u8], v2: &[u8], v3: &[u8], v4: &[u8]) {
+    assert!(v1.len() == 3);
+    assert_eq!(v2.len(), 4);
+    assert!(v3.len() == 3);
+    assert_eq!(4, v4.len());
+
+    let _ = v1[0] + v1[1] + v1[2];
+    //~^ missing_asserts_for_indexing
+
+    let _ = v2[0] + v2[1] + v2[2];
+
+    let _ = v3[0] + v3[1] + v3[2];
+    //~^ missing_asserts_for_indexing
+
+    let _ = v4[0] + v4[1] + v4[2];
+}
+
 fn main() {}

tests/ui/missing_asserts_for_indexing.rs

@@ -149,4 +149,21 @@ fn highest_index_first(v1: &[u8]) {
     let _ = v1[2] + v1[1] + v1[0];
 }
 
+fn issue14255(v1: &[u8], v2: &[u8], v3: &[u8], v4: &[u8]) {
+    assert_eq!(v1.len(), 2);
+    assert_eq!(v2.len(), 4);
+    assert_eq!(2, v3.len());
+    assert_eq!(4, v4.len());
+
+    let _ = v1[0] + v1[1] + v1[2];
+    //~^ missing_asserts_for_indexing
+
+    let _ = v2[0] + v2[1] + v2[2];
+
+    let _ = v3[0] + v3[1] + v3[2];
+    //~^ missing_asserts_for_indexing
+
+    let _ = v4[0] + v4[1] + v4[2];
+}
+
 fn main() {}

tests/ui/missing_asserts_for_indexing.stderr

@@ -301,5 +301,57 @@ 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
+error: indexing into a slice multiple times with an `assert` that does not cover the highest index
+  --> tests/ui/missing_asserts_for_indexing.rs:158:13
+   |
+LL |     assert_eq!(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
+  --> tests/ui/missing_asserts_for_indexing.rs:158:13
+   |
+LL |     let _ = v1[0] + v1[1] + v1[2];
+   |             ^^^^^
+note: slice indexed here
+  --> tests/ui/missing_asserts_for_indexing.rs:158:21
+   |
+LL |     let _ = v1[0] + v1[1] + v1[2];
+   |                     ^^^^^
+note: slice indexed here
+  --> tests/ui/missing_asserts_for_indexing.rs:158: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
+  --> tests/ui/missing_asserts_for_indexing.rs:163:13
+   |
+LL |     assert_eq!(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
+  --> tests/ui/missing_asserts_for_indexing.rs:163:13
+   |
+LL |     let _ = v3[0] + v3[1] + v3[2];
+   |             ^^^^^
+note: slice indexed here
+  --> tests/ui/missing_asserts_for_indexing.rs:163:21
+   |
+LL |     let _ = v3[0] + v3[1] + v3[2];
+   |                     ^^^^^
+note: slice indexed here
+  --> tests/ui/missing_asserts_for_indexing.rs:163:29
+   |
+LL |     let _ = v3[0] + v3[1] + v3[2];
+   |                             ^^^^^
+   = note: asserting the length before indexing will elide bounds checks
+
+error: aborting due to 13 previous errors
 

tests/ui/missing_asserts_for_indexing_unfixable.rs

@@ -79,4 +79,11 @@ fn assert_after_indexing(v1: &[u8]) {
     assert!(v1.len() > 2);
 }
 
+fn issue14255(v1: &[u8]) {
+    assert_ne!(v1.len(), 2);
+
+    let _ = v1[0] + v1[1] + v1[2];
+    //~^ missing_asserts_for_indexing
+}
+
 fn main() {}

tests/ui/missing_asserts_for_indexing_unfixable.stderr

@@ -199,5 +199,29 @@ LL |     let _ = v1[1] + v1[2];
    |                     ^^^^^
    = note: asserting the length before indexing will elide bounds checks
 
-error: aborting due to 9 previous errors
+error: indexing into a slice multiple times without an `assert`
+  --> tests/ui/missing_asserts_for_indexing_unfixable.rs:85:13
+   |
+LL |     let _ = v1[0] + 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:85:13
+   |
+LL |     let _ = v1[0] + v1[1] + v1[2];
+   |             ^^^^^
+note: slice indexed here
+  --> tests/ui/missing_asserts_for_indexing_unfixable.rs:85:21
+   |
+LL |     let _ = v1[0] + v1[1] + v1[2];
+   |                     ^^^^^
+note: slice indexed here
+  --> tests/ui/missing_asserts_for_indexing_unfixable.rs:85:29
+   |
+LL |     let _ = v1[0] + v1[1] + v1[2];
+   |                             ^^^^^
+   = note: asserting the length before indexing will elide bounds checks
+
+error: aborting due to 10 previous errors