New lint: manual_is_multiple_of

Author: samueltardieu

CHANGELOG.md

@@ -5863,6 +5863,7 @@ Released 2018-09-13
 [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check
 [`manual_is_finite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_finite
 [`manual_is_infinite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_infinite
+[`manual_is_multiple_of`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_multiple_of
 [`manual_is_power_of_two`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_power_of_two
 [`manual_is_variant_and`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_variant_and
 [`manual_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else

clippy_lints/src/casts/cast_sign_loss.rs

@@ -167,7 +167,7 @@ fn pow_call_result_sign(cx: &LateContext<'_>, base: &Expr<'_>, exponent: &Expr<'
 
     // Rust's integer pow() functions take an unsigned exponent.
     let exponent_val = get_const_unsigned_int_eval(cx, exponent, None);
-    let exponent_is_even = exponent_val.map(|val| val % 2 == 0);
+    let exponent_is_even = exponent_val.map(|val| val.is_multiple_of(2));
 
     match (base_sign, exponent_is_even) {
         // Non-negative bases always return non-negative results, ignoring overflow.

clippy_lints/src/declared_lints.rs

@@ -583,6 +583,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::operators::IMPOSSIBLE_COMPARISONS_INFO,
     crate::operators::INEFFECTIVE_BIT_MASK_INFO,
     crate::operators::INTEGER_DIVISION_INFO,
+    crate::operators::MANUAL_IS_MULTIPLE_OF_INFO,
     crate::operators::MANUAL_MIDPOINT_INFO,
     crate::operators::MISREFACTORED_ASSIGN_OP_INFO,
     crate::operators::MODULO_ARITHMETIC_INFO,

clippy_lints/src/operators/manual_is_multiple_of.rs

@@ -0,0 +1,66 @@
+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 rustc_ast::BinOpKind;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_middle::ty;
+
+use super::MANUAL_IS_MULTIPLE_OF;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &Expr<'_>,
+    op: BinOpKind,
+    lhs: &'tcx Expr<'tcx>,
+    rhs: &'tcx Expr<'tcx>,
+    msrv: Msrv,
+) {
+    if msrv.meets(cx, msrvs::UNSIGNED_IS_MULTIPLE_OF)
+        && 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
+    {
+        let mut app = Applicability::MachineApplicable;
+        let divisor = Sugg::hir_with_applicability(cx, operand_right, "_", &mut app);
+        span_lint_and_sugg(
+            cx,
+            MANUAL_IS_MULTIPLE_OF,
+            expr.span,
+            "manual implementation of `.is_multiple_of()`",
+            "replace with",
+            format!(
+                "{}{}.is_multiple_of({divisor})",
+                if op == BinOpKind::Eq { "" } else { "!" },
+                Sugg::hir_with_applicability(cx, operand_left, "_", &mut app).maybe_paren()
+            ),
+            app,
+        );
+    }
+}
+
+// If we have a `x == 0`, `x != 0` or `x > 0` (or the reverted ones), return the non-zero operand
+fn uint_compare_to_zero<'tcx>(
+    cx: &LateContext<'tcx>,
+    op: BinOpKind,
+    lhs: &'tcx Expr<'tcx>,
+    rhs: &'tcx Expr<'tcx>,
+) -> Option<&'tcx Expr<'tcx>> {
+    let operand = if matches!(lhs.kind, ExprKind::Binary(..))
+        && matches!(op, BinOpKind::Eq | BinOpKind::Ne | BinOpKind::Gt)
+        && is_zero_integer_const(cx, rhs)
+    {
+        lhs
+    } else if matches!(rhs.kind, ExprKind::Binary(..))
+        && matches!(op, BinOpKind::Eq | BinOpKind::Ne | BinOpKind::Lt)
+        && is_zero_integer_const(cx, lhs)
+    {
+        rhs
+    } else {
+        return None;
+    };
+
+    matches!(cx.typeck_results().expr_ty_adjusted(operand).kind(), ty::Uint(_)).then_some(operand)
+}

clippy_lints/src/operators/mod.rs

@@ -11,6 +11,7 @@ mod float_cmp;
 mod float_equality_without_abs;
 mod identity_op;
 mod integer_division;
+mod manual_is_multiple_of;
 mod manual_midpoint;
 mod misrefactored_assign_op;
 mod modulo_arithmetic;
@@ -830,12 +831,42 @@ declare_clippy_lint! {
     "manual implementation of `midpoint` which can overflow"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for manual implementation of `.is_multiple_of()` on
+    /// unsigned integer types.
+    ///
+    /// ### Why is this bad?
+    /// `a.is_multiple_of(b)` is a clearer way to check for divisibility
+    /// of `a` by `b`. This expression can never panic.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # let (a, b) = (3u64, 4u64);
+    /// if a % b == 0 {
+    ///     println!("{a} is divisible by {b}");
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # let (a, b) = (3u64, 4u64);
+    /// if a.is_multiple_of(b) {
+    ///     println!("{a} is divisible by {b}");
+    /// }
+    /// ```
+    #[clippy::version = "1.88.0"]
+    pub MANUAL_IS_MULTIPLE_OF,
+    complexity,
+    "manual implementation of `.is_multiple_of()`"
+}
+
 pub struct Operators {
     arithmetic_context: numeric_arithmetic::Context,
     verbose_bit_mask_threshold: u64,
     modulo_arithmetic_allow_comparison_to_zero: bool,
     msrv: Msrv,
 }
+
 impl Operators {
     pub fn new(conf: &'static Conf) -> Self {
         Self {
@@ -874,6 +905,7 @@ impl_lint_pass!(Operators => [
     NEEDLESS_BITWISE_BOOL,
     SELF_ASSIGNMENT,
     MANUAL_MIDPOINT,
+    MANUAL_IS_MULTIPLE_OF,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Operators {
@@ -891,6 +923,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
                     identity_op::check(cx, e, op.node, lhs, rhs);
                     needless_bitwise_bool::check(cx, e, op.node, lhs, rhs);
                     manual_midpoint::check(cx, e, op.node, lhs, rhs, self.msrv);
+                    manual_is_multiple_of::check(cx, e, op.node, lhs, rhs, self.msrv);
                 }
                 self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);
                 bit_mask::check(cx, e, op.node, lhs, rhs);

clippy_utils/src/msrvs.rs

@@ -23,7 +23,7 @@ macro_rules! msrv_aliases {
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
     1,88,0 { LET_CHAINS }
-    1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT }
+    1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT, UNSIGNED_IS_MULTIPLE_OF }
     1,85,0 { UINT_FLOAT_MIDPOINT }
     1,84,0 { CONST_OPTION_AS_SLICE, MANUAL_DANGLING_PTR }
     1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, CONST_MUT_REFS, CONST_UNWRAP }

tests/ui/manual_is_multiple_of.fixed

@@ -0,0 +1,25 @@
+//@aux-build: proc_macros.rs
+#![warn(clippy::manual_is_multiple_of)]
+
+fn main() {}
+
+#[clippy::msrv = "1.87"]
+fn f(a: u64, b: u64) {
+    let _ = a.is_multiple_of(b); //~ manual_is_multiple_of
+    let _ = (a + 1).is_multiple_of(b + 1); //~ manual_is_multiple_of
+    let _ = !a.is_multiple_of(b); //~ manual_is_multiple_of
+    let _ = !(a + 1).is_multiple_of(b + 1); //~ manual_is_multiple_of
+
+    let _ = !a.is_multiple_of(b); //~ manual_is_multiple_of
+    let _ = !a.is_multiple_of(b); //~ manual_is_multiple_of
+
+    proc_macros::external! {
+        let a: u64 = 23424;
+        let _ = a % 4096 == 0;
+    }
+}
+
+#[clippy::msrv = "1.86"]
+fn g(a: u64, b: u64) {
+    let _ = a % b == 0;
+}

tests/ui/manual_is_multiple_of.rs

@@ -0,0 +1,25 @@
+//@aux-build: proc_macros.rs
+#![warn(clippy::manual_is_multiple_of)]
+
+fn main() {}
+
+#[clippy::msrv = "1.87"]
+fn f(a: u64, b: u64) {
+    let _ = a % b == 0; //~ manual_is_multiple_of
+    let _ = (a + 1) % (b + 1) == 0; //~ manual_is_multiple_of
+    let _ = a % b != 0; //~ manual_is_multiple_of
+    let _ = (a + 1) % (b + 1) != 0; //~ manual_is_multiple_of
+
+    let _ = a % b > 0; //~ manual_is_multiple_of
+    let _ = 0 < a % b; //~ manual_is_multiple_of
+
+    proc_macros::external! {
+        let a: u64 = 23424;
+        let _ = a % 4096 == 0;
+    }
+}
+
+#[clippy::msrv = "1.86"]
+fn g(a: u64, b: u64) {
+    let _ = a % b == 0;
+}

tests/ui/manual_is_multiple_of.stderr

@@ -0,0 +1,41 @@
+error: manual implementation of `.is_multiple_of()`
+  --> tests/ui/manual_is_multiple_of.rs:8:13
+   |
+LL |     let _ = a % b == 0;
+   |             ^^^^^^^^^^ help: replace with: `a.is_multiple_of(b)`
+   |
+   = note: `-D clippy::manual-is-multiple-of` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::manual_is_multiple_of)]`
+
+error: manual implementation of `.is_multiple_of()`
+  --> tests/ui/manual_is_multiple_of.rs:9:13
+   |
+LL |     let _ = (a + 1) % (b + 1) == 0;
+   |             ^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `(a + 1).is_multiple_of(b + 1)`
+
+error: manual implementation of `.is_multiple_of()`
+  --> tests/ui/manual_is_multiple_of.rs:10: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:11:13
+   |
+LL |     let _ = (a + 1) % (b + 1) != 0;
+   |             ^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `!(a + 1).is_multiple_of(b + 1)`
+
+error: manual implementation of `.is_multiple_of()`
+  --> tests/ui/manual_is_multiple_of.rs:13: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:14:13
+   |
+LL |     let _ = 0 < a % b;
+   |             ^^^^^^^^^ help: replace with: `!a.is_multiple_of(b)`
+
+error: aborting due to 6 previous errors
+