Add manual_checked_div lint #16149

amerikrainian · 2025-11-28T01:19:00Z

changelog: [manual_checked_div]: new lint suggesting checked_div instead of manual zero checks before unsigned division

Implements the manual checked div from #12894. I'm relatively new to Rust and a complete newbie at Clippy, so my apologies if I forgot anything. I looked through the linked issue and could not find anything talking about the above lint.

rustbot · 2025-11-28T01:19:05Z

r? @samueltardieu

rustbot has assigned @samueltardieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

github-actions · 2025-11-28T01:31:32Z

No changes for 428d901

ada4a

Extremely impressive for a "complete newbie at Clippy"! Left a couple starting comments

View changes since this review

ada4a · 2025-12-01T18:59:08Z

clippy_lints/src/manual_checked_div.rs

+        NonZeroBranch::Else => match r#else.map(|expr| &expr.kind) {
+            Some(ExprKind::Block(block, _)) => Some(block),
+            _ => None,
+        },


Suggested change

NonZeroBranch::Else => match r#else.map(|expr| &expr.kind) {

Some(ExprKind::Block(block, _)) => Some(block),

_ => None,

},

NonZeroBranch::Else => match r#else?.kind {

ExprKind::Block(block, _) => Some(block),

_ => None,

},

ada4a · 2025-12-01T19:00:02Z

clippy_lints/src/manual_checked_div.rs

+            let Some(block) = branch_block(then, r#else, branch) else {
+                return;
+            };


this could be moved into the let-chain as well

ada4a · 2025-12-01T19:00:24Z

clippy_lints/src/manual_checked_div.rs

+        if expr.span.from_expansion() {
+            return;
+        }


this could be moved into the let-chain as well

ada4a · 2025-12-01T19:01:47Z

clippy_lints/src/manual_checked_div.rs

+                    let lhs_snip = Sugg::hir_with_applicability(cx, lhs, "..", &mut applicability);
+                    let rhs_snip = Sugg::hir_with_applicability(cx, rhs, "..", &mut applicability);


For single expressions, the preferred placeholder is "_"

Suggested change

let lhs_snip = Sugg::hir_with_applicability(cx, lhs, "..", &mut applicability);

let rhs_snip = Sugg::hir_with_applicability(cx, rhs, "..", &mut applicability);

let lhs_snip = Sugg::hir_with_applicability(cx, lhs, "_", &mut applicability);

let rhs_snip = Sugg::hir_with_applicability(cx, rhs, "_", &mut applicability);

ada4a · 2025-12-01T19:03:38Z

clippy_lints/src/manual_checked_div.rs

+            let Some(block) = branch_block(then, r#else, branch) else {
+                return;
+            };
+            let mut eq = SpanlessEq::new(cx);


This is fine to inline

ada4a · 2025-12-01T19:13:03Z

clippy_lints/src/manual_checked_div.rs

+                    span_lint_and_sugg(
+                        cx,
+                        MANUAL_CHECKED_DIV,
+                        e.span,


It could be nice to also highlight the span where the variable (let's call it b) is checked for being non-zero, which you could do by passing MultiSpan::new(vec![cond.span, e.span]).

But with that, if there were multiple instances of a / b (however unlikely that is), each firing of the lint would show the said checking place, which would be suboptimal. Because of this, a better solution could be to first collect all the instances of a / b (where each one might have a different a!), then lint on all of them at once (if there are any, of course). Now that I've written all this, it does sound like a bit too much work -- if you want to give it a try, feel free, but otherwise just highlighting both the condition and the division would be a good start.

ada4a · 2025-12-01T19:15:21Z

tests/ui/manual_checked_div.rs

+    // Should NOT trigger (signed integers)
+    let c = -5i32;
+    if c != 0 {
+        let _result = 10 / c;
+    }


Sorry, why exactly do we not lint here?... I see that the original issue only talks about unsigned integers, but I think signed ones should be fine as well?

samueltardieu · 2025-12-15T13:13:10Z

@amerikrainian Could you look at, and probably apply, @ada4a's suggestions?
@rustbot author

rustbot · 2025-12-15T13:13:15Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

amerikrainian · 2025-12-15T13:18:11Z

@amerikrainian Could you look at, and probably apply, @ada4a's suggestions? @rustbot author

Yes. I'm sorry. I'm a student and we're in the finals season, so I am deep in the trenches until tomorrow; will do my best to get this done before Friday.

samueltardieu · 2025-12-15T13:20:16Z

No hurry, I just wanted to make sure you noticed them. Take your time.

rustbot · 2025-12-16T03:46:05Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

amerikrainian · 2025-12-16T05:00:21Z

Applied @ada4a’s suggestions and rebased.

I also went ahead and gave multiple divisions implementation a shot; it turned out to be rather straightforward, although I'm not sure if the current form is what was intended.

I promise future feedback won't take as long to be applied.

@rustbot ready

samueltardieu

This is a good initial shot. Some risks of false positives need to be fixed though.

As a general rule, dividend() / divisor() can be linted only if you can prove that divisor() is identical between the check and the division, and if divisor() has no side-effect. For example, the following code should not be linted as it would not give the same result:

fn counter() -> u32 { /* Some code which increments and return a counter */ }

if counter() > 0 {
    let a = 32 / counter();
    println!("{}", a);  // This divides by the value of the counter as incremented twice
}

Also, if the divisor is used separately from the division, it might not be easy to refactor with .checked_div(), as in:

    if b > 0 {
        do_something_with(b);
        f(a / b);
    }

Another, more tortuous, example, would be, with a mutable b:

    if b > 0 {
        g(inc_and_return_value(&mut b), a / b);
    }

In the latest example, the value of b may have changed while evaluating the first argument of g(), so computing the division first using .checked_div() might not be appropriate.

In short, I think you should trigger the lint only if the division is the first use of the divisor in the "then" part of the if, and if the divisor is an expression that cannot change between the check and the division (checking for the absence of function calls should be fine).

Also, it would be great if /= could be checked the same way. And maybe % and %=, with checked_rem(), since the same logic applies.

View changes since this review

amerikrainian · 2025-12-16T14:16:02Z

I'm happy to update this for the /=, %, and %= cases, I just wanted to ensure that this is what you had in mind. I updated the tests with the listed negative examples and enforced first use/purity of divisor expressions since I think it's more trickier and extension to other ops should be mechanical and straightforward. Is this what you intended?

@rustbot ready

Jarcho · 2025-12-16T14:20:18Z

clippy_lints/src/manual_checked_div.rs

+fn is_integer(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    matches!(
+        cx.typeck_results().expr_ty(expr).peel_refs().kind(),
+        ty::Uint(_) | ty::Int(_)
+    )
+}


You can't lint signed and unsigned values with the same values since MIN / -1 also returns None in checked_div.

So I gave this some thought, and we can make it so that for signed, only accept conditions of the form b > 0 or 0 < b (depending on which side zero is on). We would then skip linting if the condition is b != 0, b == 0, or anything else. Again, this is for signed. Is this what you had in mind? Any other cases I should be aware of?

The only additional case returning None is MIN / -1 so you'll have to check for that in addition to the rhs being zero. You can't change the meaning of the code for a lint like this if it's enabled by default.

rustbot assigned samueltardieu Nov 28, 2025

rustbot added needs-fcp S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Nov 28, 2025

ada4a suggested changes Dec 1, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

gauravagerwala added a commit to gauravagerwala/rust-clippy that referenced this pull request Dec 7, 2025

Update design for PR rust-lang#16149: Add manual_checked_div lint

41eb86b

rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 15, 2025

amerikrainian added 3 commits December 15, 2025 21:44

Add manual checked div lint

bea60f9

Drop manual_checked_div changelog edit

34c252c

Run cargo dev update_lints

1316c0e

amerikrainian force-pushed the manual-checked-div branch from bf07325 to 1316c0e Compare December 16, 2025 03:46

Make checked div work on signed ints and collect multiple divisions

898e816

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Dec 16, 2025

samueltardieu requested changes Dec 16, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 16, 2025

Enforced divisor purity and first use

428d901

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Dec 16, 2025

Jarcho reviewed Dec 16, 2025

View reviewed changes

		let lhs_snip = Sugg::hir_with_applicability(cx, lhs, "..", &mut applicability);
		let rhs_snip = Sugg::hir_with_applicability(cx, rhs, "..", &mut applicability);

Add manual_checked_div lint #16149

Are you sure you want to change the base?

Add manual_checked_div lint #16149

Conversation

amerikrainian commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Nov 28, 2025

Uh oh!

github-actions bot commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ada4a left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

samueltardieu commented Dec 15, 2025

Uh oh!

rustbot commented Dec 15, 2025

Uh oh!

amerikrainian commented Dec 15, 2025

Uh oh!

samueltardieu commented Dec 15, 2025

Uh oh!

rustbot commented Dec 16, 2025

Uh oh!

amerikrainian commented Dec 16, 2025

Uh oh!

samueltardieu left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amerikrainian commented Dec 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

amerikrainian commented Nov 28, 2025 •

edited

Loading

github-actions bot commented Nov 28, 2025 •

edited

Loading

ada4a left a comment •

edited by rustbot

Loading

samueltardieu left a comment •

edited by rustbot

Loading