From 21a39ad6cc2d8054269393103d85856fdfe324ac Mon Sep 17 00:00:00 2001 From: Hubert Bugaj Date: Fri, 3 Apr 2026 10:56:53 +0200 Subject: [PATCH] feat: detect `== 0` on unsigned types as a `manual_clamp` lower bound --- clippy_lints/src/manual_clamp.rs | 46 +++++++++++++++++++++++- tests/ui/manual_clamp.fixed | 38 ++++++++++++++++++++ tests/ui/manual_clamp.rs | 61 ++++++++++++++++++++++++++++++++ tests/ui/manual_clamp.stderr | 56 ++++++++++++++++++++++++++++- 4 files changed, 199 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/manual_clamp.rs b/clippy_lints/src/manual_clamp.rs index a1a9266deaf9..407ddb3aaf20 100644 --- a/clippy_lints/src/manual_clamp.rs +++ b/clippy_lints/src/manual_clamp.rs @@ -7,7 +7,7 @@ use clippy_utils::res::{MaybeDef as _, MaybeResPath as _, MaybeTypeckRes as _}; use clippy_utils::sugg::Sugg; use clippy_utils::ty::implements_trait; use clippy_utils::visitors::is_const_evaluatable; -use clippy_utils::{eq_expr_value, is_in_const_context, peel_blocks, peel_blocks_with_stmt, sym}; +use clippy_utils::{eq_expr_value, is_in_const_context, is_integer_literal, peel_blocks, peel_blocks_with_stmt, sym}; use itertools::Itertools as _; use rustc_errors::{Applicability, Diag}; use rustc_hir::def::Res; @@ -718,6 +718,9 @@ fn is_clamp_meta_pattern<'tcx>( if exprs.iter().any(|e| peel_blocks(e).can_have_side_effects()) { return None; } + let first_bin = try_normalize_unsigned_eq_to_ord(cx, first_bin, first_expr).unwrap_or(*first_bin); + let second_bin = try_normalize_unsigned_eq_to_ord(cx, second_bin, second_expr).unwrap_or(*second_bin); + let (first_bin, second_bin) = (&first_bin, &second_bin); if !(is_ord_op(first_bin.op) && is_ord_op(second_bin.op)) { return None; } @@ -771,6 +774,47 @@ fn is_ord_op(op: BinOpKind) -> bool { matches!(op, BinOpKind::Ge | BinOpKind::Gt | BinOpKind::Le | BinOpKind::Lt) } +/// For unsigned integer types, `x == 0` is equivalent to `x < 1`. This normalizes such `==` +/// comparisons to `<` when the result expression evaluates to 1, enabling clamp detection for +/// patterns like: +/// +/// ```ignore +/// if x == 0 { x = 1; } else if x > y { x = y; } +/// ``` +fn try_normalize_unsigned_eq_to_ord<'tcx>( + cx: &LateContext<'tcx>, + bin: &BinaryOp<'tcx>, + result_expr: &'tcx Expr<'tcx>, +) -> Option> { + if bin.op != BinOpKind::Eq { + return None; + } + + // both sides of the expression have the same type, so checking either of them is sufficient + if !matches!(cx.typeck_results().expr_ty(bin.left).kind(), rustc_middle::ty::Uint(_)) { + return None; + } + + // Handles both `x == 0` and the Yoda notation `0 == x`. + let input = if is_integer_literal(bin.right, 0) { + bin.left + } else if is_integer_literal(bin.left, 0) { + bin.right + } else { + return None; + }; + + if is_integer_literal(result_expr, 1) { + Some(BinaryOp { + op: BinOpKind::Lt, + left: input, + right: result_expr, + }) + } else { + None + } +} + /// Really similar to Cow, but doesn't have a `Clone` requirement. #[derive(Debug)] enum MaybeBorrowedStmtKind<'a> { diff --git a/tests/ui/manual_clamp.fixed b/tests/ui/manual_clamp.fixed index 4cd6fe60f381..5b3670d46bfb 100644 --- a/tests/ui/manual_clamp.fixed +++ b/tests/ui/manual_clamp.fixed @@ -410,6 +410,44 @@ fn msrv_1_50() { let _ = input.clamp(CONST_MIN, CONST_MAX); } +const CONST_U32_MAX: u32 = 100; + +fn eq_unsigned_zero_lint() { + // == 0 on unsigned with result 1: equivalent to < 1 + let mut x0: u32 = 5; + x0 = x0.clamp(1, CONST_U32_MAX); + + // Flipped: `0 == x` + let mut x1: u32 = 5; + x1 = x1.clamp(1, CONST_U32_MAX); + + // Two separate if statements + let mut x2: u32 = 5; + x2 = x2.clamp(1, CONST_U32_MAX); + + // if-elseif-else (value) pattern + let input: u32 = 5; + let x3 = input.clamp(1, CONST_U32_MAX); +} + +fn eq_unsigned_zero_no_lint() { + // Result is not 1, so == 0 doesn't cover all values < 5. + let mut x0: u32 = 5; + if x0 == 0 { + x0 = 5; + } else if x0 > CONST_U32_MAX { + x0 = CONST_U32_MAX; + } + + // Signed type: not applicable + let mut x1: i32 = 5; + if x1 == 0 { + x1 = 1; + } else if x1 > CONST_MAX { + x1 = CONST_MAX; + } +} + const fn _const() { let (input, min, max) = (0, -1, 2); let _ = if input < min { diff --git a/tests/ui/manual_clamp.rs b/tests/ui/manual_clamp.rs index f0613501a8e8..04de0677893f 100644 --- a/tests/ui/manual_clamp.rs +++ b/tests/ui/manual_clamp.rs @@ -527,6 +527,67 @@ fn msrv_1_50() { }; } +const CONST_U32_MAX: u32 = 100; + +fn eq_unsigned_zero_lint() { + // == 0 on unsigned with result 1: equivalent to < 1 + let mut x0: u32 = 5; + if x0 == 0 { + //~^ manual_clamp + x0 = 1; + } else if x0 > CONST_U32_MAX { + x0 = CONST_U32_MAX; + } + + // Flipped: `0 == x` + let mut x1: u32 = 5; + if 0 == x1 { + //~^ manual_clamp + x1 = 1; + } else if x1 > CONST_U32_MAX { + x1 = CONST_U32_MAX; + } + + // Two separate if statements + let mut x2: u32 = 5; + if x2 == 0 { + //~^ manual_clamp + x2 = 1; + } + if x2 > CONST_U32_MAX { + x2 = CONST_U32_MAX; + } + + // if-elseif-else (value) pattern + let input: u32 = 5; + let x3 = if input == 0 { + //~^ manual_clamp + 1 + } else if input > CONST_U32_MAX { + CONST_U32_MAX + } else { + input + }; +} + +fn eq_unsigned_zero_no_lint() { + // Result is not 1, so == 0 doesn't cover all values < 5. + let mut x0: u32 = 5; + if x0 == 0 { + x0 = 5; + } else if x0 > CONST_U32_MAX { + x0 = CONST_U32_MAX; + } + + // Signed type: not applicable + let mut x1: i32 = 5; + if x1 == 0 { + x1 = 1; + } else if x1 > CONST_MAX { + x1 = CONST_MAX; + } +} + const fn _const() { let (input, min, max) = (0, -1, 2); let _ = if input < min { diff --git a/tests/ui/manual_clamp.stderr b/tests/ui/manual_clamp.stderr index 4876e2eb1cd1..11568be6b22d 100644 --- a/tests/ui/manual_clamp.stderr +++ b/tests/ui/manual_clamp.stderr @@ -398,5 +398,59 @@ LL | | }; | = note: clamp will panic if max < min -error: aborting due to 35 previous errors +error: clamp-like pattern without using clamp function + --> tests/ui/manual_clamp.rs:553:5 + | +LL | / if x2 == 0 { +LL | | +LL | | x2 = 1; +... | +LL | | x2 = CONST_U32_MAX; +LL | | } + | |_____^ help: replace with clamp: `x2 = x2.clamp(1, CONST_U32_MAX);` + | + = note: clamp will panic if max < min + +error: clamp-like pattern without using clamp function + --> tests/ui/manual_clamp.rs:535:5 + | +LL | / if x0 == 0 { +LL | | +LL | | x0 = 1; +LL | | } else if x0 > CONST_U32_MAX { +LL | | x0 = CONST_U32_MAX; +LL | | } + | |_____^ help: replace with clamp: `x0 = x0.clamp(1, CONST_U32_MAX);` + | + = note: clamp will panic if max < min + +error: clamp-like pattern without using clamp function + --> tests/ui/manual_clamp.rs:544:5 + | +LL | / if 0 == x1 { +LL | | +LL | | x1 = 1; +LL | | } else if x1 > CONST_U32_MAX { +LL | | x1 = CONST_U32_MAX; +LL | | } + | |_____^ help: replace with clamp: `x1 = x1.clamp(1, CONST_U32_MAX);` + | + = note: clamp will panic if max < min + +error: clamp-like pattern without using clamp function + --> tests/ui/manual_clamp.rs:563:14 + | +LL | let x3 = if input == 0 { + | ______________^ +LL | | +LL | | 1 +LL | | } else if input > CONST_U32_MAX { +... | +LL | | input +LL | | }; + | |_____^ help: replace with clamp: `input.clamp(1, CONST_U32_MAX)` + | + = note: clamp will panic if max < min + +error: aborting due to 39 previous errors