diff --git a/CHANGELOG.md b/CHANGELOG.md
index d64dd772ac5..dcc96bc10b8 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 7876f21f6e6..a1565255b0b 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 4f1c3673f85..6370264a12a 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 de22f50cf94..f706ba0620f 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 dcee11cc28c..70cf6be8b7c 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 00000000000..7ef8f78a5f6
--- /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<RustcVersion>,
+}
+
+impl ManualRemEuclid {
+ #[must_use]
+ pub fn new(msrv: Option<RustcVersion>) -> 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 b9ec2c19fdd..8000f9e21a1 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 00000000000..f176fc9ad12
--- /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 00000000000..b243065de87
--- /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 00000000000..7134759dcc9
--- /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 f83c3e0e281..301a8e54ccf 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 de225eb740d..b1c23b539ff 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!(<stripped>.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, ") {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^