From e5ebd3edabc17436de086a7ce147983c2238939a Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Tue, 21 Jun 2022 14:13:15 -0400 Subject: [PATCH 01/10] Implement manual_rem_euclid lint --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_complexity.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/manual_rem_euclid.rs | 110 ++++++++++++++++++++ clippy_utils/src/msrvs.rs | 2 +- tests/ui/manual_rem_euclid.fixed | 17 +++ tests/ui/manual_rem_euclid.rs | 17 +++ tests/ui/manual_rem_euclid.stderr | 34 ++++++ tests/ui/min_rust_version_attr.rs | 6 ++ tests/ui/min_rust_version_attr.stderr | 8 +- 12 files changed, 195 insertions(+), 5 deletions(-) create mode 100644 clippy_lints/src/manual_rem_euclid.rs create mode 100644 tests/ui/manual_rem_euclid.fixed create mode 100644 tests/ui/manual_rem_euclid.rs create mode 100644 tests/ui/manual_rem_euclid.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index d64dd772ac54..dcc96bc10b8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3527,6 +3527,7 @@ Released 2018-09-13 [`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or [`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains +[`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic [`manual_split_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once [`manual_str_repeat`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 7876f21f6e6a..a1565255b0b5 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -135,6 +135,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(manual_async_fn::MANUAL_ASYNC_FN), LintId::of(manual_bits::MANUAL_BITS), LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE), + LintId::of(manual_rem_euclid::MANUAL_REM_EUCLID), LintId::of(manual_strip::MANUAL_STRIP), LintId::of(map_clone::MAP_CLONE), LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN), diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs index 4f1c3673f853..6370264a12a6 100644 --- a/clippy_lints/src/lib.register_complexity.rs +++ b/clippy_lints/src/lib.register_complexity.rs @@ -24,6 +24,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec! LintId::of(loops::MANUAL_FLATTEN), LintId::of(loops::SINGLE_ELEMENT_LOOP), LintId::of(loops::WHILE_LET_LOOP), + LintId::of(manual_rem_euclid::MANUAL_REM_EUCLID), LintId::of(manual_strip::MANUAL_STRIP), LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index de22f50cf943..f706ba0620fd 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -254,6 +254,7 @@ store.register_lints(&[ manual_bits::MANUAL_BITS, manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE, manual_ok_or::MANUAL_OK_OR, + manual_rem_euclid::MANUAL_REM_EUCLID, manual_strip::MANUAL_STRIP, map_clone::MAP_CLONE, map_err_ignore::MAP_ERR_IGNORE, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dcee11cc28cf..70cf6be8b7cf 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -282,6 +282,7 @@ mod manual_async_fn; mod manual_bits; mod manual_non_exhaustive; mod manual_ok_or; +mod manual_rem_euclid; mod manual_strip; mod map_clone; mod map_err_ignore; @@ -912,6 +913,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(as_underscore::AsUnderscore)); store.register_late_pass(|| Box::new(read_zero_byte_vec::ReadZeroByteVec)); store.register_late_pass(|| Box::new(default_instead_of_iter_empty::DefaultIterEmpty)); + store.register_late_pass(move || Box::new(manual_rem_euclid::ManualRemEuclid::new(msrv))); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/manual_rem_euclid.rs b/clippy_lints/src/manual_rem_euclid.rs new file mode 100644 index 000000000000..7ef8f78a5f60 --- /dev/null +++ b/clippy_lints/src/manual_rem_euclid.rs @@ -0,0 +1,110 @@ +use clippy_utils::consts::{constant_full_int, FullInt}; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::{meets_msrv, msrvs, path_to_local}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{BinOpKind, Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for an expression like `((x % 4) + 4) % 4` which is a common manual reimplementation + /// of `x.rem_euclid(4)`. + /// + /// ### Why is this bad? + /// It's simpler and more readable. + /// + /// ### Example + /// ```rust + /// let x = 24; + /// let rem = ((x % 4) + 4) % 4; + /// ``` + /// Use instead: + /// ```rust + /// let x = 24; + /// let rem = x.rem_euclid(4); + /// ``` + #[clippy::version = "1.63.0"] + pub MANUAL_REM_EUCLID, + complexity, + "manually reimplementing `rem_euclid`" +} + +pub struct ManualRemEuclid { + msrv: Option, +} + +impl ManualRemEuclid { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + +impl_lint_pass!(ManualRemEuclid => [MANUAL_REM_EUCLID]); + +impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if !meets_msrv(self.msrv, msrvs::REM_EUCLID) { + return; + } + + if_chain! { + if let ExprKind::Binary(op1, ..) = expr.kind; + if op1.node == BinOpKind::Rem; + if let Some((const1, expr1)) = check_for_positive_int_constant(cx, expr); + if let ExprKind::Binary(op2, ..) = expr1.kind; + if op2.node == BinOpKind::Add; + if let Some((const2, expr2)) = check_for_positive_int_constant(cx, expr1); + if let ExprKind::Binary(op3, ..) = expr2.kind; + if op3.node == BinOpKind::Rem; + if let Some((const3, expr3)) = check_for_positive_int_constant(cx, expr2); + if const1 == const2 && const2 == const3; + if path_to_local(expr3).is_some(); + then { + let mut app = Applicability::MachineApplicable; + let rem_of = snippet_with_applicability(cx, expr3.span, "_", &mut app); + span_lint_and_sugg( + cx, + MANUAL_REM_EUCLID, + expr.span, + "manual `rem_euclid` implementation", + "consider using", + format!("{rem_of}.rem_euclid({const1})"), + app, + ); + } + } + } + + extract_msrv_attr!(LateContext); +} + +// Takes a binary expression and separates the operands into the int constant and the other +// operand. Ensures the int constant is positive. +fn check_for_positive_int_constant<'a>(cx: &'a LateContext<'_>, expr: &'a Expr<'_>) -> Option<(u128, &'a Expr<'a>)> { + let (int_const, other_op) = if let ExprKind::Binary(_, left, right) = expr.kind { + if let Some(int_const) = constant_full_int(cx, cx.typeck_results(), left) { + (int_const, right) + } else if let Some(int_const) = constant_full_int(cx, cx.typeck_results(), right) { + (int_const, left) + } else { + return None; + } + } else { + return None; + }; + + if int_const > FullInt::S(0) { + let val = match int_const { + FullInt::S(s) => s.try_into().unwrap(), + FullInt::U(u) => u, + }; + Some((val, other_op)) + } else { + None + } +} diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index b9ec2c19fdd3..8000f9e21a1e 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -23,7 +23,7 @@ msrv_aliases! { 1,42,0 { MATCHES_MACRO, SLICE_PATTERNS, PTR_SLICE_RAW_PARTS } 1,41,0 { RE_REBALANCING_COHERENCE, RESULT_MAP_OR_ELSE } 1,40,0 { MEM_TAKE, NON_EXHAUSTIVE, OPTION_AS_DEREF } - 1,38,0 { POINTER_CAST } + 1,38,0 { POINTER_CAST, REM_EUCLID } 1,37,0 { TYPE_ALIAS_ENUM_VARIANTS } 1,36,0 { ITERATOR_COPIED } 1,35,0 { OPTION_COPIED, RANGE_CONTAINS } diff --git a/tests/ui/manual_rem_euclid.fixed b/tests/ui/manual_rem_euclid.fixed new file mode 100644 index 000000000000..f176fc9ad128 --- /dev/null +++ b/tests/ui/manual_rem_euclid.fixed @@ -0,0 +1,17 @@ +// run-rustfix + +#![warn(clippy::manual_rem_euclid)] + +fn main() { + let value: i32 = 5; + + let _: i32 = value.rem_euclid(4); + let _: i32 = value.rem_euclid(4); + let _: i32 = value.rem_euclid(4); + let _: i32 = value.rem_euclid(4); + let _: i32 = 1 + value.rem_euclid(4); + + let _: i32 = (3 + value % 4) % 4; + let _: i32 = (-4 + value % -4) % -4; + let _: i32 = ((5 % 4) + 4) % 4; +} diff --git a/tests/ui/manual_rem_euclid.rs b/tests/ui/manual_rem_euclid.rs new file mode 100644 index 000000000000..b243065de87f --- /dev/null +++ b/tests/ui/manual_rem_euclid.rs @@ -0,0 +1,17 @@ +// run-rustfix + +#![warn(clippy::manual_rem_euclid)] + +fn main() { + let value: i32 = 5; + + let _: i32 = ((value % 4) + 4) % 4; + let _: i32 = (4 + (value % 4)) % 4; + let _: i32 = (value % 4 + 4) % 4; + let _: i32 = (4 + value % 4) % 4; + let _: i32 = 1 + (4 + value % 4) % 4; + + let _: i32 = (3 + value % 4) % 4; + let _: i32 = (-4 + value % -4) % -4; + let _: i32 = ((5 % 4) + 4) % 4; +} diff --git a/tests/ui/manual_rem_euclid.stderr b/tests/ui/manual_rem_euclid.stderr new file mode 100644 index 000000000000..7134759dcc9f --- /dev/null +++ b/tests/ui/manual_rem_euclid.stderr @@ -0,0 +1,34 @@ +error: manual `rem_euclid` implementation + --> $DIR/manual_rem_euclid.rs:8:18 + | +LL | let _: i32 = ((value % 4) + 4) % 4; + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)` + | + = note: `-D clippy::manual-rem-euclid` implied by `-D warnings` + +error: manual `rem_euclid` implementation + --> $DIR/manual_rem_euclid.rs:9:18 + | +LL | let _: i32 = (4 + (value % 4)) % 4; + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)` + +error: manual `rem_euclid` implementation + --> $DIR/manual_rem_euclid.rs:10:18 + | +LL | let _: i32 = (value % 4 + 4) % 4; + | ^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)` + +error: manual `rem_euclid` implementation + --> $DIR/manual_rem_euclid.rs:11:18 + | +LL | let _: i32 = (4 + value % 4) % 4; + | ^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)` + +error: manual `rem_euclid` implementation + --> $DIR/manual_rem_euclid.rs:12:22 + | +LL | let _: i32 = 1 + (4 + value % 4) % 4; + | ^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)` + +error: aborting due to 5 previous errors + diff --git a/tests/ui/min_rust_version_attr.rs b/tests/ui/min_rust_version_attr.rs index f83c3e0e281c..301a8e54ccfe 100644 --- a/tests/ui/min_rust_version_attr.rs +++ b/tests/ui/min_rust_version_attr.rs @@ -155,6 +155,11 @@ fn cast_abs_to_unsigned() { assert_eq!(10u32, x.abs() as u32); } +fn manual_rem_euclid() { + let x: i32 = 10; + let _: i32 = ((x % 4) + 4) % 4; +} + fn main() { filter_map_next(); checked_conversion(); @@ -174,6 +179,7 @@ fn main() { int_from_bool(); err_expect(); cast_abs_to_unsigned(); + manual_rem_euclid(); } mod just_under_msrv { diff --git a/tests/ui/min_rust_version_attr.stderr b/tests/ui/min_rust_version_attr.stderr index de225eb740d0..b1c23b539ffd 100644 --- a/tests/ui/min_rust_version_attr.stderr +++ b/tests/ui/min_rust_version_attr.stderr @@ -1,12 +1,12 @@ error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:198:24 + --> $DIR/min_rust_version_attr.rs:204:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::manual-strip` implied by `-D warnings` note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:197:9 + --> $DIR/min_rust_version_attr.rs:203:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,13 +17,13 @@ LL ~ assert_eq!(.to_uppercase(), "WORLD!"); | error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:210:24 + --> $DIR/min_rust_version_attr.rs:216:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:209:9 + --> $DIR/min_rust_version_attr.rs:215:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 6e1df4732b6fb877361b0dd3b209f194481bbe7b Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Tue, 21 Jun 2022 16:41:36 -0400 Subject: [PATCH 02/10] Fix case where suggestion errored for infer type --- clippy_lints/src/manual_rem_euclid.rs | 15 +++++++++++---- tests/ui/manual_rem_euclid.fixed | 7 +++++++ tests/ui/manual_rem_euclid.rs | 7 +++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/manual_rem_euclid.rs b/clippy_lints/src/manual_rem_euclid.rs index 7ef8f78a5f60..cba03389a9b8 100644 --- a/clippy_lints/src/manual_rem_euclid.rs +++ b/clippy_lints/src/manual_rem_euclid.rs @@ -4,7 +4,7 @@ use clippy_utils::source::snippet_with_applicability; use clippy_utils::{meets_msrv, msrvs, path_to_local}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{BinOpKind, Expr, ExprKind}; +use rustc_hir::{BinOpKind, Expr, ExprKind, Node, TyKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -19,12 +19,12 @@ declare_clippy_lint! { /// /// ### Example /// ```rust - /// let x = 24; + /// let x: i32 = 24; /// let rem = ((x % 4) + 4) % 4; /// ``` /// Use instead: /// ```rust - /// let x = 24; + /// let x: i32 = 24; /// let rem = x.rem_euclid(4); /// ``` #[clippy::version = "1.63.0"] @@ -63,7 +63,14 @@ impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid { if op3.node == BinOpKind::Rem; if let Some((const3, expr3)) = check_for_positive_int_constant(cx, expr2); if const1 == const2 && const2 == const3; - if path_to_local(expr3).is_some(); + // Only apply if we see an explicit type annotation on the local. + if let Some(hir_id) = path_to_local(expr3); + let hir = cx.tcx.hir(); + if let Some(Node::Binding(_)) = hir.find(hir_id); + let parent = hir.get_parent_node(hir_id); + if let Some(Node::Local(local)) = hir.find(parent); + if let Some(ty) = local.ty; + if !matches!(ty.kind, TyKind::Infer); then { let mut app = Applicability::MachineApplicable; let rem_of = snippet_with_applicability(cx, expr3.span, "_", &mut app); diff --git a/tests/ui/manual_rem_euclid.fixed b/tests/ui/manual_rem_euclid.fixed index f176fc9ad128..10a10e812f42 100644 --- a/tests/ui/manual_rem_euclid.fixed +++ b/tests/ui/manual_rem_euclid.fixed @@ -14,4 +14,11 @@ fn main() { let _: i32 = (3 + value % 4) % 4; let _: i32 = (-4 + value % -4) % -4; let _: i32 = ((5 % 4) + 4) % 4; + + // Make sure the lint does not trigger if it would cause an error, like with an ambiguous + // integer type + let not_annotated = 24; + let _ = ((not_annotated % 4) + 4) % 4; + let inferred: _ = 24; + let _ = ((inferred % 4) + 4) % 4; } diff --git a/tests/ui/manual_rem_euclid.rs b/tests/ui/manual_rem_euclid.rs index b243065de87f..bf16f977ea94 100644 --- a/tests/ui/manual_rem_euclid.rs +++ b/tests/ui/manual_rem_euclid.rs @@ -14,4 +14,11 @@ fn main() { let _: i32 = (3 + value % 4) % 4; let _: i32 = (-4 + value % -4) % -4; let _: i32 = ((5 % 4) + 4) % 4; + + // Make sure the lint does not trigger if it would cause an error, like with an ambiguous + // integer type + let not_annotated = 24; + let _ = ((not_annotated % 4) + 4) % 4; + let inferred: _ = 24; + let _ = ((inferred % 4) + 4) % 4; } From 75ed0c9f267c8e65d3bbd45c6e10acd11e702077 Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Wed, 22 Jun 2022 13:15:35 -0400 Subject: [PATCH 03/10] Prefer `.ok()?` over `.unwrap()` --- clippy_lints/src/manual_rem_euclid.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/manual_rem_euclid.rs b/clippy_lints/src/manual_rem_euclid.rs index cba03389a9b8..d6b74b3e1ca5 100644 --- a/clippy_lints/src/manual_rem_euclid.rs +++ b/clippy_lints/src/manual_rem_euclid.rs @@ -107,7 +107,7 @@ fn check_for_positive_int_constant<'a>(cx: &'a LateContext<'_>, expr: &'a Expr<' if int_const > FullInt::S(0) { let val = match int_const { - FullInt::S(s) => s.try_into().unwrap(), + FullInt::S(s) => s.try_into().ok()?, FullInt::U(u) => u, }; Some((val, other_op)) From 93e41d3305c683a4ed19ff3001832f8c84090310 Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Wed, 22 Jun 2022 13:26:13 -0400 Subject: [PATCH 04/10] Fix case where rem was considered commutative --- clippy_lints/src/manual_rem_euclid.rs | 21 +++++++++++++-------- tests/ui/manual_rem_euclid.fixed | 4 ++++ tests/ui/manual_rem_euclid.rs | 4 ++++ 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/manual_rem_euclid.rs b/clippy_lints/src/manual_rem_euclid.rs index d6b74b3e1ca5..9b31e70e3f06 100644 --- a/clippy_lints/src/manual_rem_euclid.rs +++ b/clippy_lints/src/manual_rem_euclid.rs @@ -55,13 +55,13 @@ impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid { if_chain! { if let ExprKind::Binary(op1, ..) = expr.kind; if op1.node == BinOpKind::Rem; - if let Some((const1, expr1)) = check_for_positive_int_constant(cx, expr); + if let Some((const1, expr1)) = check_for_positive_int_constant(cx, expr, false); if let ExprKind::Binary(op2, ..) = expr1.kind; if op2.node == BinOpKind::Add; - if let Some((const2, expr2)) = check_for_positive_int_constant(cx, expr1); + if let Some((const2, expr2)) = check_for_positive_int_constant(cx, expr1, true); if let ExprKind::Binary(op3, ..) = expr2.kind; if op3.node == BinOpKind::Rem; - if let Some((const3, expr3)) = check_for_positive_int_constant(cx, expr2); + if let Some((const3, expr3)) = check_for_positive_int_constant(cx, expr2, false); if const1 == const2 && const2 == const3; // Only apply if we see an explicit type annotation on the local. if let Some(hir_id) = path_to_local(expr3); @@ -91,13 +91,18 @@ impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid { } // Takes a binary expression and separates the operands into the int constant and the other -// operand. Ensures the int constant is positive. -fn check_for_positive_int_constant<'a>(cx: &'a LateContext<'_>, expr: &'a Expr<'_>) -> Option<(u128, &'a Expr<'a>)> { +// operand. Ensures the int constant is positive. Operators that are not commutative must have the +// constant appear on the right hand side to return a value. +fn check_for_positive_int_constant<'a>( + cx: &'a LateContext<'_>, + expr: &'a Expr<'_>, + is_commutative: bool, +) -> Option<(u128, &'a Expr<'a>)> { let (int_const, other_op) = if let ExprKind::Binary(_, left, right) = expr.kind { - if let Some(int_const) = constant_full_int(cx, cx.typeck_results(), left) { - (int_const, right) - } else if let Some(int_const) = constant_full_int(cx, cx.typeck_results(), right) { + if let Some(int_const) = constant_full_int(cx, cx.typeck_results(), right) { (int_const, left) + } else if is_commutative && let Some(int_const) = constant_full_int(cx, cx.typeck_results(), left) { + (int_const, right) } else { return None; } diff --git a/tests/ui/manual_rem_euclid.fixed b/tests/ui/manual_rem_euclid.fixed index 10a10e812f42..d0aa23988116 100644 --- a/tests/ui/manual_rem_euclid.fixed +++ b/tests/ui/manual_rem_euclid.fixed @@ -21,4 +21,8 @@ fn main() { let _ = ((not_annotated % 4) + 4) % 4; let inferred: _ = 24; let _ = ((inferred % 4) + 4) % 4; + + // For lint to apply the constant must always be on the RHS of the previous value for % + let _: i32 = 4 % ((value % 4) + 4); + let _: i32 = ((4 % value) + 4) % 4; } diff --git a/tests/ui/manual_rem_euclid.rs b/tests/ui/manual_rem_euclid.rs index bf16f977ea94..ff87d6fab52c 100644 --- a/tests/ui/manual_rem_euclid.rs +++ b/tests/ui/manual_rem_euclid.rs @@ -21,4 +21,8 @@ fn main() { let _ = ((not_annotated % 4) + 4) % 4; let inferred: _ = 24; let _ = ((inferred % 4) + 4) % 4; + + // For lint to apply the constant must always be on the RHS of the previous value for % + let _: i32 = 4 % ((value % 4) + 4); + let _: i32 = ((4 % value) + 4) % 4; } From c8df6d6970749abc8ee3b3b5f78d4c1a9ad33bb6 Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Wed, 22 Jun 2022 13:31:04 -0400 Subject: [PATCH 05/10] Prefer if let chain over macro --- clippy_lints/src/manual_rem_euclid.rs | 36 +++++++++++---------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/clippy_lints/src/manual_rem_euclid.rs b/clippy_lints/src/manual_rem_euclid.rs index 9b31e70e3f06..3ffd9a07376d 100644 --- a/clippy_lints/src/manual_rem_euclid.rs +++ b/clippy_lints/src/manual_rem_euclid.rs @@ -2,7 +2,6 @@ use clippy_utils::consts::{constant_full_int, FullInt}; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; use clippy_utils::{meets_msrv, msrvs, path_to_local}; -use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind, Node, TyKind}; use rustc_lint::{LateContext, LateLintPass}; @@ -52,26 +51,22 @@ impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid { return; } - if_chain! { - if let ExprKind::Binary(op1, ..) = expr.kind; - if op1.node == BinOpKind::Rem; - if let Some((const1, expr1)) = check_for_positive_int_constant(cx, expr, false); - if let ExprKind::Binary(op2, ..) = expr1.kind; - if op2.node == BinOpKind::Add; - if let Some((const2, expr2)) = check_for_positive_int_constant(cx, expr1, true); - if let ExprKind::Binary(op3, ..) = expr2.kind; - if op3.node == BinOpKind::Rem; - if let Some((const3, expr3)) = check_for_positive_int_constant(cx, expr2, false); - if const1 == const2 && const2 == const3; + if let ExprKind::Binary(op1, ..) = expr.kind + && op1.node == BinOpKind::Rem + && let Some((const1, expr1)) = check_for_positive_int_constant(cx, expr, false) + && let ExprKind::Binary(op2, ..) = expr1.kind + && op2.node == BinOpKind::Add + && let Some((const2, expr2)) = check_for_positive_int_constant(cx, expr1, true) + && let ExprKind::Binary(op3, ..) = expr2.kind + && op3.node == BinOpKind::Rem + && let Some((const3, expr3)) = check_for_positive_int_constant(cx, expr2, false) + && const1 == const2 && const2 == const3 // Only apply if we see an explicit type annotation on the local. - if let Some(hir_id) = path_to_local(expr3); - let hir = cx.tcx.hir(); - if let Some(Node::Binding(_)) = hir.find(hir_id); - let parent = hir.get_parent_node(hir_id); - if let Some(Node::Local(local)) = hir.find(parent); - if let Some(ty) = local.ty; - if !matches!(ty.kind, TyKind::Infer); - then { + && let Some(hir_id) = path_to_local(expr3) + && let Some(Node::Binding(_)) = cx.tcx.hir().find(hir_id) + && let Some(Node::Local(local)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) + && let Some(ty) = local.ty + && !matches!(ty.kind, TyKind::Infer) { let mut app = Applicability::MachineApplicable; let rem_of = snippet_with_applicability(cx, expr3.span, "_", &mut app); span_lint_and_sugg( @@ -83,7 +78,6 @@ impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid { format!("{rem_of}.rem_euclid({const1})"), app, ); - } } } From 90f8277fe3ce6d0e238feac3bf10891f8c18f23a Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Wed, 22 Jun 2022 14:08:47 -0400 Subject: [PATCH 06/10] Fix case for function params --- clippy_lints/src/manual_rem_euclid.rs | 21 ++++++++++++++++----- tests/ui/manual_rem_euclid.fixed | 5 +++++ tests/ui/manual_rem_euclid.rs | 5 +++++ tests/ui/manual_rem_euclid.stderr | 8 +++++++- 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/manual_rem_euclid.rs b/clippy_lints/src/manual_rem_euclid.rs index 3ffd9a07376d..c338447686f2 100644 --- a/clippy_lints/src/manual_rem_euclid.rs +++ b/clippy_lints/src/manual_rem_euclid.rs @@ -61,12 +61,23 @@ impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid { && op3.node == BinOpKind::Rem && let Some((const3, expr3)) = check_for_positive_int_constant(cx, expr2, false) && const1 == const2 && const2 == const3 - // Only apply if we see an explicit type annotation on the local. && let Some(hir_id) = path_to_local(expr3) - && let Some(Node::Binding(_)) = cx.tcx.hir().find(hir_id) - && let Some(Node::Local(local)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) - && let Some(ty) = local.ty - && !matches!(ty.kind, TyKind::Infer) { + && let Some(Node::Binding(_)) = cx.tcx.hir().find(hir_id) { + // Apply only to params or locals with annotated types + match cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { + Some(Node::Param(..)) => (), + Some(Node::Local(local)) => { + if let Some(ty) = local.ty { + if matches!(ty.kind, TyKind::Infer) { + return; + } + } else { + return; + } + } + _ => return, + }; + let mut app = Applicability::MachineApplicable; let rem_of = snippet_with_applicability(cx, expr3.span, "_", &mut app); span_lint_and_sugg( diff --git a/tests/ui/manual_rem_euclid.fixed b/tests/ui/manual_rem_euclid.fixed index d0aa23988116..7dcd78f46336 100644 --- a/tests/ui/manual_rem_euclid.fixed +++ b/tests/ui/manual_rem_euclid.fixed @@ -26,3 +26,8 @@ fn main() { let _: i32 = 4 % ((value % 4) + 4); let _: i32 = ((4 % value) + 4) % 4; } + +// Should lint for params too +pub fn rem_euclid_4(num: i32) -> i32 { + num.rem_euclid(4) +} diff --git a/tests/ui/manual_rem_euclid.rs b/tests/ui/manual_rem_euclid.rs index ff87d6fab52c..f5d88ed6dfc5 100644 --- a/tests/ui/manual_rem_euclid.rs +++ b/tests/ui/manual_rem_euclid.rs @@ -26,3 +26,8 @@ fn main() { let _: i32 = 4 % ((value % 4) + 4); let _: i32 = ((4 % value) + 4) % 4; } + +// Should lint for params too +pub fn rem_euclid_4(num: i32) -> i32 { + ((num % 4) + 4) % 4 +} diff --git a/tests/ui/manual_rem_euclid.stderr b/tests/ui/manual_rem_euclid.stderr index 7134759dcc9f..bad6e66beef7 100644 --- a/tests/ui/manual_rem_euclid.stderr +++ b/tests/ui/manual_rem_euclid.stderr @@ -30,5 +30,11 @@ error: manual `rem_euclid` implementation LL | let _: i32 = 1 + (4 + value % 4) % 4; | ^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)` -error: aborting due to 5 previous errors +error: manual `rem_euclid` implementation + --> $DIR/manual_rem_euclid.rs:32:5 + | +LL | ((num % 4) + 4) % 4 + | ^^^^^^^^^^^^^^^^^^^ help: consider using: `num.rem_euclid(4)` + +error: aborting due to 6 previous errors From 61e1870aff5d77088e487b1506cbf537ac81e63a Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Wed, 22 Jun 2022 14:17:52 -0400 Subject: [PATCH 07/10] Add MSRV check for const rem_euclid --- clippy_lints/src/manual_rem_euclid.rs | 6 +++++- clippy_utils/src/msrvs.rs | 2 +- tests/ui/manual_rem_euclid.fixed | 5 +++++ tests/ui/manual_rem_euclid.rs | 5 +++++ tests/ui/manual_rem_euclid.stderr | 8 +++++++- 5 files changed, 23 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/manual_rem_euclid.rs b/clippy_lints/src/manual_rem_euclid.rs index c338447686f2..d22397ca35ec 100644 --- a/clippy_lints/src/manual_rem_euclid.rs +++ b/clippy_lints/src/manual_rem_euclid.rs @@ -1,7 +1,7 @@ use clippy_utils::consts::{constant_full_int, FullInt}; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::{meets_msrv, msrvs, path_to_local}; +use clippy_utils::{in_constant, meets_msrv, msrvs, path_to_local}; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind, Node, TyKind}; use rustc_lint::{LateContext, LateLintPass}; @@ -51,6 +51,10 @@ impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid { return; } + if in_constant(cx, expr.hir_id) && !meets_msrv(self.msrv, msrvs::REM_EUCLID_CONST) { + return; + } + if let ExprKind::Binary(op1, ..) = expr.kind && op1.node == BinOpKind::Rem && let Some((const1, expr1)) = check_for_positive_int_constant(cx, expr, false) diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 8000f9e21a1e..43c0a03c42ab 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -13,7 +13,7 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { 1,53,0 { OR_PATTERNS, MANUAL_BITS } - 1,52,0 { STR_SPLIT_ONCE } + 1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST } 1,51,0 { BORROW_AS_PTR, UNSIGNED_ABS } 1,50,0 { BOOL_THEN } 1,47,0 { TAU } diff --git a/tests/ui/manual_rem_euclid.fixed b/tests/ui/manual_rem_euclid.fixed index 7dcd78f46336..a3e593eac06c 100644 --- a/tests/ui/manual_rem_euclid.fixed +++ b/tests/ui/manual_rem_euclid.fixed @@ -31,3 +31,8 @@ fn main() { pub fn rem_euclid_4(num: i32) -> i32 { num.rem_euclid(4) } + +// Constant version came later, should still lint +pub const fn const_rem_euclid_4(num: i32) -> i32 { + num.rem_euclid(4) +} diff --git a/tests/ui/manual_rem_euclid.rs b/tests/ui/manual_rem_euclid.rs index f5d88ed6dfc5..2d8c6411f20b 100644 --- a/tests/ui/manual_rem_euclid.rs +++ b/tests/ui/manual_rem_euclid.rs @@ -31,3 +31,8 @@ fn main() { pub fn rem_euclid_4(num: i32) -> i32 { ((num % 4) + 4) % 4 } + +// Constant version came later, should still lint +pub const fn const_rem_euclid_4(num: i32) -> i32 { + ((num % 4) + 4) % 4 +} diff --git a/tests/ui/manual_rem_euclid.stderr b/tests/ui/manual_rem_euclid.stderr index bad6e66beef7..3faeb6c6a883 100644 --- a/tests/ui/manual_rem_euclid.stderr +++ b/tests/ui/manual_rem_euclid.stderr @@ -36,5 +36,11 @@ error: manual `rem_euclid` implementation LL | ((num % 4) + 4) % 4 | ^^^^^^^^^^^^^^^^^^^ help: consider using: `num.rem_euclid(4)` -error: aborting due to 6 previous errors +error: manual `rem_euclid` implementation + --> $DIR/manual_rem_euclid.rs:37:5 + | +LL | ((num % 4) + 4) % 4 + | ^^^^^^^^^^^^^^^^^^^ help: consider using: `num.rem_euclid(4)` + +error: aborting due to 7 previous errors From 0447cc7affa8c4086c12eafea1b1eda3883f6677 Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Wed, 22 Jun 2022 15:26:08 -0400 Subject: [PATCH 08/10] Simplify with let else --- clippy_lints/src/manual_rem_euclid.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/manual_rem_euclid.rs b/clippy_lints/src/manual_rem_euclid.rs index d22397ca35ec..29d6027ac02b 100644 --- a/clippy_lints/src/manual_rem_euclid.rs +++ b/clippy_lints/src/manual_rem_euclid.rs @@ -71,11 +71,8 @@ impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid { match cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { Some(Node::Param(..)) => (), Some(Node::Local(local)) => { - if let Some(ty) = local.ty { - if matches!(ty.kind, TyKind::Infer) { - return; - } - } else { + let Some(ty) = local.ty else { return }; + if matches!(ty.kind, TyKind::Infer) { return; } } From 92704b494a5c284cd426d8f386c60cd1ef2c48fd Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Thu, 23 Jun 2022 10:35:14 -0400 Subject: [PATCH 09/10] Split constant check functions and simplify --- clippy_lints/src/manual_rem_euclid.rs | 52 +++++++++++---------------- 1 file changed, 21 insertions(+), 31 deletions(-) diff --git a/clippy_lints/src/manual_rem_euclid.rs b/clippy_lints/src/manual_rem_euclid.rs index 29d6027ac02b..492bd4db4341 100644 --- a/clippy_lints/src/manual_rem_euclid.rs +++ b/clippy_lints/src/manual_rem_euclid.rs @@ -55,15 +55,16 @@ impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid { return; } - if let ExprKind::Binary(op1, ..) = expr.kind + if let ExprKind::Binary(op1, expr1, right) = expr.kind && op1.node == BinOpKind::Rem - && let Some((const1, expr1)) = check_for_positive_int_constant(cx, expr, false) - && let ExprKind::Binary(op2, ..) = expr1.kind + && let Some(const1) = check_for_unsigned_int_constant(cx, right) + && let ExprKind::Binary(op2, left, right) = expr1.kind && op2.node == BinOpKind::Add - && let Some((const2, expr2)) = check_for_positive_int_constant(cx, expr1, true) - && let ExprKind::Binary(op3, ..) = expr2.kind + && let Some((const2, expr2)) = check_for_either_unsigned_int_constant(cx, left, right) + && let ExprKind::Binary(op3, expr3, right) = expr2.kind && op3.node == BinOpKind::Rem - && let Some((const3, expr3)) = check_for_positive_int_constant(cx, expr2, false) + && let Some(const3) = check_for_unsigned_int_constant(cx, right) + // Also ensures the const is nonzero since zero can't be a divisor && const1 == const2 && const2 == const3 && let Some(hir_id) = path_to_local(expr3) && let Some(Node::Binding(_)) = cx.tcx.hir().find(hir_id) { @@ -96,33 +97,22 @@ impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid { extract_msrv_attr!(LateContext); } -// Takes a binary expression and separates the operands into the int constant and the other -// operand. Ensures the int constant is positive. Operators that are not commutative must have the -// constant appear on the right hand side to return a value. -fn check_for_positive_int_constant<'a>( +// Checks if either the left or right expressions can be an unsigned int constant and returns that +// constant along with the other expression unchanged if so +fn check_for_either_unsigned_int_constant<'a>( cx: &'a LateContext<'_>, - expr: &'a Expr<'_>, - is_commutative: bool, + left: &'a Expr<'_>, + right: &'a Expr<'_>, ) -> Option<(u128, &'a Expr<'a>)> { - let (int_const, other_op) = if let ExprKind::Binary(_, left, right) = expr.kind { - if let Some(int_const) = constant_full_int(cx, cx.typeck_results(), right) { - (int_const, left) - } else if is_commutative && let Some(int_const) = constant_full_int(cx, cx.typeck_results(), left) { - (int_const, right) - } else { - return None; - } - } else { - return None; - }; + check_for_unsigned_int_constant(cx, left) + .map(|int_const| (int_const, right)) + .or_else(|| check_for_unsigned_int_constant(cx, right).map(|int_const| (int_const, left))) +} - if int_const > FullInt::S(0) { - let val = match int_const { - FullInt::S(s) => s.try_into().ok()?, - FullInt::U(u) => u, - }; - Some((val, other_op)) - } else { - None +fn check_for_unsigned_int_constant<'a>(cx: &'a LateContext<'_>, expr: &'a Expr<'_>) -> Option { + let Some(int_const) = constant_full_int(cx, cx.typeck_results(), expr) else { return None }; + match int_const { + FullInt::S(s) => s.try_into().ok(), + FullInt::U(u) => Some(u), } } From df26c3f551fec5854e3acd140782b8f8c98e987b Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Thu, 23 Jun 2022 16:34:52 -0400 Subject: [PATCH 10/10] Add external macro guard and test middle MSRV --- clippy_lints/src/manual_rem_euclid.rs | 7 ++++++- tests/ui/auxiliary/macro_rules.rs | 8 ++++++++ tests/ui/manual_rem_euclid.fixed | 17 +++++++++++++++++ tests/ui/manual_rem_euclid.rs | 17 +++++++++++++++++ tests/ui/manual_rem_euclid.stderr | 27 +++++++++++++++++++-------- tests/ui/min_rust_version_attr.rs | 9 +++++++++ 6 files changed, 76 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/manual_rem_euclid.rs b/clippy_lints/src/manual_rem_euclid.rs index 492bd4db4341..b5698965fc3d 100644 --- a/clippy_lints/src/manual_rem_euclid.rs +++ b/clippy_lints/src/manual_rem_euclid.rs @@ -4,7 +4,8 @@ use clippy_utils::source::snippet_with_applicability; use clippy_utils::{in_constant, meets_msrv, msrvs, path_to_local}; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind, Node, TyKind}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -55,6 +56,10 @@ impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid { return; } + if in_external_macro(cx.sess(), expr.span) { + return; + } + if let ExprKind::Binary(op1, expr1, right) = expr.kind && op1.node == BinOpKind::Rem && let Some(const1) = check_for_unsigned_int_constant(cx, right) diff --git a/tests/ui/auxiliary/macro_rules.rs b/tests/ui/auxiliary/macro_rules.rs index 9f283337c7e1..5bd2c2799f03 100644 --- a/tests/ui/auxiliary/macro_rules.rs +++ b/tests/ui/auxiliary/macro_rules.rs @@ -127,3 +127,11 @@ macro_rules! ptr_as_ptr_cast { $ptr as *const i32 }; } + +#[macro_export] +macro_rules! manual_rem_euclid { + () => { + let value: i32 = 5; + let _: i32 = ((value % 4) + 4) % 4; + }; +} diff --git a/tests/ui/manual_rem_euclid.fixed b/tests/ui/manual_rem_euclid.fixed index a3e593eac06c..5601c96c10b2 100644 --- a/tests/ui/manual_rem_euclid.fixed +++ b/tests/ui/manual_rem_euclid.fixed @@ -1,7 +1,18 @@ // run-rustfix +// aux-build:macro_rules.rs #![warn(clippy::manual_rem_euclid)] +#[macro_use] +extern crate macro_rules; + +macro_rules! internal_rem_euclid { + () => { + let value: i32 = 5; + let _: i32 = value.rem_euclid(4); + }; +} + fn main() { let value: i32 = 5; @@ -25,6 +36,12 @@ fn main() { // For lint to apply the constant must always be on the RHS of the previous value for % let _: i32 = 4 % ((value % 4) + 4); let _: i32 = ((4 % value) + 4) % 4; + + // Lint in internal macros + internal_rem_euclid!(); + + // Do not lint in external macros + manual_rem_euclid!(); } // Should lint for params too diff --git a/tests/ui/manual_rem_euclid.rs b/tests/ui/manual_rem_euclid.rs index 2d8c6411f20b..52135be26b73 100644 --- a/tests/ui/manual_rem_euclid.rs +++ b/tests/ui/manual_rem_euclid.rs @@ -1,7 +1,18 @@ // run-rustfix +// aux-build:macro_rules.rs #![warn(clippy::manual_rem_euclid)] +#[macro_use] +extern crate macro_rules; + +macro_rules! internal_rem_euclid { + () => { + let value: i32 = 5; + let _: i32 = ((value % 4) + 4) % 4; + }; +} + fn main() { let value: i32 = 5; @@ -25,6 +36,12 @@ fn main() { // For lint to apply the constant must always be on the RHS of the previous value for % let _: i32 = 4 % ((value % 4) + 4); let _: i32 = ((4 % value) + 4) % 4; + + // Lint in internal macros + internal_rem_euclid!(); + + // Do not lint in external macros + manual_rem_euclid!(); } // Should lint for params too diff --git a/tests/ui/manual_rem_euclid.stderr b/tests/ui/manual_rem_euclid.stderr index 3faeb6c6a883..a237fd0213c1 100644 --- a/tests/ui/manual_rem_euclid.stderr +++ b/tests/ui/manual_rem_euclid.stderr @@ -1,5 +1,5 @@ error: manual `rem_euclid` implementation - --> $DIR/manual_rem_euclid.rs:8:18 + --> $DIR/manual_rem_euclid.rs:19:18 | LL | let _: i32 = ((value % 4) + 4) % 4; | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)` @@ -7,40 +7,51 @@ LL | let _: i32 = ((value % 4) + 4) % 4; = note: `-D clippy::manual-rem-euclid` implied by `-D warnings` error: manual `rem_euclid` implementation - --> $DIR/manual_rem_euclid.rs:9:18 + --> $DIR/manual_rem_euclid.rs:20:18 | LL | let _: i32 = (4 + (value % 4)) % 4; | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)` error: manual `rem_euclid` implementation - --> $DIR/manual_rem_euclid.rs:10:18 + --> $DIR/manual_rem_euclid.rs:21:18 | LL | let _: i32 = (value % 4 + 4) % 4; | ^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)` error: manual `rem_euclid` implementation - --> $DIR/manual_rem_euclid.rs:11:18 + --> $DIR/manual_rem_euclid.rs:22:18 | LL | let _: i32 = (4 + value % 4) % 4; | ^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)` error: manual `rem_euclid` implementation - --> $DIR/manual_rem_euclid.rs:12:22 + --> $DIR/manual_rem_euclid.rs:23:22 | LL | let _: i32 = 1 + (4 + value % 4) % 4; | ^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)` error: manual `rem_euclid` implementation - --> $DIR/manual_rem_euclid.rs:32:5 + --> $DIR/manual_rem_euclid.rs:12:22 + | +LL | let _: i32 = ((value % 4) + 4) % 4; + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)` +... +LL | internal_rem_euclid!(); + | ---------------------- in this macro invocation + | + = note: this error originates in the macro `internal_rem_euclid` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: manual `rem_euclid` implementation + --> $DIR/manual_rem_euclid.rs:49:5 | LL | ((num % 4) + 4) % 4 | ^^^^^^^^^^^^^^^^^^^ help: consider using: `num.rem_euclid(4)` error: manual `rem_euclid` implementation - --> $DIR/manual_rem_euclid.rs:37:5 + --> $DIR/manual_rem_euclid.rs:54:5 | LL | ((num % 4) + 4) % 4 | ^^^^^^^^^^^^^^^^^^^ help: consider using: `num.rem_euclid(4)` -error: aborting due to 7 previous errors +error: aborting due to 8 previous errors diff --git a/tests/ui/min_rust_version_attr.rs b/tests/ui/min_rust_version_attr.rs index 301a8e54ccfe..44e407bd1ab2 100644 --- a/tests/ui/min_rust_version_attr.rs +++ b/tests/ui/min_rust_version_attr.rs @@ -217,3 +217,12 @@ mod just_above_msrv { } } } + +mod const_rem_euclid { + #![feature(custom_inner_attributes)] + #![clippy::msrv = "1.50.0"] + + pub const fn const_rem_euclid_4(num: i32) -> i32 { + ((num % 4) + 4) % 4 + } +}