diff options
| author | Manish Goregaokar <manishsmail@gmail.com> | 2025-04-15 18:11:36 +0000 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-04-15 18:11:36 +0000 |
| commit | 459897bab1814f602c98cf24b79bfb86062615a0 (patch) | |
| tree | 63f8dc5e04b70077daf39c5e5dddf70939e150fd | |
| parent | d3267e9230940757fde2fcb608605bf8dbfd85e1 (diff) | |
| parent | d1c315a288bf8c4783990ed3af817d91b4b7dacf (diff) | |
| download | rust-459897bab1814f602c98cf24b79bfb86062615a0.tar.gz rust-459897bab1814f602c98cf24b79bfb86062615a0.zip | |
`missing_asserts_for_indexing`: consider `assert_eq!()` as well (#14258)
`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. Fix rust-lang/rust-clippy#14255 changelog: [`missing_asserts_for_indexing`]: consider `assert_eq!()` as well
| -rw-r--r-- | clippy_lints/src/missing_asserts_for_indexing.rs | 39 | ||||
| -rw-r--r-- | tests/ui/missing_asserts_for_indexing.fixed | 17 | ||||
| -rw-r--r-- | tests/ui/missing_asserts_for_indexing.rs | 17 | ||||
| -rw-r--r-- | tests/ui/missing_asserts_for_indexing.stderr | 54 | ||||
| -rw-r--r-- | tests/ui/missing_asserts_for_indexing_unfixable.rs | 7 | ||||
| -rw-r--r-- | tests/ui/missing_asserts_for_indexing_unfixable.stderr | 26 |
6 files changed, 145 insertions, 15 deletions
diff --git a/clippy_lints/src/missing_asserts_for_indexing.rs b/clippy_lints/src/missing_asserts_for_indexing.rs index d9acf655b8a..c8e3462b24e 100644 --- a/clippy_lints/src/missing_asserts_for_indexing.rs +++ b/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,19 +135,31 @@ 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 { None @@ -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, }); } diff --git a/tests/ui/missing_asserts_for_indexing.fixed b/tests/ui/missing_asserts_for_indexing.fixed index 6e803322f65..9018f38100e 100644 --- a/tests/ui/missing_asserts_for_indexing.fixed +++ b/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() {} diff --git a/tests/ui/missing_asserts_for_indexing.rs b/tests/ui/missing_asserts_for_indexing.rs index 4614a8ef5d0..44c5eddf3d8 100644 --- a/tests/ui/missing_asserts_for_indexing.rs +++ b/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() {} diff --git a/tests/ui/missing_asserts_for_indexing.stderr b/tests/ui/missing_asserts_for_indexing.stderr index 5d30920ccf5..b610de94b53 100644 --- a/tests/ui/missing_asserts_for_indexing.stderr +++ b/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 diff --git a/tests/ui/missing_asserts_for_indexing_unfixable.rs b/tests/ui/missing_asserts_for_indexing_unfixable.rs index 2fac9d7a59c..eb98969efa4 100644 --- a/tests/ui/missing_asserts_for_indexing_unfixable.rs +++ b/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() {} diff --git a/tests/ui/missing_asserts_for_indexing_unfixable.stderr b/tests/ui/missing_asserts_for_indexing_unfixable.stderr index 1674861c9ed..a17ad023213 100644 --- a/tests/ui/missing_asserts_for_indexing_unfixable.stderr +++ b/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 |
