From 98e87cfa91968400a020d51a8d27383daa832cf7 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Tue, 25 Feb 2025 09:20:55 +0100 Subject: [PATCH 1/3] Add `integer_const()` and `is_zero_integer_const()` utility functions --- clippy_lints/src/if_not_else.rs | 11 ++--------- clippy_lints/src/operators/identity_op.rs | 6 ++---- clippy_utils/src/consts.rs | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 13 deletions(-) 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 1c2d6e90fc95..80db12a77fce 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_utils/src/consts.rs b/clippy_utils/src/consts.rs index 4f707e34abf9..3cab487d479e 100644 --- a/clippy_utils/src/consts.rs +++ b/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 { + 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) +} From 9a0a658a583218face7ad8c75b31d4b370482a33 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Tue, 25 Feb 2025 22:36:30 +0100 Subject: [PATCH 2/3] Use `.is_multiple_of()` in Clippy tests sources This prevents triggering the new `manual_is_multiple_of` lint on unrelated lint tests. --- tests/ui/box_default.fixed | 3 +- tests/ui/box_default.rs | 3 +- tests/ui/box_default.stderr | 20 ++++---- tests/ui/infinite_iter.rs | 3 +- tests/ui/infinite_iter.stderr | 36 ++++++------- tests/ui/iter_kv_map.fixed | 21 +++++--- tests/ui/iter_kv_map.rs | 21 +++++--- tests/ui/iter_kv_map.stderr | 80 ++++++++++++++--------------- tests/ui/let_unit.fixed | 3 +- tests/ui/let_unit.rs | 3 +- tests/ui/let_unit.stderr | 10 ++-- tests/ui/manual_contains.fixed | 3 +- tests/ui/manual_contains.rs | 3 +- tests/ui/manual_contains.stderr | 24 ++++----- tests/ui/manual_find_fixable.fixed | 5 +- tests/ui/manual_find_fixable.rs | 5 +- tests/ui/manual_find_fixable.stderr | 28 +++++----- 17 files changed, 149 insertions(+), 122 deletions(-) diff --git a/tests/ui/box_default.fixed b/tests/ui/box_default.fixed index 80000f5de4fd..5fcab02a79f2 100644 --- a/tests/ui/box_default.fixed +++ b/tests/ui/box_default.fixed @@ -1,5 +1,6 @@ #![warn(clippy::box_default)] #![allow(clippy::boxed_local, clippy::default_constructed_unit_structs)] +#![feature(unsigned_is_multiple_of)] #[derive(Default)] struct ImplementsDefault; @@ -126,7 +127,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..360829ee94a7 100644 --- a/tests/ui/box_default.rs +++ b/tests/ui/box_default.rs @@ -1,5 +1,6 @@ #![warn(clippy::box_default)] #![allow(clippy::boxed_local, clippy::default_constructed_unit_structs)] +#![feature(unsigned_is_multiple_of)] #[derive(Default)] struct ImplementsDefault; @@ -126,7 +127,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.stderr b/tests/ui/box_default.stderr index f63d97665b34..c9e4b09e7674 100644 --- a/tests/ui/box_default.stderr +++ b/tests/ui/box_default.stderr @@ -1,5 +1,5 @@ error: `Box::new(_)` of default value - --> tests/ui/box_default.rs:34:32 + --> tests/ui/box_default.rs:35:32 | LL | let string1: Box = Box::new(Default::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` @@ -8,55 +8,55 @@ LL | let string1: Box = Box::new(Default::default()); = help: to override `-D warnings` add `#[allow(clippy::box_default)]` error: `Box::new(_)` of default value - --> tests/ui/box_default.rs:36:32 + --> tests/ui/box_default.rs:37:32 | LL | let string2: Box = Box::new(String::new()); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> tests/ui/box_default.rs:38:41 + --> tests/ui/box_default.rs:39:41 | LL | let impl1: Box = Box::new(Default::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> tests/ui/box_default.rs:40:29 + --> tests/ui/box_default.rs:41:29 | LL | let vec: Box> = Box::new(Vec::new()); | ^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> tests/ui/box_default.rs:42:25 + --> tests/ui/box_default.rs:43:25 | LL | let byte: Box = Box::new(u8::default()); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> tests/ui/box_default.rs:44:45 + --> tests/ui/box_default.rs:45:45 | LL | let vec2: Box> = Box::new(vec![]); | ^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> tests/ui/box_default.rs:46:32 + --> tests/ui/box_default.rs:47:32 | LL | let vec3: Box> = Box::new(Vec::from([])); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> tests/ui/box_default.rs:49:25 + --> tests/ui/box_default.rs:50:25 | LL | let plain_default = Box::new(Default::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> tests/ui/box_default.rs:67:16 + --> tests/ui/box_default.rs:68:16 | LL | call_ty_fn(Box::new(u8::default())); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` error: `Box::new(_)` of default value - --> tests/ui/box_default.rs:95:17 + --> tests/ui/box_default.rs:96:17 | LL | Self::x(Box::new(T::default())); | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::default()` diff --git a/tests/ui/infinite_iter.rs b/tests/ui/infinite_iter.rs index 002a791a6579..3ddd9bddc167 100644 --- a/tests/ui/infinite_iter.rs +++ b/tests/ui/infinite_iter.rs @@ -1,4 +1,5 @@ #![allow(clippy::uninlined_format_args, clippy::double_ended_iterator_last)] +#![feature(unsigned_is_multiple_of)] use std::iter::repeat; fn square_is_lower_64(x: &u32) -> bool { @@ -38,7 +39,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..b1b7f7112efa 100644 --- a/tests/ui/infinite_iter.stderr +++ b/tests/ui/infinite_iter.stderr @@ -1,29 +1,29 @@ error: infinite iteration detected - --> tests/ui/infinite_iter.rs:11:5 + --> tests/ui/infinite_iter.rs:12:5 | LL | repeat(0_u8).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the lint level is defined here - --> tests/ui/infinite_iter.rs:9:8 + --> tests/ui/infinite_iter.rs:10:8 | LL | #[deny(clippy::infinite_iter)] | ^^^^^^^^^^^^^^^^^^^^^ error: infinite iteration detected - --> tests/ui/infinite_iter.rs:15:5 + --> tests/ui/infinite_iter.rs:16:5 | LL | (0..8_u32).take_while(square_is_lower_64).cycle().count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: infinite iteration detected - --> tests/ui/infinite_iter.rs:19:5 + --> tests/ui/infinite_iter.rs:20:5 | LL | (0..8_u64).chain(0..).max(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: infinite iteration detected - --> tests/ui/infinite_iter.rs:28:5 + --> tests/ui/infinite_iter.rs:29:5 | LL | / (0..8_u32) LL | | @@ -34,37 +34,37 @@ LL | | .for_each(|x| println!("{}", x)); | |________________________________________^ error: infinite iteration detected - --> tests/ui/infinite_iter.rs:37:5 + --> tests/ui/infinite_iter.rs:38:5 | LL | (0_usize..).flat_map(|x| 0..x).product::(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: infinite iteration detected - --> tests/ui/infinite_iter.rs:41:5 + --> tests/ui/infinite_iter.rs:42: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 + --> tests/ui/infinite_iter.rs:54:5 | LL | (0..).zip((0..).take_while(square_is_lower_64)).count(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the lint level is defined here - --> tests/ui/infinite_iter.rs:50:8 + --> tests/ui/infinite_iter.rs:51:8 | LL | #[deny(clippy::maybe_infinite_iter)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: possible infinite iteration detected - --> tests/ui/infinite_iter.rs:57:5 + --> tests/ui/infinite_iter.rs:58:5 | LL | repeat(42).take_while(|x| *x == 42).chain(0..42).max(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: possible infinite iteration detected - --> tests/ui/infinite_iter.rs:61:5 + --> tests/ui/infinite_iter.rs:62:5 | LL | / (1..) LL | | @@ -76,31 +76,31 @@ LL | | .min(); | |______________^ error: possible infinite iteration detected - --> tests/ui/infinite_iter.rs:69:5 + --> tests/ui/infinite_iter.rs:70:5 | LL | (0..).find(|x| *x == 24); | ^^^^^^^^^^^^^^^^^^^^^^^^ error: possible infinite iteration detected - --> tests/ui/infinite_iter.rs:73:5 + --> tests/ui/infinite_iter.rs:74:5 | LL | (0..).position(|x| x == 24); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: possible infinite iteration detected - --> tests/ui/infinite_iter.rs:77:5 + --> tests/ui/infinite_iter.rs:78:5 | LL | (0..).any(|x| x == 24); | ^^^^^^^^^^^^^^^^^^^^^^ error: possible infinite iteration detected - --> tests/ui/infinite_iter.rs:81:5 + --> tests/ui/infinite_iter.rs:82:5 | LL | (0..).all(|x| x == 24); | ^^^^^^^^^^^^^^^^^^^^^^ error: infinite iteration detected - --> tests/ui/infinite_iter.rs:107:31 + --> tests/ui/infinite_iter.rs:108:31 | LL | let _: HashSet = (0..).collect(); | ^^^^^^^^^^^^^^^ diff --git a/tests/ui/iter_kv_map.fixed b/tests/ui/iter_kv_map.fixed index 7fcab6592e26..f1e6039655e4 100644 --- a/tests/ui/iter_kv_map.fixed +++ b/tests/ui/iter_kv_map.fixed @@ -1,5 +1,6 @@ #![warn(clippy::iter_kv_map)] #![allow(unused_mut, clippy::redundant_clone, clippy::suspicious_map, clippy::map_identity)] +#![feature(unsigned_is_multiple_of)] use std::collections::{BTreeMap, HashMap}; @@ -30,15 +31,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 +89,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 b590aef7b803..c4b45710cb13 100644 --- a/tests/ui/iter_kv_map.rs +++ b/tests/ui/iter_kv_map.rs @@ -1,5 +1,6 @@ #![warn(clippy::iter_kv_map)] #![allow(unused_mut, clippy::redundant_clone, clippy::suspicious_map, clippy::map_identity)] +#![feature(unsigned_is_multiple_of)] use std::collections::{BTreeMap, HashMap}; @@ -30,15 +31,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 +91,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 00d566ed14a2..90809c991750 100644 --- a/tests/ui/iter_kv_map.stderr +++ b/tests/ui/iter_kv_map.stderr @@ -1,5 +1,5 @@ error: iterating on a map's keys - --> tests/ui/iter_kv_map.rs:14:13 + --> tests/ui/iter_kv_map.rs:15:13 | LL | let _ = map.iter().map(|(key, _)| key).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.keys()` @@ -8,73 +8,73 @@ LL | let _ = map.iter().map(|(key, _)| key).collect::>(); = help: to override `-D warnings` add `#[allow(clippy::iter_kv_map)]` error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:16:13 + --> tests/ui/iter_kv_map.rs:17: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:18:13 + --> tests/ui/iter_kv_map.rs:19: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:21:13 + --> tests/ui/iter_kv_map.rs:22: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:23:13 + --> tests/ui/iter_kv_map.rs:24: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:26:13 + --> tests/ui/iter_kv_map.rs:27: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:28:13 + --> tests/ui/iter_kv_map.rs:29: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:31:13 + --> tests/ui/iter_kv_map.rs:32: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:33:13 + --> tests/ui/iter_kv_map.rs:34: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:49: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:51: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:55: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:59: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:70: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:75: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:77: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:79: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:82: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:84: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:87: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:89: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:92: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:94: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:109: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:111: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:115: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:119:13 | LL | let _ = map | _____________^ @@ -198,67 +198,67 @@ LL + }) | error: iterating on a map's values - --> tests/ui/iter_kv_map.rs:121:13 + --> tests/ui/iter_kv_map.rs:130: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:146: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:149: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:152: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:161: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:164: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:167: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:170: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:173: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:176: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:179:13 | LL | let _ = map.iter().map(|(_, v)| v + 2).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `map.values().map(|v| v + 2)` diff --git a/tests/ui/let_unit.fixed b/tests/ui/let_unit.fixed index 5e7a2ad37a84..f27d5c4b3509 100644 --- a/tests/ui/let_unit.fixed +++ b/tests/ui/let_unit.fixed @@ -1,5 +1,6 @@ #![warn(clippy::let_unit_value)] #![allow(unused, clippy::no_effect, clippy::needless_late_init, path_statements)] +#![feature(unsigned_is_multiple_of)] macro_rules! let_and_return { ($n:expr) => {{ @@ -61,7 +62,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..4452d6e50027 100644 --- a/tests/ui/let_unit.rs +++ b/tests/ui/let_unit.rs @@ -1,5 +1,6 @@ #![warn(clippy::let_unit_value)] #![allow(unused, clippy::no_effect, clippy::needless_late_init, path_statements)] +#![feature(unsigned_is_multiple_of)] macro_rules! let_and_return { ($n:expr) => {{ @@ -61,7 +62,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..6dcffd5ec1ed 100644 --- a/tests/ui/let_unit.stderr +++ b/tests/ui/let_unit.stderr @@ -1,5 +1,5 @@ error: this let-binding has unit value - --> tests/ui/let_unit.rs:11:5 + --> tests/ui/let_unit.rs:12:5 | LL | let _x = println!("x"); | ^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `println!("x");` @@ -8,7 +8,7 @@ LL | let _x = println!("x"); = help: to override `-D warnings` add `#[allow(clippy::let_unit_value)]` error: this let-binding has unit value - --> tests/ui/let_unit.rs:60:5 + --> tests/ui/let_unit.rs:61:5 | LL | / let _ = v LL | | @@ -25,14 +25,14 @@ 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(); | error: this let-binding has unit value - --> tests/ui/let_unit.rs:110:5 + --> tests/ui/let_unit.rs:111:5 | LL | / let x = match Some(0) { LL | | @@ -55,7 +55,7 @@ LL + }; | error: this let-binding has unit value - --> tests/ui/let_unit.rs:192:9 + --> tests/ui/let_unit.rs:193:9 | LL | let res = returns_unit(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/manual_contains.fixed b/tests/ui/manual_contains.fixed index d26c948a7817..436db43356cf 100644 --- a/tests/ui/manual_contains.fixed +++ b/tests/ui/manual_contains.fixed @@ -1,5 +1,6 @@ #![warn(clippy::manual_contains)] #![allow(clippy::eq_op, clippy::useless_vec)] +#![feature(unsigned_is_multiple_of)] fn should_lint() { let vec: Vec = vec![1, 2, 3, 4, 5, 6]; @@ -58,7 +59,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..230fc6460d71 100644 --- a/tests/ui/manual_contains.rs +++ b/tests/ui/manual_contains.rs @@ -1,5 +1,6 @@ #![warn(clippy::manual_contains)] #![allow(clippy::eq_op, clippy::useless_vec)] +#![feature(unsigned_is_multiple_of)] fn should_lint() { let vec: Vec = vec![1, 2, 3, 4, 5, 6]; @@ -58,7 +59,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.stderr b/tests/ui/manual_contains.stderr index e6e2dea560c6..dd62f308a4ec 100644 --- a/tests/ui/manual_contains.stderr +++ b/tests/ui/manual_contains.stderr @@ -1,5 +1,5 @@ error: using `contains()` instead of `iter().any()` is more efficient - --> tests/ui/manual_contains.rs:7:13 + --> tests/ui/manual_contains.rs:8:13 | LL | let _ = values.iter().any(|&v| v == 4); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `values.contains(&4)` @@ -8,67 +8,67 @@ LL | let _ = values.iter().any(|&v| v == 4); = help: to override `-D warnings` add `#[allow(clippy::manual_contains)]` error: using `contains()` instead of `iter().any()` is more efficient - --> tests/ui/manual_contains.rs:12:13 + --> tests/ui/manual_contains.rs:13:13 | LL | let _ = values.iter().any(|&v| v == 4); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `values.contains(&4)` error: using `contains()` instead of `iter().any()` is more efficient - --> tests/ui/manual_contains.rs:16:13 + --> tests/ui/manual_contains.rs:17:13 | LL | let _ = values.iter().any(|&v| v == 10); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `values.contains(&10)` error: using `contains()` instead of `iter().any()` is more efficient - --> tests/ui/manual_contains.rs:21:13 + --> tests/ui/manual_contains.rs:22:13 | LL | let _ = values.iter().any(|&v| v == num); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `values.contains(&num)` error: using `contains()` instead of `iter().any()` is more efficient - --> tests/ui/manual_contains.rs:26:13 + --> tests/ui/manual_contains.rs:27:13 | LL | let _ = values.iter().any(|&v| num == v); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `values.contains(&num)` error: using `contains()` instead of `iter().any()` is more efficient - --> tests/ui/manual_contains.rs:30:13 + --> tests/ui/manual_contains.rs:31:13 | LL | let _ = values.iter().any(|v| *v == 4); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `values.contains(&4)` error: using `contains()` instead of `iter().any()` is more efficient - --> tests/ui/manual_contains.rs:35:13 + --> tests/ui/manual_contains.rs:36:13 | LL | let _ = values.iter().any(|&v| 4 == v); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `values.contains(&4)` error: using `contains()` instead of `iter().any()` is more efficient - --> tests/ui/manual_contains.rs:41:13 + --> tests/ui/manual_contains.rs:42:13 | LL | let _ = values.iter().any(|v| *v == *a); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `values.contains(a)` error: using `contains()` instead of `iter().any()` is more efficient - --> tests/ui/manual_contains.rs:46:13 + --> tests/ui/manual_contains.rs:47:13 | LL | let _ = values.iter().any(|&v| v == "4"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `values.contains(&"4")` error: using `contains()` instead of `iter().any()` is more efficient - --> tests/ui/manual_contains.rs:51:13 + --> tests/ui/manual_contains.rs:52:13 | LL | let _ = values.iter().any(|&v| v == 4 + 1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `values.contains(&(4 + 1))` error: using `contains()` instead of `iter().any()` is more efficient - --> tests/ui/manual_contains.rs:91:5 + --> tests/ui/manual_contains.rs:92:5 | LL | values.iter().any(|&v| v == 10) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `values.contains(&10)` error: using `contains()` instead of `iter().any()` is more efficient - --> tests/ui/manual_contains.rs:96:5 + --> tests/ui/manual_contains.rs:97:5 | LL | values.iter().any(|&v| v == 10) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `values.contains(&10)` diff --git a/tests/ui/manual_find_fixable.fixed b/tests/ui/manual_find_fixable.fixed index 5e6849a4dfb0..ada9a55aaaf4 100644 --- a/tests/ui/manual_find_fixable.fixed +++ b/tests/ui/manual_find_fixable.fixed @@ -1,6 +1,7 @@ #![warn(clippy::manual_find)] #![allow(unused)] #![allow(clippy::needless_return, clippy::uninlined_format_args)] +#![feature(unsigned_is_multiple_of)] use std::collections::HashMap; @@ -11,7 +12,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 +64,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..ccd05aea46c4 100644 --- a/tests/ui/manual_find_fixable.rs +++ b/tests/ui/manual_find_fixable.rs @@ -1,6 +1,7 @@ #![warn(clippy::manual_find)] #![allow(unused)] #![allow(clippy::needless_return, clippy::uninlined_format_args)] +#![feature(unsigned_is_multiple_of)] use std::collections::HashMap; @@ -19,7 +20,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 +112,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..26deb008688a 100644 --- a/tests/ui/manual_find_fixable.stderr +++ b/tests/ui/manual_find_fixable.stderr @@ -1,5 +1,5 @@ error: manual implementation of `Iterator::find` - --> tests/ui/manual_find_fixable.rs:10:5 + --> tests/ui/manual_find_fixable.rs:11:5 | LL | / for &v in ARRAY { LL | | @@ -13,18 +13,18 @@ LL | | None = help: to override `-D warnings` add `#[allow(clippy::manual_find)]` error: manual implementation of `Iterator::find` - --> tests/ui/manual_find_fixable.rs:20:5 + --> tests/ui/manual_find_fixable.rs:21:5 | 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 + --> tests/ui/manual_find_fixable.rs:35:5 | LL | / for el in arr { LL | | @@ -37,7 +37,7 @@ LL | | None = note: you may need to dereference some variables error: manual implementation of `Iterator::find` - --> tests/ui/manual_find_fixable.rs:45:5 + --> tests/ui/manual_find_fixable.rs:46:5 | LL | / for Tuple(a, _) in arr { LL | | @@ -48,7 +48,7 @@ LL | | None | |________^ help: replace with an iterator: `arr.into_iter().map(|Tuple(a, _)| a).find(|&a| a >= 3)` error: manual implementation of `Iterator::find` - --> tests/ui/manual_find_fixable.rs:61:5 + --> tests/ui/manual_find_fixable.rs:62:5 | LL | / for el in arr { LL | | @@ -61,7 +61,7 @@ LL | | None = note: you may need to dereference some variables error: manual implementation of `Iterator::find` - --> tests/ui/manual_find_fixable.rs:72:5 + --> tests/ui/manual_find_fixable.rs:73:5 | LL | / for el in arr { LL | | @@ -72,7 +72,7 @@ LL | | None | |________^ help: replace with an iterator: `arr.into_iter().find(|&el| f(el) == 20)` error: manual implementation of `Iterator::find` - --> tests/ui/manual_find_fixable.rs:83:5 + --> tests/ui/manual_find_fixable.rs:84:5 | LL | / for &el in arr.values() { LL | | @@ -83,7 +83,7 @@ LL | | None | |________^ help: replace with an iterator: `arr.values().find(|&&el| f(el)).copied()` error: manual implementation of `Iterator::find` - --> tests/ui/manual_find_fixable.rs:93:5 + --> tests/ui/manual_find_fixable.rs:94:5 | LL | / for el in arr { LL | | @@ -96,7 +96,7 @@ LL | | None = note: you may need to dereference some variables error: manual implementation of `Iterator::find` - --> tests/ui/manual_find_fixable.rs:124:5 + --> tests/ui/manual_find_fixable.rs:125:5 | LL | / for (_, &x) in v { LL | | @@ -107,7 +107,7 @@ LL | | None | |________^ help: replace with an iterator: `v.into_iter().map(|(_, &x)| x).find(|&x| x > 10)` error: manual implementation of `Iterator::find` - --> tests/ui/manual_find_fixable.rs:134:5 + --> tests/ui/manual_find_fixable.rs:135:5 | LL | / for &(_, &x) in v { LL | | @@ -118,7 +118,7 @@ LL | | None | |________^ help: replace with an iterator: `v.iter().map(|&(_, &x)| x).find(|&x| x > 10)` error: manual implementation of `Iterator::find` - --> tests/ui/manual_find_fixable.rs:144:5 + --> tests/ui/manual_find_fixable.rs:145:5 | LL | / for x in arr { LL | | @@ -129,7 +129,7 @@ LL | | return None; | |________________^ help: replace with an iterator: `arr.into_iter().find(|&x| x >= 5)` error: manual implementation of `Iterator::find` - --> tests/ui/manual_find_fixable.rs:200:9 + --> tests/ui/manual_find_fixable.rs:201:9 | LL | / for x in arr { LL | | From b5db45e40028d93f528126cb5352f8f65171b994 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Tue, 25 Feb 2025 09:20:55 +0100 Subject: [PATCH 3/3] New lint: `manual_is_multiple_of` --- CHANGELOG.md | 2 + book/src/lint_configuration.md | 10 ++ clippy_config/src/conf.rs | 3 + clippy_lints/src/casts/cast_sign_loss.rs | 2 +- clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 1 + .../src/operators/manual_is_multiple_of.rs | 115 ++++++++++++++++++ clippy_lints/src/operators/mod.rs | 47 +++++++ clippy_utils/src/msrvs.rs | 1 + .../ui-toml/manual_is_multiple_of/clippy.toml | 1 + .../manual_is_multiple_of.fixed | 9 ++ .../manual_is_multiple_of.rs | 9 ++ .../manual_is_multiple_of.stderr | 17 +++ .../toml_unknown_key/conf_unknown_key.stderr | 3 + tests/ui/manual_is_multiple_of.fixed | 33 +++++ tests/ui/manual_is_multiple_of.rs | 33 +++++ tests/ui/manual_is_multiple_of.stderr | 89 ++++++++++++++ 17 files changed, 375 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/operators/manual_is_multiple_of.rs create mode 100644 tests/ui-toml/manual_is_multiple_of/clippy.toml create mode 100644 tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.fixed create mode 100644 tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.rs create mode 100644 tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.stderr create mode 100644 tests/ui/manual_is_multiple_of.fixed create mode 100644 tests/ui/manual_is_multiple_of.rs create mode 100644 tests/ui/manual_is_multiple_of.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index be5e95e9744e..058ad150604e 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 7a70c6c40ac8..1206f5dfd3f1 100644 --- a/book/src/lint_configuration.md +++ b/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. diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 23d585a6c159..399f00702351 100644 --- a/clippy_config/src/conf.rs +++ b/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, diff --git a/clippy_lints/src/casts/cast_sign_loss.rs b/clippy_lints/src/casts/cast_sign_loss.rs index c8abf9dac9af..f0e3c48ca37d 100644 --- a/clippy_lints/src/casts/cast_sign_loss.rs +++ b/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. diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 9df9a62438ce..53f7299a998f 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f00ae5a9731c..58bfd41331f9 100644 --- a/clippy_lints/src/lib.rs +++ b/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( 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..61aa6596928a --- /dev/null +++ b/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 { + 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 + } +} diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index 9ad32c2bd396..8a7af7a02111 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 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 { diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 8c9832af0a19..9d3f772395fc 100644 --- a/clippy_utils/src/msrvs.rs +++ b/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 } diff --git a/tests/ui-toml/manual_is_multiple_of/clippy.toml b/tests/ui-toml/manual_is_multiple_of/clippy.toml new file mode 100644 index 000000000000..07fac21e0237 --- /dev/null +++ b/tests/ui-toml/manual_is_multiple_of/clippy.toml @@ -0,0 +1 @@ +min-and-mask-size = 0 diff --git a/tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.fixed b/tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.fixed new file mode 100644 index 000000000000..445a96efb23a --- /dev/null +++ b/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 +} diff --git a/tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.rs b/tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.rs new file mode 100644 index 000000000000..0c41dcd87db1 --- /dev/null +++ b/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 +} diff --git a/tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.stderr b/tests/ui-toml/manual_is_multiple_of/manual_is_multiple_of.stderr new file mode 100644 index 000000000000..23af8696e8a5 --- /dev/null +++ b/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 + diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 842059df1e92..c4b26c14c43c 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -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 @@ -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 @@ -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 diff --git a/tests/ui/manual_is_multiple_of.fixed b/tests/ui/manual_is_multiple_of.fixed new file mode 100644 index 000000000000..8ae923da71c2 --- /dev/null +++ b/tests/ui/manual_is_multiple_of.fixed @@ -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; +} diff --git a/tests/ui/manual_is_multiple_of.rs b/tests/ui/manual_is_multiple_of.rs new file mode 100644 index 000000000000..24583bc119c7 --- /dev/null +++ b/tests/ui/manual_is_multiple_of.rs @@ -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 % 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 & 4095 == 0; //~ manual_is_multiple_of + let _ = a & 4095 != 0; //~ manual_is_multiple_of + let _ = a & ((1 << b) - 1) == 0; //~ manual_is_multiple_of + let _ = a & ((1 << b) - 1) != 0; //~ manual_is_multiple_of + let _ = ((1 << b) - 1) & a == 0; //~ manual_is_multiple_of + + let _ = a % b > 0; //~ manual_is_multiple_of + let _ = 0 < a % b; //~ manual_is_multiple_of + + let _ = a & 0xff == 0; //~ 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 & 7 == 0; //~ manual_is_multiple_of + let _ = a & ((1 << 3) - 1) == 0; //~ manual_is_multiple_of +} + +#[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..60256fd27dad --- /dev/null +++ b/tests/ui/manual_is_multiple_of.stderr @@ -0,0 +1,89 @@ +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 & 4095 == 0; + | ^^^^^^^^^^^^^ help: replace with: `a.is_multiple_of(4096)` + +error: manual implementation of `.is_multiple_of()` + --> tests/ui/manual_is_multiple_of.rs:14:13 + | +LL | let _ = a & 4095 != 0; + | ^^^^^^^^^^^^^ help: replace with: `!a.is_multiple_of(4096)` + +error: manual implementation of `.is_multiple_of()` + --> tests/ui/manual_is_multiple_of.rs:15:13 + | +LL | let _ = a & ((1 << b) - 1) == 0; + | ^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `a.is_multiple_of(1 << b)` + +error: manual implementation of `.is_multiple_of()` + --> tests/ui/manual_is_multiple_of.rs:16:13 + | +LL | let _ = a & ((1 << b) - 1) != 0; + | ^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `!a.is_multiple_of(1 << b)` + +error: manual implementation of `.is_multiple_of()` + --> tests/ui/manual_is_multiple_of.rs:17:13 + | +LL | let _ = ((1 << b) - 1) & a == 0; + | ^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `a.is_multiple_of(1 << b)` + +error: manual implementation of `.is_multiple_of()` + --> tests/ui/manual_is_multiple_of.rs:19: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:20:13 + | +LL | let _ = 0 < a % b; + | ^^^^^^^^^ help: replace with: `!a.is_multiple_of(b)` + +error: manual implementation of `.is_multiple_of()` + --> tests/ui/manual_is_multiple_of.rs:22:13 + | +LL | let _ = a & 0xff == 0; + | ^^^^^^^^^^^^^ help: replace with: `a.is_multiple_of(0x100)` + +error: manual implementation of `.is_multiple_of()` + --> tests/ui/manual_is_multiple_of.rs:26:13 + | +LL | let _ = a & 7 == 0; + | ^^^^^^^^^^ help: replace with: `a.is_multiple_of(8)` + +error: manual implementation of `.is_multiple_of()` + --> tests/ui/manual_is_multiple_of.rs:27:13 + | +LL | let _ = a & ((1 << 3) - 1) == 0; + | ^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `a.is_multiple_of(1 << 3)` + +error: aborting due to 14 previous errors +