Skip to content

Commit

Permalink
Move MSRV check later in implicit_saturating_sub
Browse files Browse the repository at this point in the history
  • Loading branch information
GuillaumeGomez committed Aug 31, 2024
1 parent d7c4456 commit 754255a
Showing 1 changed file with 33 additions and 7 deletions.
40 changes: 33 additions & 7 deletions clippy_lints/src/implicit_saturating_sub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use clippy_config::Conf;
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::source::snippet_opt;
use clippy_utils::{
higher, in_constant, is_integer_literal, path_to_local, peel_blocks, peel_blocks_with_stmt, SpanlessEq,
higher, is_in_const_context, is_integer_literal, path_to_local, peel_blocks, peel_blocks_with_stmt, SpanlessEq,
};
use rustc_ast::ast::LitKind;
use rustc_data_structures::packed::Pu128;
Expand Down Expand Up @@ -94,9 +94,6 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
if expr.span.from_expansion() {
return;
}
if in_constant(cx, expr.hir_id) && !self.msrv.meets(msrvs::SATURATING_SUB_CONST) {
return;
}
if let Some(higher::If { cond, then, r#else: None }) = higher::If::hir(expr)

// Check if the conditional expression is a binary operation
Expand All @@ -110,13 +107,16 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
}) = higher::If::hir(expr)
&& let ExprKind::Binary(ref cond_op, cond_left, cond_right) = cond.kind
{
check_manual_check(cx, expr, cond_op, cond_left, cond_right, if_block, else_block);
check_manual_check(
cx, expr, cond_op, cond_left, cond_right, if_block, else_block, &self.msrv,
);
}
}

extract_msrv_attr!(LateContext);
}

#[allow(clippy::too_many_arguments)]
fn check_manual_check<'tcx>(
cx: &LateContext<'tcx>,
expr: &Expr<'tcx>,
Expand All @@ -125,6 +125,7 @@ fn check_manual_check<'tcx>(
right_hand: &Expr<'tcx>,
if_block: &Expr<'tcx>,
else_block: &Expr<'tcx>,
msrv: &Msrv,
) {
let ty = cx.typeck_results().expr_ty(left_hand);
if ty.is_numeric() && !ty.is_signed() {
Expand All @@ -137,6 +138,7 @@ fn check_manual_check<'tcx>(
right_hand,
if_block,
else_block,
msrv,
),
BinOpKind::Lt | BinOpKind::Le => check_gt(
cx,
Expand All @@ -146,12 +148,14 @@ fn check_manual_check<'tcx>(
left_hand,
if_block,
else_block,
msrv,
),
_ => {},
}
}
}

#[allow(clippy::too_many_arguments)]
fn check_gt(
cx: &LateContext<'_>,
condition_span: Span,
Expand All @@ -160,11 +164,21 @@ fn check_gt(
little_var: &Expr<'_>,
if_block: &Expr<'_>,
else_block: &Expr<'_>,
msrv: &Msrv,
) {
if let Some(big_var) = Var::new(big_var)
&& let Some(little_var) = Var::new(little_var)
{
check_subtraction(cx, condition_span, expr_span, big_var, little_var, if_block, else_block);
check_subtraction(
cx,
condition_span,
expr_span,
big_var,
little_var,
if_block,
else_block,
msrv,
);
}
}

Expand All @@ -182,6 +196,7 @@ impl Var {
}
}

#[allow(clippy::too_many_arguments)]
fn check_subtraction(
cx: &LateContext<'_>,
condition_span: Span,
Expand All @@ -190,6 +205,7 @@ fn check_subtraction(
little_var: Var,
if_block: &Expr<'_>,
else_block: &Expr<'_>,
msrv: &Msrv,
) {
let if_block = peel_blocks(if_block);
let else_block = peel_blocks(else_block);
Expand All @@ -201,7 +217,16 @@ fn check_subtraction(
}
// If the subtraction is done in the `else` block, then we need to also revert the two
// variables as it means that the check was reverted too.
check_subtraction(cx, condition_span, expr_span, little_var, big_var, else_block, if_block);
check_subtraction(
cx,
condition_span,
expr_span,
little_var,
big_var,
else_block,
if_block,
msrv,
);
return;
}
if is_integer_literal(else_block, 0)
Expand All @@ -215,6 +240,7 @@ fn check_subtraction(
// if `snippet_opt` fails, it won't try the next conditions.
if let Some(big_var_snippet) = snippet_opt(cx, big_var.span)
&& let Some(little_var_snippet) = snippet_opt(cx, little_var.span)
&& (!is_in_const_context(cx) || msrv.meets(msrvs::SATURATING_SUB_CONST))
{
span_lint_and_sugg(
cx,
Expand Down

0 comments on commit 754255a

Please sign in to comment.