From 5ede2424f847f1306e2c6a179b236a365f44a445 Mon Sep 17 00:00:00 2001 From: Amerikrainian Date: Thu, 27 Nov 2025 19:03:41 -0600 Subject: [PATCH 1/9] 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 91d793489be2..7fe1b14a1fbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,10 @@ Current stable, released 2025-12-11 * [`double_parens`] fixed FP when macros are involved [#15420](https://github.com/rust-lang/rust-clippy/pull/15420) +### New Lints + +* Added [`manual_checked_div`] to `nursery` + ## Rust 1.91 Current stable, released 2025-10-30 @@ -6607,6 +6611,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 6b68940c6423..6e9baf9ed1e0 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -296,6 +296,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 a957afdb1910..172525291a82 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -198,6 +198,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; @@ -857,6 +858,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(|_| Box::::default()), Box::new(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))), Box::new(|_| Box::new(same_length_and_capacity::SameLengthAndCapacity)), + 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 1264a9172126b09eded895323ab524689c9ecb5e Mon Sep 17 00:00:00 2001 From: Amerikrainian Date: Thu, 27 Nov 2025 19:11:33 -0600 Subject: [PATCH 2/9] Drop manual_checked_div changelog edit --- CHANGELOG.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fe1b14a1fbb..91d793489be2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,10 +84,6 @@ Current stable, released 2025-12-11 * [`double_parens`] fixed FP when macros are involved [#15420](https://github.com/rust-lang/rust-clippy/pull/15420) -### New Lints - -* Added [`manual_checked_div`] to `nursery` - ## Rust 1.91 Current stable, released 2025-10-30 @@ -6611,7 +6607,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 66ddbdcf13956f16dee9f988639c1f4d259c26b9 Mon Sep 17 00:00:00 2001 From: Amerikrainian Date: Thu, 27 Nov 2025 19:21:50 -0600 Subject: [PATCH 3/9] Run cargo dev update_lints --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91d793489be2..ad2c264021ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6607,6 +6607,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 From f7615a4a2f848f427c7e302633a6cd5a23f6960f Mon Sep 17 00:00:00 2001 From: Amerikrainian Date: Mon, 15 Dec 2025 22:53:11 -0600 Subject: [PATCH 4/9] Make checked div work on signed ints and collect multiple divisions --- clippy_lints/src/manual_checked_div.rs | 100 ++++++++++++++++--------- tests/ui/manual_checked_div.fixed | 11 +-- tests/ui/manual_checked_div.rs | 9 ++- tests/ui/manual_checked_div.stderr | 35 +++++++-- 4 files changed, 104 insertions(+), 51 deletions(-) diff --git a/clippy_lints/src/manual_checked_div.rs b/clippy_lints/src/manual_checked_div.rs index 9acab601e06f..195ca38c5561 100644 --- a/clippy_lints/src/manual_checked_div.rs +++ b/clippy_lints/src/manual_checked_div.rs @@ -1,17 +1,19 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::span_lint_and_then; 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_ast::{LitIntType, LitKind}; +use rustc_errors::{Applicability, MultiSpan}; +use rustc_hir::{BinOpKind, Block, Expr, ExprKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::declare_lint_pass; +use rustc_span::source_map::Spanned; 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 }`. + /// Detects manual zero checks before dividing 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 @@ -35,7 +37,7 @@ declare_clippy_lint! { #[clippy::version = "1.93.0"] pub MANUAL_CHECKED_DIV, nursery, - "manual zero checks before dividing unsigned integers" + "manual zero checks before dividing integers" } declare_lint_pass!(ManualCheckedDiv => [MANUAL_CHECKED_DIV]); @@ -47,44 +49,68 @@ enum NonZeroBranch { 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 + && !expr.span.from_expansion() && let Some((divisor, branch)) = divisor_from_condition(cond) - && is_unsigned(cx, divisor) + && is_integer(cx, divisor) + && let Some(block) = branch_block(then, r#else, branch) { - let Some(block) = branch_block(then, r#else, branch) else { - return; - }; let mut eq = SpanlessEq::new(cx); + let mut applicability = Applicability::MaybeIncorrect; + let mut divisions = Vec::new(); 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) + && is_integer(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, - ); + let lhs_snip = Sugg::hir_with_applicability(cx, lhs, "_", &mut applicability); + let rhs_snip = Sugg::hir_with_applicability(cx, rhs, "_", &mut applicability); + let lhs_ty = cx.typeck_results().expr_ty(lhs); + let lhs_sugg = lhs_snip.maybe_paren().to_string(); + let type_suffix = if matches!( + lhs.kind, + ExprKind::Lit(Spanned { + node: LitKind::Int(_, LitIntType::Unsuffixed), + .. + }) | ExprKind::Unary( + UnOp::Neg, + Expr { + kind: ExprKind::Lit(Spanned { + node: LitKind::Int(_, LitIntType::Unsuffixed), + .. + }), + .. + } + ) + ) { + format!("_{lhs_ty}") + } else { + String::new() + }; + let suggestion = format!("{lhs_sugg}{type_suffix}.checked_div({rhs_snip})"); + divisions.push((e.span, suggestion)); ControlFlow::<(), _>::Continue(Descend::No) } else { ControlFlow::<(), _>::Continue(Descend::Yes) } }); + + if !divisions.is_empty() { + let mut spans: Vec<_> = divisions.iter().map(|(span, _)| *span).collect(); + spans.push(cond.span); + span_lint_and_then( + cx, + MANUAL_CHECKED_DIV, + MultiSpan::from_spans(spans), + "manual checked division", + |diag| { + diag.multipart_suggestion("consider using `checked_div`", divisions, applicability); + }, + ); + } } } } @@ -109,15 +135,12 @@ fn branch_block<'tcx>( branch: NonZeroBranch, ) -> Option<&'tcx Block<'tcx>> { match branch { - NonZeroBranch::Then => { - if let ExprKind::Block(block, _) = then.kind { - Some(block) - } else { - None - } + NonZeroBranch::Then => match then.kind { + ExprKind::Block(block, _) => Some(block), + _ => None, }, - NonZeroBranch::Else => match r#else.map(|expr| &expr.kind) { - Some(ExprKind::Block(block, _)) => Some(block), + NonZeroBranch::Else => match r#else?.kind { + ExprKind::Block(block, _) => Some(block), _ => None, }, } @@ -127,6 +150,9 @@ 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(_)) +fn is_integer(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + matches!( + cx.typeck_results().expr_ty(expr).peel_refs().kind(), + ty::Uint(_) | ty::Int(_) + ) } diff --git a/tests/ui/manual_checked_div.fixed b/tests/ui/manual_checked_div.fixed index 84cb1603c823..b09f356f2b36 100644 --- a/tests/ui/manual_checked_div.fixed +++ b/tests/ui/manual_checked_div.fixed @@ -6,20 +6,21 @@ fn main() { // Should trigger lint if b != 0 { - let _result = a.checked_div(b); //~^ manual_checked_div + let _result = a.checked_div(b); + let _another = (a + 1).checked_div(b); } if b > 0 { - let _result = a.checked_div(b); //~^ manual_checked_div + let _result = a.checked_div(b); } if b == 0 { + //~^ manual_checked_div println!("zero"); } else { let _result = a.checked_div(b); - //~^ manual_checked_div } // Should NOT trigger (already using checked_div) @@ -27,9 +28,9 @@ fn main() { println!("{result}"); } - // Should NOT trigger (signed integers) let c = -5i32; if c != 0 { - let _result = 10 / c; + //~^ manual_checked_div + let _result = 10_i32.checked_div(c); } } diff --git a/tests/ui/manual_checked_div.rs b/tests/ui/manual_checked_div.rs index 3812e22c167f..bdbbe93c6064 100644 --- a/tests/ui/manual_checked_div.rs +++ b/tests/ui/manual_checked_div.rs @@ -6,20 +6,21 @@ fn main() { // Should trigger lint if b != 0 { - let _result = a / b; //~^ manual_checked_div + let _result = a / b; + let _another = (a + 1) / b; } if b > 0 { - let _result = a / b; //~^ manual_checked_div + let _result = a / b; } if b == 0 { + //~^ manual_checked_div println!("zero"); } else { let _result = a / b; - //~^ manual_checked_div } // Should NOT trigger (already using checked_div) @@ -27,9 +28,9 @@ fn main() { println!("{result}"); } - // Should NOT trigger (signed integers) let c = -5i32; if c != 0 { + //~^ manual_checked_div let _result = 10 / c; } } diff --git a/tests/ui/manual_checked_div.stderr b/tests/ui/manual_checked_div.stderr index 967ff56a63fc..062d953d9c96 100644 --- a/tests/ui/manual_checked_div.stderr +++ b/tests/ui/manual_checked_div.stderr @@ -1,23 +1,48 @@ error: manual checked division - --> tests/ui/manual_checked_div.rs:9:23 + --> tests/ui/manual_checked_div.rs:8:8 | +LL | if b != 0 { + | ^^^^^^ +LL | LL | let _result = a / b; - | ^^^^^ help: consider using `checked_div`: `a.checked_div(b)` + | ^^^^^ +LL | let _another = (a + 1) / b; + | ^^^^^^^^^^^ | = note: `-D clippy::manual-checked-div` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::manual_checked_div)]` +help: consider using `checked_div` + | +LL ~ let _result = a.checked_div(b); +LL ~ let _another = (a + 1).checked_div(b); + | error: manual checked division - --> tests/ui/manual_checked_div.rs:14:23 + --> tests/ui/manual_checked_div.rs:14:8 | +LL | if b > 0 { + | ^^^^^ +LL | 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 + --> tests/ui/manual_checked_div.rs:19:8 | +LL | if b == 0 { + | ^^^^^^ +... LL | let _result = a / b; | ^^^^^ help: consider using `checked_div`: `a.checked_div(b)` -error: aborting due to 3 previous errors +error: manual checked division + --> tests/ui/manual_checked_div.rs:32:8 + | +LL | if c != 0 { + | ^^^^^^ +LL | +LL | let _result = 10 / c; + | ^^^^^^ help: consider using `checked_div`: `10_i32.checked_div(c)` + +error: aborting due to 4 previous errors From 04e9ae5df9e362fa5c9a14f02396a20ae35361e4 Mon Sep 17 00:00:00 2001 From: Amerikrainian Date: Tue, 16 Dec 2025 08:09:09 -0600 Subject: [PATCH 5/9] Enforced divisor purity and first use --- clippy_lints/src/manual_checked_div.rs | 52 +++++++++++++++++++------- tests/ui/manual_checked_div.fixed | 32 +++++++++++++++- tests/ui/manual_checked_div.rs | 32 +++++++++++++++- 3 files changed, 100 insertions(+), 16 deletions(-) diff --git a/clippy_lints/src/manual_checked_div.rs b/clippy_lints/src/manual_checked_div.rs index 195ca38c5561..864fde1ef3ec 100644 --- a/clippy_lints/src/manual_checked_div.rs +++ b/clippy_lints/src/manual_checked_div.rs @@ -55,16 +55,27 @@ impl LateLintPass<'_> for ManualCheckedDiv { && is_integer(cx, divisor) && let Some(block) = branch_block(then, r#else, branch) { - let mut eq = SpanlessEq::new(cx); + let mut eq = SpanlessEq::new(cx).deny_side_effects().paths_by_resolution(); + if !eq.eq_expr(divisor, divisor) { + return; + } + let mut applicability = Applicability::MaybeIncorrect; let mut divisions = Vec::new(); + let mut first_use = None; - for_each_expr_without_closures(block, |e| { + let found_early_use = 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_integer(cx, lhs) { + match first_use { + None => first_use = Some(UseKind::Division), + Some(UseKind::Other) => return ControlFlow::Break(()), + Some(UseKind::Division) => {}, + } + let lhs_snip = Sugg::hir_with_applicability(cx, lhs, "_", &mut applicability); let rhs_snip = Sugg::hir_with_applicability(cx, rhs, "_", &mut applicability); let lhs_ty = cx.typeck_results().expr_ty(lhs); @@ -93,28 +104,41 @@ impl LateLintPass<'_> for ManualCheckedDiv { divisions.push((e.span, suggestion)); ControlFlow::<(), _>::Continue(Descend::No) + } else if eq.eq_expr(e, divisor) { + if first_use.is_none() { + first_use = Some(UseKind::Other); + } + ControlFlow::<(), _>::Continue(Descend::Yes) } else { ControlFlow::<(), _>::Continue(Descend::Yes) } }); - if !divisions.is_empty() { - let mut spans: Vec<_> = divisions.iter().map(|(span, _)| *span).collect(); - spans.push(cond.span); - span_lint_and_then( - cx, - MANUAL_CHECKED_DIV, - MultiSpan::from_spans(spans), - "manual checked division", - |diag| { - diag.multipart_suggestion("consider using `checked_div`", divisions, applicability); - }, - ); + if found_early_use.is_some() || first_use != Some(UseKind::Division) || divisions.is_empty() { + return; } + + let mut spans: Vec<_> = divisions.iter().map(|(span, _)| *span).collect(); + spans.push(cond.span); + span_lint_and_then( + cx, + MANUAL_CHECKED_DIV, + MultiSpan::from_spans(spans), + "manual checked division", + |diag| { + diag.multipart_suggestion("consider using `checked_div`", divisions, applicability); + }, + ); } } } +#[derive(Copy, Clone, Eq, PartialEq)] +enum UseKind { + Division, + Other, +} + 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; diff --git a/tests/ui/manual_checked_div.fixed b/tests/ui/manual_checked_div.fixed index b09f356f2b36..36b1b68d6e87 100644 --- a/tests/ui/manual_checked_div.fixed +++ b/tests/ui/manual_checked_div.fixed @@ -2,7 +2,7 @@ fn main() { let a = 10u32; - let b = 5u32; + let mut b = 5u32; // Should trigger lint if b != 0 { @@ -33,4 +33,34 @@ fn main() { //~^ manual_checked_div let _result = 10_i32.checked_div(c); } + + // Should NOT trigger (side effects in divisor) + if counter() > 0 { + let _ = 32 / counter(); + } + + // Should NOT trigger (divisor used before division) + if b > 0 { + use_value(b); + let _ = a / b; + } + + // Should NOT trigger (divisor may change during evaluation) + if b > 0 { + g(inc_and_return_value(&mut b), a / b); + } +} + +fn counter() -> u32 { + println!("counter"); + 1 } + +fn use_value(_v: u32) {} + +fn inc_and_return_value(x: &mut u32) -> u32 { + *x += 1; + *x +} + +fn g(_lhs: u32, _rhs: u32) {} diff --git a/tests/ui/manual_checked_div.rs b/tests/ui/manual_checked_div.rs index bdbbe93c6064..ba8b4d817d64 100644 --- a/tests/ui/manual_checked_div.rs +++ b/tests/ui/manual_checked_div.rs @@ -2,7 +2,7 @@ fn main() { let a = 10u32; - let b = 5u32; + let mut b = 5u32; // Should trigger lint if b != 0 { @@ -33,4 +33,34 @@ fn main() { //~^ manual_checked_div let _result = 10 / c; } + + // Should NOT trigger (side effects in divisor) + if counter() > 0 { + let _ = 32 / counter(); + } + + // Should NOT trigger (divisor used before division) + if b > 0 { + use_value(b); + let _ = a / b; + } + + // Should NOT trigger (divisor may change during evaluation) + if b > 0 { + g(inc_and_return_value(&mut b), a / b); + } +} + +fn counter() -> u32 { + println!("counter"); + 1 } + +fn use_value(_v: u32) {} + +fn inc_and_return_value(x: &mut u32) -> u32 { + *x += 1; + *x +} + +fn g(_lhs: u32, _rhs: u32) {} From fc95a21f30e98d09ed422d1af284c16d1a631f39 Mon Sep 17 00:00:00 2001 From: Amerikrainian Date: Tue, 16 Dec 2025 15:03:52 -0600 Subject: [PATCH 6/9] Handle min and -1 case --- clippy_lints/src/manual_checked_div.rs | 54 +++++++++++++++++++++++++- tests/ui/manual_checked_div.fixed | 15 +++++++ tests/ui/manual_checked_div.rs | 15 +++++++ tests/ui/manual_checked_div.stderr | 11 +++++- 4 files changed, 93 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/manual_checked_div.rs b/clippy_lints/src/manual_checked_div.rs index 864fde1ef3ec..28320d9fc243 100644 --- a/clippy_lints/src/manual_checked_div.rs +++ b/clippy_lints/src/manual_checked_div.rs @@ -1,3 +1,4 @@ +use clippy_utils::consts::{ConstEvalCtxt, FullInt}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::sugg::Sugg; use clippy_utils::visitors::{Descend, for_each_expr_without_closures}; @@ -55,6 +56,7 @@ impl LateLintPass<'_> for ManualCheckedDiv { && is_integer(cx, divisor) && let Some(block) = branch_block(then, r#else, branch) { + let divisor_excludes_minus_one = excludes_minus_one(cx, cond, divisor); let mut eq = SpanlessEq::new(cx).deny_side_effects().paths_by_resolution(); if !eq.eq_expr(divisor, divisor) { return; @@ -70,6 +72,14 @@ impl LateLintPass<'_> for ManualCheckedDiv { && eq.eq_expr(rhs, divisor) && is_integer(cx, lhs) { + let lhs_ty = cx.typeck_results().expr_ty(lhs); + if let ty::Int(int_ty) = lhs_ty.peel_refs().kind() + && !divisor_excludes_minus_one + && min_and_minus_one(cx, lhs, rhs, *int_ty) + { + return ControlFlow::Break(()); + } + match first_use { None => first_use = Some(UseKind::Division), Some(UseKind::Other) => return ControlFlow::Break(()), @@ -78,7 +88,6 @@ impl LateLintPass<'_> for ManualCheckedDiv { let lhs_snip = Sugg::hir_with_applicability(cx, lhs, "_", &mut applicability); let rhs_snip = Sugg::hir_with_applicability(cx, rhs, "_", &mut applicability); - let lhs_ty = cx.typeck_results().expr_ty(lhs); let lhs_sugg = lhs_snip.maybe_paren().to_string(); let type_suffix = if matches!( lhs.kind, @@ -174,6 +183,49 @@ fn is_zero(expr: &Expr<'_>) -> bool { is_integer_literal(expr, 0) } +fn min_and_minus_one(cx: &LateContext<'_>, lhs: &Expr<'_>, rhs: &Expr<'_>, int_ty: ty::IntTy) -> bool { + let lhs_val = int_value(cx, lhs); + let rhs_val = int_value(cx, rhs); + + let lhs_maybe_min = lhs_val.is_none_or(|val| match val { + FullInt::S(signed) => signed == int_min_value(int_ty, cx), + FullInt::U(_) => false, + }); + let rhs_maybe_minus_one = rhs_val.is_none_or(|val| matches!(val, FullInt::S(-1))); + + lhs_maybe_min && rhs_maybe_minus_one +} + +fn int_min_value(int_ty: ty::IntTy, cx: &LateContext<'_>) -> i128 { + let bits = match int_ty { + ty::IntTy::Isize => cx.tcx.data_layout.pointer_size().bits(), + ty::IntTy::I8 => 8, + ty::IntTy::I16 => 16, + ty::IntTy::I32 => 32, + ty::IntTy::I64 => 64, + ty::IntTy::I128 => 128, + }; + -(1_i128 << (bits - 1)) +} + +fn int_value(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { + let ecx = ConstEvalCtxt::new(cx); + ecx.eval_full_int(expr, expr.span.ctxt()) +} + +fn excludes_minus_one(cx: &LateContext<'_>, cond: &Expr<'_>, divisor: &Expr<'_>) -> bool { + let ExprKind::Binary(binop, lhs, rhs) = cond.kind else { + return false; + }; + + let mut eq = SpanlessEq::new(cx).deny_side_effects().paths_by_resolution(); + match binop.node { + BinOpKind::Gt if is_zero(rhs) && eq.eq_expr(lhs, divisor) => true, + BinOpKind::Lt if is_zero(lhs) && eq.eq_expr(rhs, divisor) => true, + _ => false, + } +} + fn is_integer(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { matches!( cx.typeck_results().expr_ty(expr).peel_refs().kind(), diff --git a/tests/ui/manual_checked_div.fixed b/tests/ui/manual_checked_div.fixed index 36b1b68d6e87..7d0329d86530 100644 --- a/tests/ui/manual_checked_div.fixed +++ b/tests/ui/manual_checked_div.fixed @@ -45,6 +45,21 @@ fn main() { let _ = a / b; } + let signed_min = i32::MIN; + let mut signed_div: i32 = -1; + + // Should NOT trigger (would change behavior for MIN / -1) + if signed_div != 0 { + let _ = signed_min / signed_div; + } + + signed_div = 2; + + if signed_div > 0 { + //~^ manual_checked_div + let _ = signed_min.checked_div(signed_div); + } + // Should NOT trigger (divisor may change during evaluation) if b > 0 { g(inc_and_return_value(&mut b), a / b); diff --git a/tests/ui/manual_checked_div.rs b/tests/ui/manual_checked_div.rs index ba8b4d817d64..7a0b7d06d745 100644 --- a/tests/ui/manual_checked_div.rs +++ b/tests/ui/manual_checked_div.rs @@ -45,6 +45,21 @@ fn main() { let _ = a / b; } + let signed_min = i32::MIN; + let mut signed_div: i32 = -1; + + // Should NOT trigger (would change behavior for MIN / -1) + if signed_div != 0 { + let _ = signed_min / signed_div; + } + + signed_div = 2; + + if signed_div > 0 { + //~^ manual_checked_div + let _ = signed_min / signed_div; + } + // Should NOT trigger (divisor may change during evaluation) if b > 0 { g(inc_and_return_value(&mut b), a / b); diff --git a/tests/ui/manual_checked_div.stderr b/tests/ui/manual_checked_div.stderr index 062d953d9c96..ef1f6bb4c070 100644 --- a/tests/ui/manual_checked_div.stderr +++ b/tests/ui/manual_checked_div.stderr @@ -44,5 +44,14 @@ LL | LL | let _result = 10 / c; | ^^^^^^ help: consider using `checked_div`: `10_i32.checked_div(c)` -error: aborting due to 4 previous errors +error: manual checked division + --> tests/ui/manual_checked_div.rs:58:8 + | +LL | if signed_div > 0 { + | ^^^^^^^^^^^^^^ +LL | +LL | let _ = signed_min / signed_div; + | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `checked_div`: `signed_min.checked_div(signed_div)` + +error: aborting due to 5 previous errors From b710a93f6f70d007865afa82813199720c5c20e9 Mon Sep 17 00:00:00 2001 From: Amerikrainian Date: Fri, 19 Dec 2025 10:01:23 -0600 Subject: [PATCH 7/9] Revert to unsigned lints --- clippy_lints/src/manual_checked_div.rs | 116 ++++--------------------- tests/ui/manual_checked_div.fixed | 81 ----------------- tests/ui/manual_checked_div.rs | 18 +++- tests/ui/manual_checked_div.stderr | 35 +++----- 4 files changed, 48 insertions(+), 202 deletions(-) delete mode 100644 tests/ui/manual_checked_div.fixed diff --git a/clippy_lints/src/manual_checked_div.rs b/clippy_lints/src/manual_checked_div.rs index 28320d9fc243..ba1b89af7290 100644 --- a/clippy_lints/src/manual_checked_div.rs +++ b/clippy_lints/src/manual_checked_div.rs @@ -1,15 +1,11 @@ -use clippy_utils::consts::{ConstEvalCtxt, FullInt}; use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::sugg::Sugg; use clippy_utils::visitors::{Descend, for_each_expr_without_closures}; use clippy_utils::{SpanlessEq, is_integer_literal}; -use rustc_ast::{LitIntType, LitKind}; -use rustc_errors::{Applicability, MultiSpan}; -use rustc_hir::{BinOpKind, Block, Expr, ExprKind, UnOp}; +use rustc_errors::MultiSpan; +use rustc_hir::{BinOpKind, Block, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::declare_lint_pass; -use rustc_span::source_map::Spanned; use std::ops::ControlFlow; declare_clippy_lint! { @@ -53,64 +49,37 @@ impl LateLintPass<'_> for ManualCheckedDiv { if let ExprKind::If(cond, then, r#else) = expr.kind && !expr.span.from_expansion() && let Some((divisor, branch)) = divisor_from_condition(cond) - && is_integer(cx, divisor) + && is_unsigned_integer(cx, divisor) && let Some(block) = branch_block(then, r#else, branch) { - let divisor_excludes_minus_one = excludes_minus_one(cx, cond, divisor); + // This lint is intended for unsigned integers only. + // + // For signed integers, the most direct refactor to `checked_div` is often not + // semantically equivalent to the original guard. For example, `rhs > 0` deliberately + // excludes negative divisors, while `checked_div` would return `Some` for `rhs = -2`. + // Also, `checked_div` can return `None` for `MIN / -1`, which requires additional handling beyond + // the zero check. let mut eq = SpanlessEq::new(cx).deny_side_effects().paths_by_resolution(); if !eq.eq_expr(divisor, divisor) { return; } - let mut applicability = Applicability::MaybeIncorrect; - let mut divisions = Vec::new(); + let mut division_spans = Vec::new(); let mut first_use = None; let found_early_use = 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_integer(cx, lhs) + && is_unsigned_integer(cx, lhs) { - let lhs_ty = cx.typeck_results().expr_ty(lhs); - if let ty::Int(int_ty) = lhs_ty.peel_refs().kind() - && !divisor_excludes_minus_one - && min_and_minus_one(cx, lhs, rhs, *int_ty) - { - return ControlFlow::Break(()); - } - match first_use { None => first_use = Some(UseKind::Division), Some(UseKind::Other) => return ControlFlow::Break(()), Some(UseKind::Division) => {}, } - let lhs_snip = Sugg::hir_with_applicability(cx, lhs, "_", &mut applicability); - let rhs_snip = Sugg::hir_with_applicability(cx, rhs, "_", &mut applicability); - let lhs_sugg = lhs_snip.maybe_paren().to_string(); - let type_suffix = if matches!( - lhs.kind, - ExprKind::Lit(Spanned { - node: LitKind::Int(_, LitIntType::Unsuffixed), - .. - }) | ExprKind::Unary( - UnOp::Neg, - Expr { - kind: ExprKind::Lit(Spanned { - node: LitKind::Int(_, LitIntType::Unsuffixed), - .. - }), - .. - } - ) - ) { - format!("_{lhs_ty}") - } else { - String::new() - }; - let suggestion = format!("{lhs_sugg}{type_suffix}.checked_div({rhs_snip})"); - divisions.push((e.span, suggestion)); + division_spans.push(e.span); ControlFlow::<(), _>::Continue(Descend::No) } else if eq.eq_expr(e, divisor) { @@ -123,19 +92,18 @@ impl LateLintPass<'_> for ManualCheckedDiv { } }); - if found_early_use.is_some() || first_use != Some(UseKind::Division) || divisions.is_empty() { + if found_early_use.is_some() || first_use != Some(UseKind::Division) || division_spans.is_empty() { return; } - let mut spans: Vec<_> = divisions.iter().map(|(span, _)| *span).collect(); - spans.push(cond.span); + division_spans.push(cond.span); span_lint_and_then( cx, MANUAL_CHECKED_DIV, - MultiSpan::from_spans(spans), + MultiSpan::from_spans(division_spans), "manual checked division", |diag| { - diag.multipart_suggestion("consider using `checked_div`", divisions, applicability); + diag.help("consider using `checked_div` and handling the `Option` result"); }, ); } @@ -183,52 +151,6 @@ fn is_zero(expr: &Expr<'_>) -> bool { is_integer_literal(expr, 0) } -fn min_and_minus_one(cx: &LateContext<'_>, lhs: &Expr<'_>, rhs: &Expr<'_>, int_ty: ty::IntTy) -> bool { - let lhs_val = int_value(cx, lhs); - let rhs_val = int_value(cx, rhs); - - let lhs_maybe_min = lhs_val.is_none_or(|val| match val { - FullInt::S(signed) => signed == int_min_value(int_ty, cx), - FullInt::U(_) => false, - }); - let rhs_maybe_minus_one = rhs_val.is_none_or(|val| matches!(val, FullInt::S(-1))); - - lhs_maybe_min && rhs_maybe_minus_one -} - -fn int_min_value(int_ty: ty::IntTy, cx: &LateContext<'_>) -> i128 { - let bits = match int_ty { - ty::IntTy::Isize => cx.tcx.data_layout.pointer_size().bits(), - ty::IntTy::I8 => 8, - ty::IntTy::I16 => 16, - ty::IntTy::I32 => 32, - ty::IntTy::I64 => 64, - ty::IntTy::I128 => 128, - }; - -(1_i128 << (bits - 1)) -} - -fn int_value(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { - let ecx = ConstEvalCtxt::new(cx); - ecx.eval_full_int(expr, expr.span.ctxt()) -} - -fn excludes_minus_one(cx: &LateContext<'_>, cond: &Expr<'_>, divisor: &Expr<'_>) -> bool { - let ExprKind::Binary(binop, lhs, rhs) = cond.kind else { - return false; - }; - - let mut eq = SpanlessEq::new(cx).deny_side_effects().paths_by_resolution(); - match binop.node { - BinOpKind::Gt if is_zero(rhs) && eq.eq_expr(lhs, divisor) => true, - BinOpKind::Lt if is_zero(lhs) && eq.eq_expr(rhs, divisor) => true, - _ => false, - } -} - -fn is_integer(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - matches!( - cx.typeck_results().expr_ty(expr).peel_refs().kind(), - ty::Uint(_) | ty::Int(_) - ) +fn is_unsigned_integer(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 deleted file mode 100644 index 7d0329d86530..000000000000 --- a/tests/ui/manual_checked_div.fixed +++ /dev/null @@ -1,81 +0,0 @@ -#![warn(clippy::manual_checked_div)] - -fn main() { - let a = 10u32; - let mut b = 5u32; - - // Should trigger lint - if b != 0 { - //~^ manual_checked_div - let _result = a.checked_div(b); - let _another = (a + 1).checked_div(b); - } - - if b > 0 { - //~^ manual_checked_div - let _result = a.checked_div(b); - } - - if b == 0 { - //~^ manual_checked_div - println!("zero"); - } else { - let _result = a.checked_div(b); - } - - // Should NOT trigger (already using checked_div) - if let Some(result) = b.checked_div(a) { - println!("{result}"); - } - - let c = -5i32; - if c != 0 { - //~^ manual_checked_div - let _result = 10_i32.checked_div(c); - } - - // Should NOT trigger (side effects in divisor) - if counter() > 0 { - let _ = 32 / counter(); - } - - // Should NOT trigger (divisor used before division) - if b > 0 { - use_value(b); - let _ = a / b; - } - - let signed_min = i32::MIN; - let mut signed_div: i32 = -1; - - // Should NOT trigger (would change behavior for MIN / -1) - if signed_div != 0 { - let _ = signed_min / signed_div; - } - - signed_div = 2; - - if signed_div > 0 { - //~^ manual_checked_div - let _ = signed_min.checked_div(signed_div); - } - - // Should NOT trigger (divisor may change during evaluation) - if b > 0 { - g(inc_and_return_value(&mut b), a / b); - } -} - -fn counter() -> u32 { - println!("counter"); - 1 -} - -fn use_value(_v: u32) {} - -fn inc_and_return_value(x: &mut u32) -> u32 { - *x += 1; - *x -} - -fn g(_lhs: u32, _rhs: u32) {} diff --git a/tests/ui/manual_checked_div.rs b/tests/ui/manual_checked_div.rs index 7a0b7d06d745..a7282c255c3c 100644 --- a/tests/ui/manual_checked_div.rs +++ b/tests/ui/manual_checked_div.rs @@ -28,9 +28,9 @@ fn main() { println!("{result}"); } + // Should NOT trigger (signed integers are not linted) let c = -5i32; if c != 0 { - //~^ manual_checked_div let _result = 10 / c; } @@ -48,7 +48,7 @@ fn main() { let signed_min = i32::MIN; let mut signed_div: i32 = -1; - // Should NOT trigger (would change behavior for MIN / -1) + // Should NOT trigger (signed integers are not linted) if signed_div != 0 { let _ = signed_min / signed_div; } @@ -56,7 +56,6 @@ fn main() { signed_div = 2; if signed_div > 0 { - //~^ manual_checked_div let _ = signed_min / signed_div; } @@ -79,3 +78,16 @@ fn inc_and_return_value(x: &mut u32) -> u32 { } fn g(_lhs: u32, _rhs: u32) {} + +fn arbitrary_signed(lhs: i32, rhs: i32) -> i32 { + if rhs != 0 { lhs / rhs } else { lhs } +} + +fn arbitrary_unsigned(lhs: u32, rhs: u32) -> u32 { + if rhs != 0 { + //~^ manual_checked_div + lhs / rhs + } else { + lhs + } +} diff --git a/tests/ui/manual_checked_div.stderr b/tests/ui/manual_checked_div.stderr index ef1f6bb4c070..e69c6ee40e40 100644 --- a/tests/ui/manual_checked_div.stderr +++ b/tests/ui/manual_checked_div.stderr @@ -9,13 +9,9 @@ LL | let _result = a / b; LL | let _another = (a + 1) / b; | ^^^^^^^^^^^ | + = help: consider using `checked_div` and handling the `Option` result = note: `-D clippy::manual-checked-div` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::manual_checked_div)]` -help: consider using `checked_div` - | -LL ~ let _result = a.checked_div(b); -LL ~ let _another = (a + 1).checked_div(b); - | error: manual checked division --> tests/ui/manual_checked_div.rs:14:8 @@ -24,7 +20,9 @@ LL | if b > 0 { | ^^^^^ LL | LL | let _result = a / b; - | ^^^^^ help: consider using `checked_div`: `a.checked_div(b)` + | ^^^^^ + | + = help: consider using `checked_div` and handling the `Option` result error: manual checked division --> tests/ui/manual_checked_div.rs:19:8 @@ -33,25 +31,20 @@ LL | if b == 0 { | ^^^^^^ ... LL | let _result = a / b; - | ^^^^^ help: consider using `checked_div`: `a.checked_div(b)` - -error: manual checked division - --> tests/ui/manual_checked_div.rs:32:8 + | ^^^^^ | -LL | if c != 0 { - | ^^^^^^ -LL | -LL | let _result = 10 / c; - | ^^^^^^ help: consider using `checked_div`: `10_i32.checked_div(c)` + = help: consider using `checked_div` and handling the `Option` result error: manual checked division - --> tests/ui/manual_checked_div.rs:58:8 + --> tests/ui/manual_checked_div.rs:87:8 | -LL | if signed_div > 0 { - | ^^^^^^^^^^^^^^ +LL | if rhs != 0 { + | ^^^^^^^^ LL | -LL | let _ = signed_min / signed_div; - | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `checked_div`: `signed_min.checked_div(signed_div)` +LL | lhs / rhs + | ^^^^^^^^^ + | + = help: consider using `checked_div` and handling the `Option` result -error: aborting due to 5 previous errors +error: aborting due to 4 previous errors From ce3048c51f03b67da4a078edf3fca1d5f66f1242 Mon Sep 17 00:00:00 2001 From: Amerikrainian Date: Fri, 19 Dec 2025 10:14:10 -0600 Subject: [PATCH 8/9] Fix clippy error --- clippy_lints/src/manual_checked_div.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/manual_checked_div.rs b/clippy_lints/src/manual_checked_div.rs index ba1b89af7290..0482ccf6fd8d 100644 --- a/clippy_lints/src/manual_checked_div.rs +++ b/clippy_lints/src/manual_checked_div.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::visitors::{Descend, for_each_expr_without_closures}; use clippy_utils::{SpanlessEq, is_integer_literal}; use rustc_errors::MultiSpan; @@ -97,14 +97,13 @@ impl LateLintPass<'_> for ManualCheckedDiv { } division_spans.push(cond.span); - span_lint_and_then( + span_lint_and_help( cx, MANUAL_CHECKED_DIV, MultiSpan::from_spans(division_spans), "manual checked division", - |diag| { - diag.help("consider using `checked_div` and handling the `Option` result"); - }, + None, + "consider using `checked_div` and handling the `Option` result", ); } } From c9dc19d908dcc1e67ae338307fa51529181045a3 Mon Sep 17 00:00:00 2001 From: Amerikrainian Date: Fri, 19 Dec 2025 10:32:52 -0600 Subject: [PATCH 9/9] Clean up tests and error message --- clippy_lints/src/manual_checked_div.rs | 2 +- tests/ui/manual_checked_div.rs | 13 ------------- tests/ui/manual_checked_div.stderr | 19 ++++--------------- 3 files changed, 5 insertions(+), 29 deletions(-) diff --git a/clippy_lints/src/manual_checked_div.rs b/clippy_lints/src/manual_checked_div.rs index 0482ccf6fd8d..427fa9417565 100644 --- a/clippy_lints/src/manual_checked_div.rs +++ b/clippy_lints/src/manual_checked_div.rs @@ -103,7 +103,7 @@ impl LateLintPass<'_> for ManualCheckedDiv { MultiSpan::from_spans(division_spans), "manual checked division", None, - "consider using `checked_div` and handling the `Option` result", + "consider using `checked_div`", ); } } diff --git a/tests/ui/manual_checked_div.rs b/tests/ui/manual_checked_div.rs index a7282c255c3c..d22c5c9f2360 100644 --- a/tests/ui/manual_checked_div.rs +++ b/tests/ui/manual_checked_div.rs @@ -78,16 +78,3 @@ fn inc_and_return_value(x: &mut u32) -> u32 { } fn g(_lhs: u32, _rhs: u32) {} - -fn arbitrary_signed(lhs: i32, rhs: i32) -> i32 { - if rhs != 0 { lhs / rhs } else { lhs } -} - -fn arbitrary_unsigned(lhs: u32, rhs: u32) -> u32 { - if rhs != 0 { - //~^ manual_checked_div - lhs / rhs - } else { - lhs - } -} diff --git a/tests/ui/manual_checked_div.stderr b/tests/ui/manual_checked_div.stderr index e69c6ee40e40..24bdd5078b5d 100644 --- a/tests/ui/manual_checked_div.stderr +++ b/tests/ui/manual_checked_div.stderr @@ -9,7 +9,7 @@ LL | let _result = a / b; LL | let _another = (a + 1) / b; | ^^^^^^^^^^^ | - = help: consider using `checked_div` and handling the `Option` result + = help: consider using `checked_div` = note: `-D clippy::manual-checked-div` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::manual_checked_div)]` @@ -22,7 +22,7 @@ LL | LL | let _result = a / b; | ^^^^^ | - = help: consider using `checked_div` and handling the `Option` result + = help: consider using `checked_div` error: manual checked division --> tests/ui/manual_checked_div.rs:19:8 @@ -33,18 +33,7 @@ LL | if b == 0 { LL | let _result = a / b; | ^^^^^ | - = help: consider using `checked_div` and handling the `Option` result + = help: consider using `checked_div` -error: manual checked division - --> tests/ui/manual_checked_div.rs:87:8 - | -LL | if rhs != 0 { - | ^^^^^^^^ -LL | -LL | lhs / rhs - | ^^^^^^^^^ - | - = help: consider using `checked_div` and handling the `Option` result - -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors