Skip to content

Commit a8ab02c

Browse files
committed
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.
1 parent 83148cb commit a8ab02c

File tree

4 files changed

+210
-3
lines changed

4 files changed

+210
-3
lines changed

clippy_lints/src/operators/manual_is_multiple_of.rs

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@ use clippy_utils::consts::is_zero_integer_const;
22
use clippy_utils::diagnostics::span_lint_and_sugg;
33
use clippy_utils::msrvs::{self, Msrv};
44
use clippy_utils::sugg::Sugg;
5+
use clippy_utils::ty::expr_type_is_certain;
56
use rustc_ast::BinOpKind;
67
use rustc_errors::Applicability;
78
use rustc_hir::{Expr, ExprKind};
89
use rustc_lint::LateContext;
9-
use rustc_middle::ty;
10+
use rustc_middle::ty::{self, Ty};
1011

1112
use super::MANUAL_IS_MULTIPLE_OF;
1213

@@ -22,9 +23,21 @@ pub(super) fn check<'tcx>(
2223
&& let Some(operand) = uint_compare_to_zero(cx, op, lhs, rhs)
2324
&& let ExprKind::Binary(operand_op, operand_left, operand_right) = operand.kind
2425
&& operand_op.node == BinOpKind::Rem
26+
&& matches!(
27+
cx.typeck_results().expr_ty_adjusted(operand_left).peel_refs().kind(),
28+
ty::Uint(_)
29+
)
30+
&& matches!(
31+
cx.typeck_results().expr_ty_adjusted(operand_right).peel_refs().kind(),
32+
ty::Uint(_)
33+
)
34+
&& expr_type_is_certain(cx, operand_left)
2535
{
2636
let mut app = Applicability::MachineApplicable;
27-
let divisor = Sugg::hir_with_applicability(cx, operand_right, "_", &mut app);
37+
let divisor = deref_sugg(
38+
Sugg::hir_with_applicability(cx, operand_right, "_", &mut app),
39+
cx.typeck_results().expr_ty_adjusted(operand_right),
40+
);
2841
span_lint_and_sugg(
2942
cx,
3043
MANUAL_IS_MULTIPLE_OF,
@@ -64,3 +77,11 @@ fn uint_compare_to_zero<'tcx>(
6477

6578
matches!(cx.typeck_results().expr_ty_adjusted(operand).kind(), ty::Uint(_)).then_some(operand)
6679
}
80+
81+
fn deref_sugg<'a>(sugg: Sugg<'a>, ty: Ty<'_>) -> Sugg<'a> {
82+
if let ty::Ref(_, target_ty, _) = ty.kind() {
83+
deref_sugg(sugg.deref(), *target_ty)
84+
} else {
85+
sugg
86+
}
87+
}

tests/ui/manual_is_multiple_of.fixed

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,81 @@ fn f(a: u64, b: u64) {
2323
fn g(a: u64, b: u64) {
2424
let _ = a % b == 0;
2525
}
26+
27+
fn needs_deref(a: &u64, b: &u64) {
28+
let _ = a.is_multiple_of(*b); //~ manual_is_multiple_of
29+
}
30+
31+
fn closures(a: u64, b: u64) {
32+
// Do not lint, types are ambiguous at this point
33+
let cl = |a, b| a % b == 0;
34+
let _ = cl(a, b);
35+
36+
// Do not lint, types are ambiguous at this point
37+
let cl = |a: _, b: _| a % b == 0;
38+
let _ = cl(a, b);
39+
40+
// Type of `a` is enough
41+
let cl = |a: u64, b| a.is_multiple_of(b); //~ manual_is_multiple_of
42+
let _ = cl(a, b);
43+
44+
// Type of `a` is enough
45+
let cl = |a: &u64, b| a.is_multiple_of(b); //~ manual_is_multiple_of
46+
let _ = cl(&a, b);
47+
48+
// Type of `b` is not enough
49+
let cl = |a, b: u64| a % b == 0;
50+
let _ = cl(&a, b);
51+
}
52+
53+
fn any_rem<T: std::ops::Rem<Output = u32>>(a: T, b: T) {
54+
// An arbitrary `Rem` implementation should not lint
55+
let _ = a % b == 0;
56+
}
57+
58+
mod issue15103 {
59+
fn foo() -> Option<u64> {
60+
let mut n: u64 = 150_000_000;
61+
62+
(2..).find(|p| {
63+
while n.is_multiple_of(*p) {
64+
//~^ manual_is_multiple_of
65+
n /= p;
66+
}
67+
n <= 1
68+
})
69+
}
70+
71+
const fn generate_primes<const N: usize>() -> [u64; N] {
72+
let mut result = [0; N];
73+
if N == 0 {
74+
return result;
75+
}
76+
result[0] = 2;
77+
if N == 1 {
78+
return result;
79+
}
80+
let mut idx = 1;
81+
let mut p = 3;
82+
while idx < N {
83+
let mut j = 0;
84+
while j < idx && p % result[j] != 0 {
85+
j += 1;
86+
}
87+
if j == idx {
88+
result[idx] = p;
89+
idx += 1;
90+
}
91+
p += 1;
92+
}
93+
result
94+
}
95+
96+
fn bar() -> u32 {
97+
let d = |n: u32| -> u32 { (1..=n / 2).filter(|i| n.is_multiple_of(*i)).sum() };
98+
//~^ manual_is_multiple_of
99+
100+
let d = |n| (1..=n / 2).filter(|i| n % i == 0).sum();
101+
(1..1_000).filter(|&i| i == d(d(i)) && i != d(i)).sum()
102+
}
103+
}

tests/ui/manual_is_multiple_of.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,81 @@ fn f(a: u64, b: u64) {
2323
fn g(a: u64, b: u64) {
2424
let _ = a % b == 0;
2525
}
26+
27+
fn needs_deref(a: &u64, b: &u64) {
28+
let _ = a % b == 0; //~ manual_is_multiple_of
29+
}
30+
31+
fn closures(a: u64, b: u64) {
32+
// Do not lint, types are ambiguous at this point
33+
let cl = |a, b| a % b == 0;
34+
let _ = cl(a, b);
35+
36+
// Do not lint, types are ambiguous at this point
37+
let cl = |a: _, b: _| a % b == 0;
38+
let _ = cl(a, b);
39+
40+
// Type of `a` is enough
41+
let cl = |a: u64, b| a % b == 0; //~ manual_is_multiple_of
42+
let _ = cl(a, b);
43+
44+
// Type of `a` is enough
45+
let cl = |a: &u64, b| a % b == 0; //~ manual_is_multiple_of
46+
let _ = cl(&a, b);
47+
48+
// Type of `b` is not enough
49+
let cl = |a, b: u64| a % b == 0;
50+
let _ = cl(&a, b);
51+
}
52+
53+
fn any_rem<T: std::ops::Rem<Output = u32>>(a: T, b: T) {
54+
// An arbitrary `Rem` implementation should not lint
55+
let _ = a % b == 0;
56+
}
57+
58+
mod issue15103 {
59+
fn foo() -> Option<u64> {
60+
let mut n: u64 = 150_000_000;
61+
62+
(2..).find(|p| {
63+
while n % p == 0 {
64+
//~^ manual_is_multiple_of
65+
n /= p;
66+
}
67+
n <= 1
68+
})
69+
}
70+
71+
const fn generate_primes<const N: usize>() -> [u64; N] {
72+
let mut result = [0; N];
73+
if N == 0 {
74+
return result;
75+
}
76+
result[0] = 2;
77+
if N == 1 {
78+
return result;
79+
}
80+
let mut idx = 1;
81+
let mut p = 3;
82+
while idx < N {
83+
let mut j = 0;
84+
while j < idx && p % result[j] != 0 {
85+
j += 1;
86+
}
87+
if j == idx {
88+
result[idx] = p;
89+
idx += 1;
90+
}
91+
p += 1;
92+
}
93+
result
94+
}
95+
96+
fn bar() -> u32 {
97+
let d = |n: u32| -> u32 { (1..=n / 2).filter(|i| n % i == 0).sum() };
98+
//~^ manual_is_multiple_of
99+
100+
let d = |n| (1..=n / 2).filter(|i| n % i == 0).sum();
101+
(1..1_000).filter(|&i| i == d(d(i)) && i != d(i)).sum()
102+
}
103+
}

tests/ui/manual_is_multiple_of.stderr

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,5 +37,35 @@ error: manual implementation of `.is_multiple_of()`
3737
LL | let _ = 0 < a % b;
3838
| ^^^^^^^^^ help: replace with: `!a.is_multiple_of(b)`
3939

40-
error: aborting due to 6 previous errors
40+
error: manual implementation of `.is_multiple_of()`
41+
--> tests/ui/manual_is_multiple_of.rs:28:13
42+
|
43+
LL | let _ = a % b == 0;
44+
| ^^^^^^^^^^ help: replace with: `a.is_multiple_of(*b)`
45+
46+
error: manual implementation of `.is_multiple_of()`
47+
--> tests/ui/manual_is_multiple_of.rs:41:26
48+
|
49+
LL | let cl = |a: u64, b| a % b == 0;
50+
| ^^^^^^^^^^ help: replace with: `a.is_multiple_of(b)`
51+
52+
error: manual implementation of `.is_multiple_of()`
53+
--> tests/ui/manual_is_multiple_of.rs:45:27
54+
|
55+
LL | let cl = |a: &u64, b| a % b == 0;
56+
| ^^^^^^^^^^ help: replace with: `a.is_multiple_of(b)`
57+
58+
error: manual implementation of `.is_multiple_of()`
59+
--> tests/ui/manual_is_multiple_of.rs:63:19
60+
|
61+
LL | while n % p == 0 {
62+
| ^^^^^^^^^^ help: replace with: `n.is_multiple_of(*p)`
63+
64+
error: manual implementation of `.is_multiple_of()`
65+
--> tests/ui/manual_is_multiple_of.rs:97:58
66+
|
67+
LL | let d = |n: u32| -> u32 { (1..=n / 2).filter(|i| n % i == 0).sum() };
68+
| ^^^^^^^^^^ help: replace with: `n.is_multiple_of(*i)`
69+
70+
error: aborting due to 11 previous errors
4171

0 commit comments

Comments
 (0)