diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a98217f625a..6e5fa367040b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5963,6 +5963,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 diff --git a/clippy_lints/src/casts/cast_sign_loss.rs b/clippy_lints/src/casts/cast_sign_loss.rs index 9a1ad8a74738..a70bd8861919 100644 --- a/clippy_lints/src/casts/cast_sign_loss.rs +++ b/clippy_lints/src/casts/cast_sign_loss.rs @@ -168,7 +168,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. diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 5fcb851dfebc..8a68a9df4155 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -585,6 +585,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, diff --git a/clippy_lints/src/if_not_else.rs b/clippy_lints/src/if_not_else.rs index 45f9aa0a53e4..ab7a965b3672 100644 --- a/clippy_lints/src/if_not_else.rs +++ b/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`", ), diff --git a/clippy_lints/src/operators/identity_op.rs b/clippy_lints/src/operators/identity_op.rs index e1fd09549a4b..12f3f63e73e1 100644 --- a/clippy_lints/src/operators/identity_op.rs +++ b/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) { diff --git a/clippy_lints/src/operators/manual_is_multiple_of.rs b/clippy_lints/src/operators/manual_is_multiple_of.rs new file mode 100644 index 000000000000..821178a43158 --- /dev/null +++ b/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) +} diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index d32c062cf56a..0d8dd0c46b5f 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/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.89.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); diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 6f5b0ec54cd8..eb4bf5ab4e0c 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -960,3 +960,18 @@ 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 { + 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. +#[inline] +pub fn is_zero_integer_const(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + integer_const(cx, expr) == Some(0) +} diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index ec3de2b9dc8d..1c77e84946a4 100644 --- a/clippy_utils/src/msrvs.rs +++ b/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, CONST_CHAR_IS_DIGIT } + 1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT, CONST_CHAR_IS_DIGIT, UNSIGNED_IS_MULTIPLE_OF } 1,85,0 { UINT_FLOAT_MIDPOINT, CONST_SIZE_OF_VAL } 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 } diff --git a/tests/ui/box_default.fixed b/tests/ui/box_default.fixed index 80000f5de4fd..ed00494433b9 100644 --- a/tests/ui/box_default.fixed +++ b/tests/ui/box_default.fixed @@ -126,7 +126,7 @@ fn issue_10381() { impl Bar for Foo {} fn maybe_get_bar(i: u32) -> Option> { - if i % 2 == 0 { + if i.is_multiple_of(2) { Some(Box::new(Foo::default())) } else { None diff --git a/tests/ui/box_default.rs b/tests/ui/box_default.rs index 4681016d7cd3..801d92f5c290 100644 --- a/tests/ui/box_default.rs +++ b/tests/ui/box_default.rs @@ -126,7 +126,7 @@ fn issue_10381() { impl Bar for Foo {} fn maybe_get_bar(i: u32) -> Option> { - if i % 2 == 0 { + if i.is_multiple_of(2) { Some(Box::new(Foo::default())) } else { None diff --git a/tests/ui/infinite_iter.rs b/tests/ui/infinite_iter.rs index 002a791a6579..701a86534ba0 100644 --- a/tests/ui/infinite_iter.rs +++ b/tests/ui/infinite_iter.rs @@ -38,7 +38,7 @@ fn infinite_iters() { //~^ infinite_iter // infinite iter - (0_u64..).filter(|x| x % 2 == 0).last(); + (0_u64..).filter(|x| x.is_multiple_of(2)).last(); //~^ infinite_iter // not an infinite, because ranges are double-ended diff --git a/tests/ui/infinite_iter.stderr b/tests/ui/infinite_iter.stderr index 47133a2ea62e..b9e7c008f93e 100644 --- a/tests/ui/infinite_iter.stderr +++ b/tests/ui/infinite_iter.stderr @@ -42,8 +42,8 @@ LL | (0_usize..).flat_map(|x| 0..x).product::(); error: infinite iteration detected --> tests/ui/infinite_iter.rs:41:5 | -LL | (0_u64..).filter(|x| x % 2 == 0).last(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | (0_u64..).filter(|x| x.is_multiple_of(2)).last(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: possible infinite iteration detected --> tests/ui/infinite_iter.rs:53:5 diff --git a/tests/ui/iter_kv_map.fixed b/tests/ui/iter_kv_map.fixed index 874f749b33d0..b18dda358877 100644 --- a/tests/ui/iter_kv_map.fixed +++ b/tests/ui/iter_kv_map.fixed @@ -30,15 +30,19 @@ fn main() { let _ = map.clone().values().collect::>(); //~^ iter_kv_map - let _ = map.keys().filter(|x| *x % 2 == 0).count(); + let _ = map.keys().filter(|x| x.is_multiple_of(2)).count(); //~^ iter_kv_map // Don't lint - let _ = map.iter().filter(|(_, val)| *val % 2 == 0).map(|(key, _)| key).count(); + let _ = map + .iter() + .filter(|(_, val)| val.is_multiple_of(2)) + .map(|(key, _)| key) + .count(); let _ = map.iter().map(get_key).collect::>(); // Linting the following could be an improvement to the lint - // map.iter().filter_map(|(_, val)| (val % 2 == 0).then(val * 17)).count(); + // map.iter().filter_map(|(_, val)| (val.is_multiple_of(2)).then(val * 17)).count(); // Lint let _ = map.keys().map(|key| key * 9).count(); @@ -84,15 +88,19 @@ fn main() { let _ = map.clone().values().collect::>(); //~^ iter_kv_map - let _ = map.keys().filter(|x| *x % 2 == 0).count(); + let _ = map.keys().filter(|x| x.is_multiple_of(2)).count(); //~^ iter_kv_map // Don't lint - let _ = map.iter().filter(|(_, val)| *val % 2 == 0).map(|(key, _)| key).count(); + let _ = map + .iter() + .filter(|(_, val)| val.is_multiple_of(2)) + .map(|(key, _)| key) + .count(); let _ = map.iter().map(get_key).collect::>(); // Linting the following could be an improvement to the lint - // map.iter().filter_map(|(_, val)| (val % 2 == 0).then(val * 17)).count(); + // map.iter().filter_map(|(_, val)| (val.is_multiple_of(2)).then(val * 17)).count(); // Lint let _ = map.keys().map(|key| key * 9).count(); diff --git a/tests/ui/iter_kv_map.rs b/tests/ui/iter_kv_map.rs index f570e3c32cb6..729e4e8a266c 100644 --- a/tests/ui/iter_kv_map.rs +++ b/tests/ui/iter_kv_map.rs @@ -30,15 +30,19 @@ fn main() { let _ = map.clone().iter().map(|(_, val)| val).collect::>(); //~^ iter_kv_map - let _ = map.iter().map(|(key, _)| key).filter(|x| *x % 2 == 0).count(); + let _ = map.iter().map(|(key, _)| key).filter(|x| x.is_multiple_of(2)).count(); //~^ iter_kv_map // Don't lint - let _ = map.iter().filter(|(_, val)| *val % 2 == 0).map(|(key, _)| key).count(); + let _ = map + .iter() + .filter(|(_, val)| val.is_multiple_of(2)) + .map(|(key, _)| key) + .count(); let _ = map.iter().map(get_key).collect::>(); // Linting the following could be an improvement to the lint - // map.iter().filter_map(|(_, val)| (val % 2 == 0).then(val * 17)).count(); + // map.iter().filter_map(|(_, val)| (val.is_multiple_of(2)).then(val * 17)).count(); // Lint let _ = map.iter().map(|(key, _value)| key * 9).count(); @@ -86,15 +90,19 @@ fn main() { let _ = map.clone().iter().map(|(_, val)| val).collect::>(); //~^ iter_kv_map - let _ = map.iter().map(|(key, _)| key).filter(|x| *x % 2 == 0).count(); + let _ = map.iter().map(|(key, _)| key).filter(|x| x.is_multiple_of(2)).count(); //~^ iter_kv_map // Don't lint - let _ = map.iter().filter(|(_, val)| *val % 2 == 0).map(|(key, _)| key).count(); + let _ = map + .iter() + .filter(|(_, val)| val.is_multiple_of(2)) + .map(|(key, _)| key) + .count(); let _ = map.iter().map(get_key).collect::>(); // Linting the following could be an improvement to the lint - // map.iter().filter_map(|(_, val)| (val % 2 == 0).then(val * 17)).count(); + // map.iter().filter_map(|(_, val)| (val.is_multiple_of(2)).then(val * 17)).count(); // Lint let _ = map.iter().map(|(key, _value)| key * 9).count(); diff --git a/tests/ui/iter_kv_map.stderr b/tests/ui/iter_kv_map.stderr index 31ee76c25b7a..8f73541f5033 100644 --- a/tests/ui/iter_kv_map.stderr +++ b/tests/ui/iter_kv_map.stderr @@ -52,29 +52,29 @@ LL | let _ = map.clone().iter().map(|(_, val)| val).collect::>(); error: iterating on a map's keys --> tests/ui/iter_kv_map.rs:33:13 | -LL | let _ = map.iter().map(|(key, _)| key).filter(|x| *x % 2 == 0).count(); +LL | let _ = map.iter().map(|(key, _)| key).filter(|x| x.is_multiple_of(2)).count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys()` error: iterating on a map's keys - --> tests/ui/iter_kv_map.rs:44:13 + --> tests/ui/iter_kv_map.rs:48:13 | LL | let _ = map.iter().map(|(key, _value)| key * 9).count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys().map(|key| key * 9)` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:46:13 + --> tests/ui/iter_kv_map.rs:50:13 | LL | let _ = map.iter().map(|(_key, value)| value * 17).count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values().map(|value| value * 17)` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:50:13 + --> tests/ui/iter_kv_map.rs:54:13 | LL | let _ = map.clone().into_iter().map(|(_, ref val)| ref_acceptor(val)).count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values().map(|ref val| ref_acceptor(val))` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:54:13 + --> tests/ui/iter_kv_map.rs:58:13 | LL | let _ = map | _____________^ @@ -97,85 +97,85 @@ LL + }) | error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:65:13 + --> tests/ui/iter_kv_map.rs:69:13 | LL | let _ = map.clone().into_iter().map(|(_, mut val)| val).count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values()` error: iterating on a map's keys - --> tests/ui/iter_kv_map.rs:70:13 + --> tests/ui/iter_kv_map.rs:74:13 | LL | let _ = map.iter().map(|(key, _)| key).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys()` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:72:13 + --> tests/ui/iter_kv_map.rs:76:13 | LL | let _ = map.iter().map(|(_, value)| value).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values()` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:74:13 + --> tests/ui/iter_kv_map.rs:78:13 | LL | let _ = map.iter().map(|(_, v)| v + 2).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values().map(|v| v + 2)` error: iterating on a map's keys - --> tests/ui/iter_kv_map.rs:77:13 + --> tests/ui/iter_kv_map.rs:81:13 | LL | let _ = map.clone().into_iter().map(|(key, _)| key).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_keys()` error: iterating on a map's keys - --> tests/ui/iter_kv_map.rs:79:13 + --> tests/ui/iter_kv_map.rs:83:13 | LL | let _ = map.clone().into_iter().map(|(key, _)| key + 2).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_keys().map(|key| key + 2)` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:82:13 + --> tests/ui/iter_kv_map.rs:86:13 | LL | let _ = map.clone().into_iter().map(|(_, val)| val).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values()` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:84:13 + --> tests/ui/iter_kv_map.rs:88:13 | LL | let _ = map.clone().into_iter().map(|(_, val)| val + 2).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values().map(|val| val + 2)` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:87:13 + --> tests/ui/iter_kv_map.rs:91:13 | LL | let _ = map.clone().iter().map(|(_, val)| val).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().values()` error: iterating on a map's keys - --> tests/ui/iter_kv_map.rs:89:13 + --> tests/ui/iter_kv_map.rs:93:13 | -LL | let _ = map.iter().map(|(key, _)| key).filter(|x| *x % 2 == 0).count(); +LL | let _ = map.iter().map(|(key, _)| key).filter(|x| x.is_multiple_of(2)).count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys()` error: iterating on a map's keys - --> tests/ui/iter_kv_map.rs:100:13 + --> tests/ui/iter_kv_map.rs:108:13 | LL | let _ = map.iter().map(|(key, _value)| key * 9).count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys().map(|key| key * 9)` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:102:13 + --> tests/ui/iter_kv_map.rs:110:13 | LL | let _ = map.iter().map(|(_key, value)| value * 17).count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values().map(|value| value * 17)` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:106:13 + --> tests/ui/iter_kv_map.rs:114:13 | LL | let _ = map.clone().into_iter().map(|(_, ref val)| ref_acceptor(val)).count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values().map(|ref val| ref_acceptor(val))` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:110:13 + --> tests/ui/iter_kv_map.rs:118:13 | LL | let _ = map | _____________^ @@ -198,73 +198,73 @@ LL + }) | error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:121:13 + --> tests/ui/iter_kv_map.rs:129:13 | LL | let _ = map.clone().into_iter().map(|(_, mut val)| val).count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values()` error: iterating on a map's keys - --> tests/ui/iter_kv_map.rs:137:13 + --> tests/ui/iter_kv_map.rs:145:13 | LL | let _ = map.iter().map(|(key, _)| key).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys()` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:140:13 + --> tests/ui/iter_kv_map.rs:148:13 | LL | let _ = map.iter().map(|(_, value)| value).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values()` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:143:13 + --> tests/ui/iter_kv_map.rs:151:13 | LL | let _ = map.iter().map(|(_, v)| v + 2).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values().map(|v| v + 2)` error: iterating on a map's keys - --> tests/ui/iter_kv_map.rs:152:13 + --> tests/ui/iter_kv_map.rs:160:13 | LL | let _ = map.clone().into_iter().map(|(key, _)| key).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_keys()` error: iterating on a map's keys - --> tests/ui/iter_kv_map.rs:155:13 + --> tests/ui/iter_kv_map.rs:163:13 | LL | let _ = map.clone().into_iter().map(|(key, _)| key + 2).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_keys().map(|key| key + 2)` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:158:13 + --> tests/ui/iter_kv_map.rs:166:13 | LL | let _ = map.clone().into_iter().map(|(_, val)| val).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values()` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:161:13 + --> tests/ui/iter_kv_map.rs:169:13 | LL | let _ = map.clone().into_iter().map(|(_, val)| val + 2).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.clone().into_values().map(|val| val + 2)` error: iterating on a map's keys - --> tests/ui/iter_kv_map.rs:164:13 + --> tests/ui/iter_kv_map.rs:172:13 | LL | let _ = map.iter().map(|(key, _)| key).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys()` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:167:13 + --> tests/ui/iter_kv_map.rs:175:13 | LL | let _ = map.iter().map(|(_, value)| value).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values()` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:170:13 + --> tests/ui/iter_kv_map.rs:178:13 | LL | let _ = map.iter().map(|(_, v)| v + 2).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values().map(|v| v + 2)` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:185:13 + --> tests/ui/iter_kv_map.rs:193:13 | LL | let _ = map.as_ref().iter().map(|(_, v)| v).copied().collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.as_ref().values()` diff --git a/tests/ui/let_unit.fixed b/tests/ui/let_unit.fixed index 5e7a2ad37a84..304eacecd942 100644 --- a/tests/ui/let_unit.fixed +++ b/tests/ui/let_unit.fixed @@ -61,7 +61,7 @@ fn multiline_sugg() { //~^ let_unit_value .into_iter() .map(|i| i * 2) - .filter(|i| i % 2 == 0) + .filter(|i| i.is_multiple_of(2)) .map(|_| ()) .next() .unwrap(); diff --git a/tests/ui/let_unit.rs b/tests/ui/let_unit.rs index 7b06f6940121..a02cb346ff99 100644 --- a/tests/ui/let_unit.rs +++ b/tests/ui/let_unit.rs @@ -61,7 +61,7 @@ fn multiline_sugg() { //~^ let_unit_value .into_iter() .map(|i| i * 2) - .filter(|i| i % 2 == 0) + .filter(|i| i.is_multiple_of(2)) .map(|_| ()) .next() .unwrap(); diff --git a/tests/ui/let_unit.stderr b/tests/ui/let_unit.stderr index d7d01d304cad..d743110c99dd 100644 --- a/tests/ui/let_unit.stderr +++ b/tests/ui/let_unit.stderr @@ -25,7 +25,7 @@ LL ~ v LL + LL + .into_iter() LL + .map(|i| i * 2) -LL + .filter(|i| i % 2 == 0) +LL + .filter(|i| i.is_multiple_of(2)) LL + .map(|_| ()) LL + .next() LL + .unwrap(); diff --git a/tests/ui/manual_contains.fixed b/tests/ui/manual_contains.fixed index d26c948a7817..18171f0b2b40 100644 --- a/tests/ui/manual_contains.fixed +++ b/tests/ui/manual_contains.fixed @@ -58,7 +58,7 @@ fn should_not_lint() { let vec: Vec = vec![1, 2, 3, 4, 5, 6]; let values = &vec[..]; - let _ = values.iter().any(|&v| v % 2 == 0); + let _ = values.iter().any(|&v| v.is_multiple_of(2)); let _ = values.iter().any(|&v| v * 2 == 6); let _ = values.iter().any(|&v| v == v); let _ = values.iter().any(|&v| 4 == 4); diff --git a/tests/ui/manual_contains.rs b/tests/ui/manual_contains.rs index fe67d2ee5d5c..918f4d6b8dd7 100644 --- a/tests/ui/manual_contains.rs +++ b/tests/ui/manual_contains.rs @@ -58,7 +58,7 @@ fn should_not_lint() { let vec: Vec = vec![1, 2, 3, 4, 5, 6]; let values = &vec[..]; - let _ = values.iter().any(|&v| v % 2 == 0); + let _ = values.iter().any(|&v| v.is_multiple_of(2)); let _ = values.iter().any(|&v| v * 2 == 6); let _ = values.iter().any(|&v| v == v); let _ = values.iter().any(|&v| 4 == 4); diff --git a/tests/ui/manual_find_fixable.fixed b/tests/ui/manual_find_fixable.fixed index 5e6849a4dfb0..48dfa7abaac0 100644 --- a/tests/ui/manual_find_fixable.fixed +++ b/tests/ui/manual_find_fixable.fixed @@ -11,7 +11,7 @@ fn lookup(n: u32) -> Option { } fn with_pat(arr: Vec<(u32, u32)>) -> Option { - arr.into_iter().map(|(a, _)| a).find(|&a| a % 2 == 0) + arr.into_iter().map(|(a, _)| a).find(|&a| a.is_multiple_of(2)) } struct Data { @@ -63,7 +63,7 @@ fn with_side_effects(arr: Vec) -> Option { fn with_else(arr: Vec) -> Option { for el in arr { - if el % 2 == 0 { + if el.is_multiple_of(2) { return Some(el); } else { println!("{}", el); diff --git a/tests/ui/manual_find_fixable.rs b/tests/ui/manual_find_fixable.rs index 08a7dd2c6eee..41ce0da1346a 100644 --- a/tests/ui/manual_find_fixable.rs +++ b/tests/ui/manual_find_fixable.rs @@ -19,7 +19,7 @@ fn lookup(n: u32) -> Option { fn with_pat(arr: Vec<(u32, u32)>) -> Option { for (a, _) in arr { //~^ manual_find - if a % 2 == 0 { + if a.is_multiple_of(2) { return Some(a); } } @@ -111,7 +111,7 @@ fn with_side_effects(arr: Vec) -> Option { fn with_else(arr: Vec) -> Option { for el in arr { - if el % 2 == 0 { + if el.is_multiple_of(2) { return Some(el); } else { println!("{}", el); diff --git a/tests/ui/manual_find_fixable.stderr b/tests/ui/manual_find_fixable.stderr index afa453c5a876..bcf1fc5b381e 100644 --- a/tests/ui/manual_find_fixable.stderr +++ b/tests/ui/manual_find_fixable.stderr @@ -17,11 +17,11 @@ error: manual implementation of `Iterator::find` | LL | / for (a, _) in arr { LL | | -LL | | if a % 2 == 0 { +LL | | if a.is_multiple_of(2) { LL | | return Some(a); ... | LL | | None - | |________^ help: replace with an iterator: `arr.into_iter().map(|(a, _)| a).find(|&a| a % 2 == 0)` + | |________^ help: replace with an iterator: `arr.into_iter().map(|(a, _)| a).find(|&a| a.is_multiple_of(2))` error: manual implementation of `Iterator::find` --> tests/ui/manual_find_fixable.rs:34:5 diff --git a/tests/ui/manual_is_multiple_of.fixed b/tests/ui/manual_is_multiple_of.fixed new file mode 100644 index 000000000000..6735b99f298c --- /dev/null +++ b/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; +} diff --git a/tests/ui/manual_is_multiple_of.rs b/tests/ui/manual_is_multiple_of.rs new file mode 100644 index 000000000000..00b638e4fd9f --- /dev/null +++ b/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; +} diff --git a/tests/ui/manual_is_multiple_of.stderr b/tests/ui/manual_is_multiple_of.stderr new file mode 100644 index 000000000000..0b1ae70c2a70 --- /dev/null +++ b/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 +