New lint: manual_is_multiple_of

CHANGELOG.md

@@ -5800,6 +5800,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
@@ -6365,6 +6366,7 @@ Released 2018-09-13
 [`max-struct-bools`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-struct-bools
 [`max-suggested-slice-pattern-length`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-suggested-slice-pattern-length
 [`max-trait-bounds`]: https://doc.rust-lang.org/clippy/lint_configuration.html#max-trait-bounds
+[`min-and-mask-size`]: https://doc.rust-lang.org/clippy/lint_configuration.html#min-and-mask-size
 [`min-ident-chars-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#min-ident-chars-threshold
 [`missing-docs-in-crate-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#missing-docs-in-crate-items
 [`module-item-order-groupings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#module-item-order-groupings

book/src/lint_configuration.md

@@ -703,6 +703,16 @@ The maximum number of bounds a trait can have to be linted
 * [`type_repetition_in_bounds`](https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds)
 
 
+## `min-and-mask-size`
+The smallest number of bits masked with `&` which will be replaced by `.is_multiple_of()`.
+
+**Default Value:** `3`
+
+---
+**Affected lints:**
+* [`manual_is_multiple_of`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_multiple_of)
+
+
 ## `min-ident-chars-threshold`
 Minimum chars an ident can have, anything below or equal to this will be linted.
 

clippy_config/src/conf.rs

@@ -583,6 +583,9 @@ define_Conf! {
     /// The maximum number of bounds a trait can have to be linted
     #[lints(type_repetition_in_bounds)]
     max_trait_bounds: u64 = 3,
+    /// The smallest number of bits masked with `&` which will be replaced by `.is_multiple_of()`.
+    #[lints(manual_is_multiple_of)]
+    min_and_mask_size: u8 = 3,
     /// Minimum chars an ident can have, anything below or equal to this will be linted.
     #[lints(min_ident_chars)]
     min_ident_chars_threshold: u64 = 1,

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

@@ -606,6 +606,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::MISREFACTORED_ASSIGN_OP_INFO,
     crate::operators::MODULO_ARITHMETIC_INFO,
     crate::operators::MODULO_ONE_INFO,

clippy_lints/src/if_not_else.rs

@@ -1,4 +1,4 @@
-use clippy_utils::consts::{ConstEvalCtxt, Constant};
+use clippy_utils::consts::is_zero_integer_const;
 use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
 use clippy_utils::is_else_clause;
 use clippy_utils::source::{HasSession, indent_of, reindent_multiline, snippet};
@@ -48,13 +48,6 @@ declare_clippy_lint! {
 
 declare_lint_pass!(IfNotElse => [IF_NOT_ELSE]);
 
-fn is_zero_const(expr: &Expr<'_>, cx: &LateContext<'_>) -> bool {
-    if let Some(value) = ConstEvalCtxt::new(cx).eval_simple(expr) {
-        return Constant::Int(0) == value;
-    }
-    false
-}
-
 impl LateLintPass<'_> for IfNotElse {
     fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
         if let ExprKind::If(cond, cond_inner, Some(els)) = e.kind
@@ -68,7 +61,7 @@ impl LateLintPass<'_> for IfNotElse {
                 ),
                 // Don't lint on `… != 0`, as these are likely to be bit tests.
                 // For example, `if foo & 0x0F00 != 0 { … } else { … }` is already in the "proper" order.
-                ExprKind::Binary(op, _, rhs) if op.node == BinOpKind::Ne && !is_zero_const(rhs, cx) => (
+                ExprKind::Binary(op, _, rhs) if op.node == BinOpKind::Ne && !is_zero_integer_const(cx, rhs) => (
                     "unnecessary `!=` operation",
                     "change to `==` and swap the blocks of the `if`/`else`",
                 ),

clippy_lints/src/lib.rs

@@ -12,6 +12,7 @@
 #![feature(round_char_boundary)]
 #![feature(rustc_private)]
 #![feature(stmt_expr_attributes)]
+#![feature(unsigned_is_multiple_of)]
 #![feature(unwrap_infallible)]
 #![recursion_limit = "512"]
 #![allow(

clippy_lints/src/operators/identity_op.rs

@@ -1,4 +1,4 @@
-use clippy_utils::consts::{ConstEvalCtxt, Constant, FullInt};
+use clippy_utils::consts::{ConstEvalCtxt, Constant, FullInt, integer_const, is_zero_integer_const};
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::{clip, peel_hir_expr_refs, unsext};
@@ -170,9 +170,7 @@ fn is_allowed(cx: &LateContext<'_>, cmp: BinOpKind, left: &Expr<'_>, right: &Exp
     cx.typeck_results().expr_ty(left).peel_refs().is_integral()
         && cx.typeck_results().expr_ty(right).peel_refs().is_integral()
         // `1 << 0` is a common pattern in bit manipulation code
-        && !(cmp == BinOpKind::Shl
-            && ConstEvalCtxt::new(cx).eval_simple(right) == Some(Constant::Int(0))
-            && ConstEvalCtxt::new(cx).eval_simple(left) == Some(Constant::Int(1)))
+        && !(cmp == BinOpKind::Shl && is_zero_integer_const(cx, right) && integer_const(cx, left) == Some(1))
 }
 
 fn check_remainder(cx: &LateContext<'_>, left: &Expr<'_>, right: &Expr<'_>, span: Span, arg: Span) {

clippy_lints/src/operators/manual_is_multiple_of.rs

@@ -0,0 +1,115 @@
+use clippy_utils::consts::{integer_const, is_zero_integer_const};
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::source::SpanRangeExt;
+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>,
+    min_and_mask_size: u8,
+    msrv: &Msrv,
+) {
+    if let Some(operand) = uint_compare_to_zero(cx, op, lhs, rhs)
+        && let ExprKind::Binary(lhs_op, lhs_left, lhs_right) = operand.kind
+        && msrv.meets(msrvs::UNSIGNED_IS_MULTIPLE_OF)
+    {
+        let mut app = Applicability::MachineApplicable;
+        let (dividend, divisor) = if lhs_op.node == BinOpKind::Rem {
+            (
+                lhs_left,
+                Sugg::hir_with_applicability(cx, lhs_right, "_", &mut app).into_string(),
+            )
+        } else if lhs_op.node == BinOpKind::BitAnd {
+            let min_divisor = 1 << u128::from(min_and_mask_size);
+            if let Some(divisor) = is_all_ones(cx, lhs_right, min_divisor, &mut app) {
+                (lhs_left, divisor)
+            } else if let Some(divisor) = is_all_ones(cx, lhs_left, min_divisor, &mut app) {
+                (lhs_right, divisor)
+            } else {
+                return;
+            }
+        } else {
+            return;
+        };
+        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, dividend, "_", &mut app).maybe_par()
+            ),
+            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)
+}
+
+/// If `expr` is made of all ones, return the representation of `expr+1` if it is no smaller than
+/// `min_divisor`.
+fn is_all_ones<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'tcx>,
+    min_divisor: u128,
+    app: &mut Applicability,
+) -> Option<String> {
+    if let ExprKind::Binary(op, lhs, rhs) = expr.kind
+        && op.node == BinOpKind::Sub
+        && let ExprKind::Binary(op, lhs_left, lhs_right) = lhs.kind
+        && op.node == BinOpKind::Shl
+        && let Some(1) = integer_const(cx, lhs_left)
+        && let Some(1) = integer_const(cx, rhs)
+        && integer_const(cx, lhs_right).is_none_or(|v| 1 << v >= min_divisor)
+    {
+        Some(Sugg::hir_with_applicability(cx, lhs, "_", app).to_string())
+    } else if let Some(value) = integer_const(cx, expr)
+        && let Some(inc_value) = value.checked_add(1)
+        && inc_value.is_power_of_two()
+    {
+        let repr = if expr.span.check_source_text(cx, |s| s.starts_with("0x")) {
+            format!("{inc_value:#x}")
+        } else {
+            inc_value.to_string()
+        };
+        (inc_value >= min_divisor).then_some(repr)
+    } else {
+        None
+    }
+}

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 misrefactored_assign_op;
 mod modulo_arithmetic;
 mod modulo_one;
@@ -24,6 +25,7 @@ mod verbose_bit_mask;
 pub(crate) mod arithmetic_side_effects;
 
 use clippy_config::Conf;
+use clippy_utils::msrvs::Msrv;
 use rustc_hir::{Body, Expr, ExprKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
@@ -834,17 +836,58 @@ declare_clippy_lint! {
     "explicit self-assignment"
 }
 
+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}");
+    /// }
+    /// if a & 3 != 0 {
+    ///     println!("{a} is not properly aligned");
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # #![feature(unsigned_is_multiple_of)]
+    /// # let (a, b) = (3u64, 4u64);
+    /// if a.is_multiple_of(b) {
+    ///     println!("{a} is divisible by {b}");
+    /// }
+    /// if !a.is_multiple_of(4) {
+    ///     println!("{a} is not properly aligned");
+    /// }
+    /// ```
+    #[clippy::version = "1.87.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,
+    min_and_mask_size: u8,
+    msrv: Msrv,
 }
+
 impl Operators {
     pub fn new(conf: &'static Conf) -> Self {
         Self {
             arithmetic_context: numeric_arithmetic::Context::default(),
             verbose_bit_mask_threshold: conf.verbose_bit_mask_threshold,
             modulo_arithmetic_allow_comparison_to_zero: conf.allow_comparison_to_zero,
+            min_and_mask_size: conf.min_and_mask_size,
+            msrv: conf.msrv.clone(),
         }
     }
 }
@@ -876,6 +919,7 @@ impl_lint_pass!(Operators => [
     NEEDLESS_BITWISE_BOOL,
     PTR_EQ,
     SELF_ASSIGNMENT,
+    MANUAL_IS_MULTIPLE_OF,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Operators {
@@ -913,6 +957,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
                     rhs,
                     self.modulo_arithmetic_allow_comparison_to_zero,
                 );
+                manual_is_multiple_of::check(cx, e, op.node, lhs, rhs, self.min_and_mask_size, &self.msrv);
             },
             ExprKind::AssignOp(op, lhs, rhs) => {
                 self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);
@@ -943,6 +988,8 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
     fn check_body_post(&mut self, cx: &LateContext<'tcx>, b: &Body<'_>) {
         self.arithmetic_context.body_post(cx, b);
     }
+
+    extract_msrv_attr!(LateContext);
 }
 
 fn macro_with_not_op(e: &Expr<'_>) -> bool {

clippy_utils/src/consts.rs

@@ -961,3 +961,17 @@ fn field_of_struct<'tcx>(
         None
     }
 }
+
+/// If `expr` evaluates to an integer constant, return its value.
+pub fn integer_const(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<u128> {
+    if let Some(Constant::Int(value)) = ConstEvalCtxt::new(cx).eval_simple(expr) {
+        Some(value)
+    } else {
+        None
+    }
+}
+
+/// Check if `expr` evaluates to an integer constant of 0.
+pub fn is_zero_integer_const(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    integer_const(cx, expr) == Some(0)
+}

clippy_utils/src/msrvs.rs

@@ -19,6 +19,7 @@ macro_rules! msrv_aliases {
 
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
+    1,87,0 { UNSIGNED_IS_MULTIPLE_OF }
     1,84,0 { CONST_OPTION_AS_SLICE }
     1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, CONST_MUT_REFS, CONST_UNWRAP }
     1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP }

tests/ui-toml/manual_is_multiple_of/clippy.toml

@@ -0,0 +1 @@
+min-and-mask-size = 0

tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.fixed

@@ -0,0 +1,9 @@
+#![warn(clippy::manual_is_multiple_of)]
+#![feature(unsigned_is_multiple_of)]
+
+fn main() {}
+
+fn f(a: u64, b: u64) {
+    let _ = a.is_multiple_of(2); //~ manual_is_multiple_of
+    let _ = a.is_multiple_of(1 << 1); //~ manual_is_multiple_of
+}

tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.rs

@@ -0,0 +1,9 @@
+#![warn(clippy::manual_is_multiple_of)]
+#![feature(unsigned_is_multiple_of)]
+
+fn main() {}
+
+fn f(a: u64, b: u64) {
+    let _ = a & 1 == 0; //~ manual_is_multiple_of
+    let _ = a & ((1 << 1) - 1) == 0; //~ manual_is_multiple_of
+}

tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.stderr

@@ -0,0 +1,17 @@
+error: manual implementation of `.is_multiple_of()`
+  --> tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.rs:7:13
+   |
+LL |     let _ = a & 1 == 0;
+   |             ^^^^^^^^^^ help: replace with: `a.is_multiple_of(2)`
+   |
+   = 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-toml/manual_is_multiple_of/manual_is_multiple_of.rs:8:13
+   |
+LL |     let _ = a & ((1 << 1) - 1) == 0;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `a.is_multiple_of(1 << 1)`
+
+error: aborting due to 2 previous errors
+