Skip to content

Commit 139e905

Browse files
committed
improve sniffing logic
1 parent 541f1f4 commit 139e905

File tree

1 file changed

+40
-23
lines changed

1 file changed

+40
-23
lines changed

clippy_lints/src/missing_asserts_for_indexing.rs

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,11 @@ enum AssertionSide<'hir> {
108108
AssertedLen(EvaluatedIntExpr<'hir>),
109109
}
110110
impl<'hir> AssertionSide<'hir> {
111+
pub fn from_expr(cx: &LateContext<'_>, expr: &'hir Expr<'hir>) -> Option<Self> {
112+
Self::asserted_len_from_int_lit_expr(expr)
113+
.or_else(|| Self::slice_len_from_expr(cx, expr))
114+
.or_else(|| Self::asserted_len_from_possibly_const_expr(cx, expr))
115+
}
111116
pub fn slice_len_from_expr(cx: &LateContext<'_>, expr: &'hir Expr<'hir>) -> Option<Self> {
112117
if let ExprKind::MethodCall(method, recv, [], _) = expr.kind
113118
// checking method name first rather than receiver's type could improve performance
@@ -119,20 +124,22 @@ impl<'hir> AssertionSide<'hir> {
119124
None
120125
}
121126
}
122-
pub fn asserted_len_from_expr(cx: &LateContext<'_>, expr: &'hir Expr<'hir>) -> Option<Self> {
123-
if let ExprKind::Lit(Spanned { node, .. }) = expr.kind {
124-
// cases like `b'A'` or `5.0` would be extremely rare so
125-
// ignore those and consider only integers
126-
if let LitKind::Int(Pu128(x), _) = node {
127-
// integer literals
128-
Some(Self::AssertedLen(EvaluatedIntExpr {
129-
expr,
130-
value: x as usize,
131-
}))
132-
} else {
133-
None
134-
}
135-
} else if let Some(Constant::Int(x)) = ConstEvalCtxt::new(cx).eval(expr) {
127+
pub fn asserted_len_from_int_lit_expr(expr: &'hir Expr<'hir>) -> Option<Self> {
128+
if let ExprKind::Lit(Spanned {
129+
node: LitKind::Int(Pu128(x), _),
130+
..
131+
}) = expr.kind
132+
{
133+
Some(Self::AssertedLen(EvaluatedIntExpr {
134+
expr,
135+
value: x as usize,
136+
}))
137+
} else {
138+
None
139+
}
140+
}
141+
pub fn asserted_len_from_possibly_const_expr(cx: &LateContext<'_>, expr: &'hir Expr<'hir>) -> Option<Self> {
142+
if let Some(Constant::Int(x)) = ConstEvalCtxt::new(cx).eval(expr) {
136143
Some(Self::AssertedLen(EvaluatedIntExpr {
137144
expr,
138145
value: x as usize,
@@ -152,21 +159,31 @@ fn len_comparison<'hir>(
152159
left_expr: &'hir Expr<'hir>,
153160
right_expr: &'hir Expr<'hir>,
154161
) -> Option<(LengthComparison, EvaluatedIntExpr<'hir>, &'hir Expr<'hir>)> {
162+
fn sniff_operands<'hir>(
163+
cx: &LateContext<'_>,
164+
left: &'hir Expr<'hir>,
165+
right: &'hir Expr<'hir>,
166+
) -> Option<(AssertionSide<'hir>, AssertionSide<'hir>)> {
167+
// sniff as cheap as possible
168+
if let Some(left) = AssertionSide::asserted_len_from_int_lit_expr(left) {
169+
Some((left, AssertionSide::from_expr(cx, right)?))
170+
} else if let Some(left) = AssertionSide::slice_len_from_expr(cx, left) {
171+
Some((left, AssertionSide::from_expr(cx, right)?))
172+
} else {
173+
Some((
174+
AssertionSide::asserted_len_from_possibly_const_expr(cx, left)?,
175+
AssertionSide::slice_len_from_expr(cx, right)?,
176+
))
177+
}
178+
}
179+
155180
type Side<'hir> = AssertionSide<'hir>;
156181

157182
// normalize comparison, `v.len() > 4` becomes `4 < v.len()`
158183
// this simplifies the logic a bit
159184
let (op, left_expr, right_expr) = normalize_comparison(bin_op, left_expr, right_expr)?;
160185

161-
// sniff operands as cheap as possible
162-
// eliminate possibility of both sides being `v.len()` variant
163-
let (left, right) = match AssertionSide::slice_len_from_expr(cx, left_expr) {
164-
Some(left) => (left, AssertionSide::asserted_len_from_expr(cx, right_expr)?),
165-
None => (
166-
AssertionSide::asserted_len_from_expr(cx, left_expr)?,
167-
AssertionSide::slice_len_from_expr(cx, right_expr)?,
168-
),
169-
};
186+
let (left, right) = sniff_operands(cx, left_expr, right_expr)?;
170187
let (swapped, asserted_len, slice) = match (left, right) {
171188
// `A > B` (e.g. `5 > 4`)
172189
| (Side::AssertedLen(_), Side::AssertedLen(_))

0 commit comments

Comments
 (0)