From e5cc3d7a25df71efdec31aec6587b7a37acbc4f7 Mon Sep 17 00:00:00 2001 From: Amerikrainian Date: Thu, 27 Nov 2025 19:03:41 -0600 Subject: [PATCH 1/3] Add manual checked div lint --- CHANGELOG.md | 5 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/manual_checked_div.rs | 132 +++++++++++++++++++++++++ tests/ui/manual_checked_div.fixed | 35 +++++++ tests/ui/manual_checked_div.rs | 35 +++++++ tests/ui/manual_checked_div.stderr | 23 +++++ 7 files changed, 233 insertions(+) create mode 100644 clippy_lints/src/manual_checked_div.rs create mode 100644 tests/ui/manual_checked_div.fixed create mode 100644 tests/ui/manual_checked_div.rs create mode 100644 tests/ui/manual_checked_div.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 78b81b5b74d6..3e004e5dbf69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ document. [e9b7045...master](https://github.com/rust-lang/rust-clippy/compare/e9b7045...master) +### New Lints + +* Added [`manual_checked_div`] to `nursery` + ## Rust 1.91 Current stable, released 2025-10-30 @@ -6530,6 +6534,7 @@ Released 2018-09-13 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn [`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits [`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals +[`manual_checked_div`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_checked_div [`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp [`manual_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_contains [`manual_dangling_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_dangling_ptr diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 4a350dca2993..e26d15b1ec10 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -295,6 +295,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::manual_assert::MANUAL_ASSERT_INFO, crate::manual_async_fn::MANUAL_ASYNC_FN_INFO, crate::manual_bits::MANUAL_BITS_INFO, + crate::manual_checked_div::MANUAL_CHECKED_DIV_INFO, crate::manual_clamp::MANUAL_CLAMP_INFO, crate::manual_float_methods::MANUAL_IS_FINITE_INFO, crate::manual_float_methods::MANUAL_IS_INFINITE_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 230d83dacc95..38dc3d05498e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -197,6 +197,7 @@ mod manual_abs_diff; mod manual_assert; mod manual_async_fn; mod manual_bits; +mod manual_checked_div; mod manual_clamp; mod manual_float_methods; mod manual_hash_one; @@ -848,6 +849,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)), Box::new(|_| Box::new(volatile_composites::VolatileComposites)), Box::new(|_| Box::::default()), + Box::new(|_| Box::new(manual_checked_div::ManualCheckedDiv)), // add late passes here, used by `cargo dev new_lint` ]; store.late_passes.extend(late_lints); diff --git a/clippy_lints/src/manual_checked_div.rs b/clippy_lints/src/manual_checked_div.rs new file mode 100644 index 000000000000..9acab601e06f --- /dev/null +++ b/clippy_lints/src/manual_checked_div.rs @@ -0,0 +1,132 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::sugg::Sugg; +use clippy_utils::visitors::{Descend, for_each_expr_without_closures}; +use clippy_utils::{SpanlessEq, is_integer_literal}; +use rustc_errors::Applicability; +use rustc_hir::{BinOpKind, Block, Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_session::declare_lint_pass; +use std::ops::ControlFlow; + +declare_clippy_lint! { + /// ### What it does + /// Detects manual zero checks before dividing unsigned integers, such as `if x != 0 { y / x }`. + /// + /// ### Why is this bad? + /// `checked_div` already handles the zero case and makes the intent clearer while avoiding a + /// panic from a manual division. + /// + /// ### Example + /// ```no_run + /// # let (a, b) = (10u32, 5u32); + /// if b != 0 { + /// let result = a / b; + /// println!("{result}"); + /// } + /// ``` + /// Use instead: + /// ```no_run + /// # let (a, b) = (10u32, 5u32); + /// if let Some(result) = a.checked_div(b) { + /// println!("{result}"); + /// } + /// ``` + #[clippy::version = "1.93.0"] + pub MANUAL_CHECKED_DIV, + nursery, + "manual zero checks before dividing unsigned integers" +} +declare_lint_pass!(ManualCheckedDiv => [MANUAL_CHECKED_DIV]); + +#[derive(Copy, Clone)] +enum NonZeroBranch { + Then, + Else, +} + +impl LateLintPass<'_> for ManualCheckedDiv { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + if expr.span.from_expansion() { + return; + } + + if let ExprKind::If(cond, then, r#else) = expr.kind + && let Some((divisor, branch)) = divisor_from_condition(cond) + && is_unsigned(cx, divisor) + { + let Some(block) = branch_block(then, r#else, branch) else { + return; + }; + let mut eq = SpanlessEq::new(cx); + + for_each_expr_without_closures(block, |e| { + if let ExprKind::Binary(binop, lhs, rhs) = e.kind + && binop.node == BinOpKind::Div + && eq.eq_expr(rhs, divisor) + && is_unsigned(cx, lhs) + { + let mut applicability = Applicability::MaybeIncorrect; + let lhs_snip = Sugg::hir_with_applicability(cx, lhs, "..", &mut applicability); + let rhs_snip = Sugg::hir_with_applicability(cx, rhs, "..", &mut applicability); + + span_lint_and_sugg( + cx, + MANUAL_CHECKED_DIV, + e.span, + "manual checked division", + "consider using `checked_div`", + format!("{}.checked_div({})", lhs_snip.maybe_paren(), rhs_snip), + applicability, + ); + + ControlFlow::<(), _>::Continue(Descend::No) + } else { + ControlFlow::<(), _>::Continue(Descend::Yes) + } + }); + } + } +} + +fn divisor_from_condition<'tcx>(cond: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, NonZeroBranch)> { + let ExprKind::Binary(binop, lhs, rhs) = cond.kind else { + return None; + }; + + match binop.node { + BinOpKind::Ne | BinOpKind::Lt if is_zero(lhs) => Some((rhs, NonZeroBranch::Then)), + BinOpKind::Ne | BinOpKind::Gt if is_zero(rhs) => Some((lhs, NonZeroBranch::Then)), + BinOpKind::Eq if is_zero(lhs) => Some((rhs, NonZeroBranch::Else)), + BinOpKind::Eq if is_zero(rhs) => Some((lhs, NonZeroBranch::Else)), + _ => None, + } +} + +fn branch_block<'tcx>( + then: &'tcx Expr<'tcx>, + r#else: Option<&'tcx Expr<'tcx>>, + branch: NonZeroBranch, +) -> Option<&'tcx Block<'tcx>> { + match branch { + NonZeroBranch::Then => { + if let ExprKind::Block(block, _) = then.kind { + Some(block) + } else { + None + } + }, + NonZeroBranch::Else => match r#else.map(|expr| &expr.kind) { + Some(ExprKind::Block(block, _)) => Some(block), + _ => None, + }, + } +} + +fn is_zero(expr: &Expr<'_>) -> bool { + is_integer_literal(expr, 0) +} + +fn is_unsigned(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + matches!(cx.typeck_results().expr_ty(expr).peel_refs().kind(), ty::Uint(_)) +} diff --git a/tests/ui/manual_checked_div.fixed b/tests/ui/manual_checked_div.fixed new file mode 100644 index 000000000000..84cb1603c823 --- /dev/null +++ b/tests/ui/manual_checked_div.fixed @@ -0,0 +1,35 @@ +#![warn(clippy::manual_checked_div)] + +fn main() { + let a = 10u32; + let b = 5u32; + + // Should trigger lint + if b != 0 { + let _result = a.checked_div(b); + //~^ manual_checked_div + } + + if b > 0 { + let _result = a.checked_div(b); + //~^ manual_checked_div + } + + if b == 0 { + println!("zero"); + } else { + let _result = a.checked_div(b); + //~^ manual_checked_div + } + + // Should NOT trigger (already using checked_div) + if let Some(result) = b.checked_div(a) { + println!("{result}"); + } + + // Should NOT trigger (signed integers) + let c = -5i32; + if c != 0 { + let _result = 10 / c; + } +} diff --git a/tests/ui/manual_checked_div.rs b/tests/ui/manual_checked_div.rs new file mode 100644 index 000000000000..3812e22c167f --- /dev/null +++ b/tests/ui/manual_checked_div.rs @@ -0,0 +1,35 @@ +#![warn(clippy::manual_checked_div)] + +fn main() { + let a = 10u32; + let b = 5u32; + + // Should trigger lint + if b != 0 { + let _result = a / b; + //~^ manual_checked_div + } + + if b > 0 { + let _result = a / b; + //~^ manual_checked_div + } + + if b == 0 { + println!("zero"); + } else { + let _result = a / b; + //~^ manual_checked_div + } + + // Should NOT trigger (already using checked_div) + if let Some(result) = b.checked_div(a) { + println!("{result}"); + } + + // Should NOT trigger (signed integers) + let c = -5i32; + if c != 0 { + let _result = 10 / c; + } +} diff --git a/tests/ui/manual_checked_div.stderr b/tests/ui/manual_checked_div.stderr new file mode 100644 index 000000000000..967ff56a63fc --- /dev/null +++ b/tests/ui/manual_checked_div.stderr @@ -0,0 +1,23 @@ +error: manual checked division + --> tests/ui/manual_checked_div.rs:9:23 + | +LL | let _result = a / b; + | ^^^^^ help: consider using `checked_div`: `a.checked_div(b)` + | + = note: `-D clippy::manual-checked-div` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::manual_checked_div)]` + +error: manual checked division + --> tests/ui/manual_checked_div.rs:14:23 + | +LL | let _result = a / b; + | ^^^^^ help: consider using `checked_div`: `a.checked_div(b)` + +error: manual checked division + --> tests/ui/manual_checked_div.rs:21:23 + | +LL | let _result = a / b; + | ^^^^^ help: consider using `checked_div`: `a.checked_div(b)` + +error: aborting due to 3 previous errors + From 736b40c290e3a3f81e67a03b5dce99f9c9f2a167 Mon Sep 17 00:00:00 2001 From: Amerikrainian Date: Thu, 27 Nov 2025 19:11:33 -0600 Subject: [PATCH 2/3] Drop manual_checked_div changelog edit --- CHANGELOG.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e004e5dbf69..78b81b5b74d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,10 +8,6 @@ document. [e9b7045...master](https://github.com/rust-lang/rust-clippy/compare/e9b7045...master) -### New Lints - -* Added [`manual_checked_div`] to `nursery` - ## Rust 1.91 Current stable, released 2025-10-30 @@ -6534,7 +6530,6 @@ Released 2018-09-13 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn [`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits [`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals -[`manual_checked_div`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_checked_div [`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp [`manual_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_contains [`manual_dangling_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_dangling_ptr From bf07325428a1d96e9dfcba6652c855c08e46514e Mon Sep 17 00:00:00 2001 From: Amerikrainian Date: Thu, 27 Nov 2025 19:21:50 -0600 Subject: [PATCH 3/3] Run cargo dev update_lints --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78b81b5b74d6..040bd0c79608 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6530,6 +6530,7 @@ Released 2018-09-13 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn [`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits [`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals +[`manual_checked_div`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_checked_div [`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp [`manual_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_contains [`manual_dangling_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_dangling_ptr