Skip to content

Commit

Permalink
New lint: manual_is_multiple_of
Browse files Browse the repository at this point in the history
  • Loading branch information
samueltardieu committed Feb 25, 2025
1 parent 9a0a658 commit b5db45e
Show file tree
Hide file tree
Showing 17 changed files with 375 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
3 changes: 3 additions & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/casts/cast_sign_loss.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
115 changes: 115 additions & 0 deletions clippy_lints/src/operators/manual_is_multiple_of.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
47 changes: 47 additions & 0 deletions clippy_lints/src/operators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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(),
}
}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/manual_is_multiple_of/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
min-and-mask-size = 0
Original file line number Diff line number Diff line change
@@ -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
}
9 changes: 9 additions & 0 deletions tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.rs
Original file line number Diff line number Diff line change
@@ -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
}
17 changes: 17 additions & 0 deletions tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.stderr
Original file line number Diff line number Diff line change
@@ -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

3 changes: 3 additions & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
max-struct-bools
max-suggested-slice-pattern-length
max-trait-bounds
min-and-mask-size
min-ident-chars-threshold
missing-docs-in-crate-items
module-item-order-groupings
Expand Down Expand Up @@ -147,6 +148,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
max-struct-bools
max-suggested-slice-pattern-length
max-trait-bounds
min-and-mask-size
min-ident-chars-threshold
missing-docs-in-crate-items
module-item-order-groupings
Expand Down Expand Up @@ -238,6 +240,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
max-struct-bools
max-suggested-slice-pattern-length
max-trait-bounds
min-and-mask-size
min-ident-chars-threshold
missing-docs-in-crate-items
module-item-order-groupings
Expand Down
33 changes: 33 additions & 0 deletions tests/ui/manual_is_multiple_of.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#![warn(clippy::manual_is_multiple_of)]
#![feature(unsigned_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(4096); //~ manual_is_multiple_of
let _ = !a.is_multiple_of(4096); //~ manual_is_multiple_of
let _ = a.is_multiple_of(1 << b); //~ manual_is_multiple_of
let _ = !a.is_multiple_of(1 << b); //~ manual_is_multiple_of
let _ = a.is_multiple_of(1 << b); //~ manual_is_multiple_of

let _ = !a.is_multiple_of(b); //~ manual_is_multiple_of
let _ = !a.is_multiple_of(b); //~ manual_is_multiple_of

let _ = a.is_multiple_of(0x100); //~ manual_is_multiple_of

let _ = a & 1 == 0; // Do not lint: below `min-and-mask-size`
let _ = a & ((1 << 1) - 1) == 0; // Do not lint: below `min-and-mask-size`
let _ = a.is_multiple_of(8); //~ manual_is_multiple_of
let _ = a.is_multiple_of(1 << 3); //~ manual_is_multiple_of
}

#[clippy::msrv = "1.86"]
fn g(a: u64, b: u64) {
let _ = a % b == 0;
}
Loading

0 comments on commit b5db45e

Please sign in to comment.