about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel Tardieu <sam@rfc1149.net>2025-07-03 20:22:18 +0200
committerSamuel Tardieu <sam@rfc1149.net>2025-07-03 22:14:00 +0200
commita8ab02c4641de4e3ec634b3479fddb60e4ad7763 (patch)
treea6ec346ca3f015191e64a460af4c3c2f8e921f0c
parent83148cb28343bf15ed469f946915c73deeaa9a28 (diff)
downloadrust-a8ab02c4641de4e3ec634b3479fddb60e4ad7763.tar.gz
rust-a8ab02c4641de4e3ec634b3479fddb60e4ad7763.zip
Fix several issues with `manual_is_multiple_of`
- `&a % &b == 0` compiles, but requires dereferencing `b` when
  replacing with `a.is_multiple_of(b)`.
- In `a % b == 0`, if type of `a` is not certain,
  `a.is_multiple_of(b)` might not be typable.
- In `a % b == 0`, `a` and `b` must be unsigned integers, not any
  arbitrary types implementing `Rem` and outputing an integer.
-rw-r--r--clippy_lints/src/operators/manual_is_multiple_of.rs25
-rw-r--r--tests/ui/manual_is_multiple_of.fixed78
-rw-r--r--tests/ui/manual_is_multiple_of.rs78
-rw-r--r--tests/ui/manual_is_multiple_of.stderr32
4 files changed, 210 insertions, 3 deletions
diff --git a/clippy_lints/src/operators/manual_is_multiple_of.rs b/clippy_lints/src/operators/manual_is_multiple_of.rs
index 821178a4315..55bb78cfce5 100644
--- a/clippy_lints/src/operators/manual_is_multiple_of.rs
+++ b/clippy_lints/src/operators/manual_is_multiple_of.rs
@@ -2,11 +2,12 @@ use clippy_utils::consts::is_zero_integer_const;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::msrvs::{self, Msrv};
 use clippy_utils::sugg::Sugg;
+use clippy_utils::ty::expr_type_is_certain;
 use rustc_ast::BinOpKind;
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, ExprKind};
 use rustc_lint::LateContext;
-use rustc_middle::ty;
+use rustc_middle::ty::{self, Ty};
 
 use super::MANUAL_IS_MULTIPLE_OF;
 
@@ -22,9 +23,21 @@ pub(super) fn check<'tcx>(
         && let Some(operand) = uint_compare_to_zero(cx, op, lhs, rhs)
         && let ExprKind::Binary(operand_op, operand_left, operand_right) = operand.kind
         && operand_op.node == BinOpKind::Rem
+        && matches!(
+            cx.typeck_results().expr_ty_adjusted(operand_left).peel_refs().kind(),
+            ty::Uint(_)
+        )
+        && matches!(
+            cx.typeck_results().expr_ty_adjusted(operand_right).peel_refs().kind(),
+            ty::Uint(_)
+        )
+        && expr_type_is_certain(cx, operand_left)
     {
         let mut app = Applicability::MachineApplicable;
-        let divisor = Sugg::hir_with_applicability(cx, operand_right, "_", &mut app);
+        let divisor = deref_sugg(
+            Sugg::hir_with_applicability(cx, operand_right, "_", &mut app),
+            cx.typeck_results().expr_ty_adjusted(operand_right),
+        );
         span_lint_and_sugg(
             cx,
             MANUAL_IS_MULTIPLE_OF,
@@ -64,3 +77,11 @@ fn uint_compare_to_zero<'tcx>(
 
     matches!(cx.typeck_results().expr_ty_adjusted(operand).kind(), ty::Uint(_)).then_some(operand)
 }
+
+fn deref_sugg<'a>(sugg: Sugg<'a>, ty: Ty<'_>) -> Sugg<'a> {
+    if let ty::Ref(_, target_ty, _) = ty.kind() {
+        deref_sugg(sugg.deref(), *target_ty)
+    } else {
+        sugg
+    }
+}
diff --git a/tests/ui/manual_is_multiple_of.fixed b/tests/ui/manual_is_multiple_of.fixed
index 6735b99f298..03f75e725ed 100644
--- a/tests/ui/manual_is_multiple_of.fixed
+++ b/tests/ui/manual_is_multiple_of.fixed
@@ -23,3 +23,81 @@ fn f(a: u64, b: u64) {
 fn g(a: u64, b: u64) {
     let _ = a % b == 0;
 }
+
+fn needs_deref(a: &u64, b: &u64) {
+    let _ = a.is_multiple_of(*b); //~ manual_is_multiple_of
+}
+
+fn closures(a: u64, b: u64) {
+    // Do not lint, types are ambiguous at this point
+    let cl = |a, b| a % b == 0;
+    let _ = cl(a, b);
+
+    // Do not lint, types are ambiguous at this point
+    let cl = |a: _, b: _| a % b == 0;
+    let _ = cl(a, b);
+
+    // Type of `a` is enough
+    let cl = |a: u64, b| a.is_multiple_of(b); //~ manual_is_multiple_of
+    let _ = cl(a, b);
+
+    // Type of `a` is enough
+    let cl = |a: &u64, b| a.is_multiple_of(b); //~ manual_is_multiple_of
+    let _ = cl(&a, b);
+
+    // Type of `b` is not enough
+    let cl = |a, b: u64| a % b == 0;
+    let _ = cl(&a, b);
+}
+
+fn any_rem<T: std::ops::Rem<Output = u32>>(a: T, b: T) {
+    // An arbitrary `Rem` implementation should not lint
+    let _ = a % b == 0;
+}
+
+mod issue15103 {
+    fn foo() -> Option<u64> {
+        let mut n: u64 = 150_000_000;
+
+        (2..).find(|p| {
+            while n.is_multiple_of(*p) {
+                //~^ manual_is_multiple_of
+                n /= p;
+            }
+            n <= 1
+        })
+    }
+
+    const fn generate_primes<const N: usize>() -> [u64; N] {
+        let mut result = [0; N];
+        if N == 0 {
+            return result;
+        }
+        result[0] = 2;
+        if N == 1 {
+            return result;
+        }
+        let mut idx = 1;
+        let mut p = 3;
+        while idx < N {
+            let mut j = 0;
+            while j < idx && p % result[j] != 0 {
+                j += 1;
+            }
+            if j == idx {
+                result[idx] = p;
+                idx += 1;
+            }
+            p += 1;
+        }
+        result
+    }
+
+    fn bar() -> u32 {
+        let d = |n: u32| -> u32 { (1..=n / 2).filter(|i| n.is_multiple_of(*i)).sum() };
+        //~^ manual_is_multiple_of
+
+        let d = |n| (1..=n / 2).filter(|i| n % i == 0).sum();
+        (1..1_000).filter(|&i| i == d(d(i)) && i != d(i)).sum()
+    }
+}
diff --git a/tests/ui/manual_is_multiple_of.rs b/tests/ui/manual_is_multiple_of.rs
index 00b638e4fd9..7b6fa64c843 100644
--- a/tests/ui/manual_is_multiple_of.rs
+++ b/tests/ui/manual_is_multiple_of.rs
@@ -23,3 +23,81 @@ fn f(a: u64, b: u64) {
 fn g(a: u64, b: u64) {
     let _ = a % b == 0;
 }
+
+fn needs_deref(a: &u64, b: &u64) {
+    let _ = a % b == 0; //~ manual_is_multiple_of
+}
+
+fn closures(a: u64, b: u64) {
+    // Do not lint, types are ambiguous at this point
+    let cl = |a, b| a % b == 0;
+    let _ = cl(a, b);
+
+    // Do not lint, types are ambiguous at this point
+    let cl = |a: _, b: _| a % b == 0;
+    let _ = cl(a, b);
+
+    // Type of `a` is enough
+    let cl = |a: u64, b| a % b == 0; //~ manual_is_multiple_of
+    let _ = cl(a, b);
+
+    // Type of `a` is enough
+    let cl = |a: &u64, b| a % b == 0; //~ manual_is_multiple_of
+    let _ = cl(&a, b);
+
+    // Type of `b` is not enough
+    let cl = |a, b: u64| a % b == 0;
+    let _ = cl(&a, b);
+}
+
+fn any_rem<T: std::ops::Rem<Output = u32>>(a: T, b: T) {
+    // An arbitrary `Rem` implementation should not lint
+    let _ = a % b == 0;
+}
+
+mod issue15103 {
+    fn foo() -> Option<u64> {
+        let mut n: u64 = 150_000_000;
+
+        (2..).find(|p| {
+            while n % p == 0 {
+                //~^ manual_is_multiple_of
+                n /= p;
+            }
+            n <= 1
+        })
+    }
+
+    const fn generate_primes<const N: usize>() -> [u64; N] {
+        let mut result = [0; N];
+        if N == 0 {
+            return result;
+        }
+        result[0] = 2;
+        if N == 1 {
+            return result;
+        }
+        let mut idx = 1;
+        let mut p = 3;
+        while idx < N {
+            let mut j = 0;
+            while j < idx && p % result[j] != 0 {
+                j += 1;
+            }
+            if j == idx {
+                result[idx] = p;
+                idx += 1;
+            }
+            p += 1;
+        }
+        result
+    }
+
+    fn bar() -> u32 {
+        let d = |n: u32| -> u32 { (1..=n / 2).filter(|i| n % i == 0).sum() };
+        //~^ manual_is_multiple_of
+
+        let d = |n| (1..=n / 2).filter(|i| n % i == 0).sum();
+        (1..1_000).filter(|&i| i == d(d(i)) && i != d(i)).sum()
+    }
+}
diff --git a/tests/ui/manual_is_multiple_of.stderr b/tests/ui/manual_is_multiple_of.stderr
index 0b1ae70c2a7..8523599ec40 100644
--- a/tests/ui/manual_is_multiple_of.stderr
+++ b/tests/ui/manual_is_multiple_of.stderr
@@ -37,5 +37,35 @@ error: manual implementation of `.is_multiple_of()`
 LL |     let _ = 0 < a % b;
    |             ^^^^^^^^^ help: replace with: `!a.is_multiple_of(b)`
 
-error: aborting due to 6 previous errors
+error: manual implementation of `.is_multiple_of()`
+  --> tests/ui/manual_is_multiple_of.rs:28:13
+   |
+LL |     let _ = a % b == 0;
+   |             ^^^^^^^^^^ help: replace with: `a.is_multiple_of(*b)`
+
+error: manual implementation of `.is_multiple_of()`
+  --> tests/ui/manual_is_multiple_of.rs:41:26
+   |
+LL |     let cl = |a: u64, b| a % b == 0;
+   |                          ^^^^^^^^^^ help: replace with: `a.is_multiple_of(b)`
+
+error: manual implementation of `.is_multiple_of()`
+  --> tests/ui/manual_is_multiple_of.rs:45:27
+   |
+LL |     let cl = |a: &u64, b| a % b == 0;
+   |                           ^^^^^^^^^^ help: replace with: `a.is_multiple_of(b)`
+
+error: manual implementation of `.is_multiple_of()`
+  --> tests/ui/manual_is_multiple_of.rs:63:19
+   |
+LL |             while n % p == 0 {
+   |                   ^^^^^^^^^^ help: replace with: `n.is_multiple_of(*p)`
+
+error: manual implementation of `.is_multiple_of()`
+  --> tests/ui/manual_is_multiple_of.rs:97:58
+   |
+LL |         let d = |n: u32| -> u32 { (1..=n / 2).filter(|i| n % i == 0).sum() };
+   |                                                          ^^^^^^^^^^ help: replace with: `n.is_multiple_of(*i)`
+
+error: aborting due to 11 previous errors