Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit e5ebd3e

Browse files
author
Evan Typanski
committed
Implement manual_rem_euclid lint
1 parent 93c6f9e commit e5ebd3e

File tree

12 files changed

+195
-5
lines changed

12 files changed

+195
-5
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3527,6 +3527,7 @@ Released 2018-09-13
35273527
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
35283528
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
35293529
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
3530+
[`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid
35303531
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
35313532
[`manual_split_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once
35323533
[`manual_str_repeat`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat

‎clippy_lints/src/lib.register_all.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
135135
LintId::of(manual_async_fn::MANUAL_ASYNC_FN),
136136
LintId::of(manual_bits::MANUAL_BITS),
137137
LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
138+
LintId::of(manual_rem_euclid::MANUAL_REM_EUCLID),
138139
LintId::of(manual_strip::MANUAL_STRIP),
139140
LintId::of(map_clone::MAP_CLONE),
140141
LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN),

‎clippy_lints/src/lib.register_complexity.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
2424
LintId::of(loops::MANUAL_FLATTEN),
2525
LintId::of(loops::SINGLE_ELEMENT_LOOP),
2626
LintId::of(loops::WHILE_LET_LOOP),
27+
LintId::of(manual_rem_euclid::MANUAL_REM_EUCLID),
2728
LintId::of(manual_strip::MANUAL_STRIP),
2829
LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN),
2930
LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN),

‎clippy_lints/src/lib.register_lints.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ store.register_lints(&[
254254
manual_bits::MANUAL_BITS,
255255
manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
256256
manual_ok_or::MANUAL_OK_OR,
257+
manual_rem_euclid::MANUAL_REM_EUCLID,
257258
manual_strip::MANUAL_STRIP,
258259
map_clone::MAP_CLONE,
259260
map_err_ignore::MAP_ERR_IGNORE,

‎clippy_lints/src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,7 @@ mod manual_async_fn;
282282
mod manual_bits;
283283
mod manual_non_exhaustive;
284284
mod manual_ok_or;
285+
mod manual_rem_euclid;
285286
mod manual_strip;
286287
mod map_clone;
287288
mod map_err_ignore;
@@ -912,6 +913,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
912913
store.register_late_pass(|| Box::new(as_underscore::AsUnderscore));
913914
store.register_late_pass(|| Box::new(read_zero_byte_vec::ReadZeroByteVec));
914915
store.register_late_pass(|| Box::new(default_instead_of_iter_empty::DefaultIterEmpty));
916+
store.register_late_pass(move || Box::new(manual_rem_euclid::ManualRemEuclid::new(msrv)));
915917
// add lints here, do not remove this comment, it's used in `new_lint`
916918
}
917919

‎clippy_lints/src/manual_rem_euclid.rs‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
use clippy_utils::consts::{constant_full_int, FullInt};
2+
use clippy_utils::diagnostics::span_lint_and_sugg;
3+
use clippy_utils::source::snippet_with_applicability;
4+
use clippy_utils::{meets_msrv, msrvs, path_to_local};
5+
use if_chain::if_chain;
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{BinOpKind, Expr, ExprKind};
8+
use rustc_lint::{LateContext, LateLintPass};
9+
use rustc_semver::RustcVersion;
10+
use rustc_session::{declare_tool_lint, impl_lint_pass};
11+
12+
declare_clippy_lint! {
13+
/// ### What it does
14+
/// Checks for an expression like `((x % 4) + 4) % 4` which is a common manual reimplementation
15+
/// of `x.rem_euclid(4)`.
16+
///
17+
/// ### Why is this bad?
18+
/// It's simpler and more readable.
19+
///
20+
/// ### Example
21+
/// ```rust
22+
/// let x = 24;
23+
/// let rem = ((x % 4) + 4) % 4;
24+
/// ```
25+
/// Use instead:
26+
/// ```rust
27+
/// let x = 24;
28+
/// let rem = x.rem_euclid(4);
29+
/// ```
30+
#[clippy::version = "1.63.0"]
31+
pub MANUAL_REM_EUCLID,
32+
complexity,
33+
"manually reimplementing `rem_euclid`"
34+
}
35+
36+
pub struct ManualRemEuclid {
37+
msrv: Option<RustcVersion>,
38+
}
39+
40+
impl ManualRemEuclid {
41+
#[must_use]
42+
pub fn new(msrv: Option<RustcVersion>) -> Self {
43+
Self { msrv }
44+
}
45+
}
46+
47+
impl_lint_pass!(ManualRemEuclid => [MANUAL_REM_EUCLID]);
48+
49+
impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid {
50+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
51+
if !meets_msrv(self.msrv, msrvs::REM_EUCLID) {
52+
return;
53+
}
54+
55+
if_chain! {
56+
if let ExprKind::Binary(op1, ..) = expr.kind;
57+
if op1.node == BinOpKind::Rem;
58+
if let Some((const1, expr1)) = check_for_positive_int_constant(cx, expr);
59+
if let ExprKind::Binary(op2, ..) = expr1.kind;
60+
if op2.node == BinOpKind::Add;
61+
if let Some((const2, expr2)) = check_for_positive_int_constant(cx, expr1);
62+
if let ExprKind::Binary(op3, ..) = expr2.kind;
63+
if op3.node == BinOpKind::Rem;
64+
if let Some((const3, expr3)) = check_for_positive_int_constant(cx, expr2);
65+
if const1 == const2 && const2 == const3;
66+
if path_to_local(expr3).is_some();
67+
then {
68+
let mut app = Applicability::MachineApplicable;
69+
let rem_of = snippet_with_applicability(cx, expr3.span, "_", &mut app);
70+
span_lint_and_sugg(
71+
cx,
72+
MANUAL_REM_EUCLID,
73+
expr.span,
74+
"manual `rem_euclid` implementation",
75+
"consider using",
76+
format!("{rem_of}.rem_euclid({const1})"),
77+
app,
78+
);
79+
}
80+
}
81+
}
82+
83+
extract_msrv_attr!(LateContext);
84+
}
85+
86+
// Takes a binary expression and separates the operands into the int constant and the other
87+
// operand. Ensures the int constant is positive.
88+
fn check_for_positive_int_constant<'a>(cx: &'a LateContext<'_>, expr: &'a Expr<'_>) -> Option<(u128, &'a Expr<'a>)> {
89+
let (int_const, other_op) = if let ExprKind::Binary(_, left, right) = expr.kind {
90+
if let Some(int_const) = constant_full_int(cx, cx.typeck_results(), left) {
91+
(int_const, right)
92+
} else if let Some(int_const) = constant_full_int(cx, cx.typeck_results(), right) {
93+
(int_const, left)
94+
} else {
95+
return None;
96+
}
97+
} else {
98+
return None;
99+
};
100+
101+
if int_const > FullInt::S(0) {
102+
let val = match int_const {
103+
FullInt::S(s) => s.try_into().unwrap(),
104+
FullInt::U(u) => u,
105+
};
106+
Some((val, other_op))
107+
} else {
108+
None
109+
}
110+
}

‎clippy_utils/src/msrvs.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ msrv_aliases! {
2323
1,42,0 { MATCHES_MACRO, SLICE_PATTERNS, PTR_SLICE_RAW_PARTS }
2424
1,41,0 { RE_REBALANCING_COHERENCE, RESULT_MAP_OR_ELSE }
2525
1,40,0 { MEM_TAKE, NON_EXHAUSTIVE, OPTION_AS_DEREF }
26-
1,38,0 { POINTER_CAST }
26+
1,38,0 { POINTER_CAST, REM_EUCLID }
2727
1,37,0 { TYPE_ALIAS_ENUM_VARIANTS }
2828
1,36,0 { ITERATOR_COPIED }
2929
1,35,0 { OPTION_COPIED, RANGE_CONTAINS }

‎tests/ui/manual_rem_euclid.fixed‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::manual_rem_euclid)]
4+
5+
fn main() {
6+
let value: i32 = 5;
7+
8+
let _: i32 = value.rem_euclid(4);
9+
let _: i32 = value.rem_euclid(4);
10+
let _: i32 = value.rem_euclid(4);
11+
let _: i32 = value.rem_euclid(4);
12+
let _: i32 = 1 + value.rem_euclid(4);
13+
14+
let _: i32 = (3 + value % 4) % 4;
15+
let _: i32 = (-4 + value % -4) % -4;
16+
let _: i32 = ((5 % 4) + 4) % 4;
17+
}

‎tests/ui/manual_rem_euclid.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::manual_rem_euclid)]
4+
5+
fn main() {
6+
let value: i32 = 5;
7+
8+
let _: i32 = ((value % 4) + 4) % 4;
9+
let _: i32 = (4 + (value % 4)) % 4;
10+
let _: i32 = (value % 4 + 4) % 4;
11+
let _: i32 = (4 + value % 4) % 4;
12+
let _: i32 = 1 + (4 + value % 4) % 4;
13+
14+
let _: i32 = (3 + value % 4) % 4;
15+
let _: i32 = (-4 + value % -4) % -4;
16+
let _: i32 = ((5 % 4) + 4) % 4;
17+
}

‎tests/ui/manual_rem_euclid.stderr‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
error: manual `rem_euclid` implementation
2+
--> $DIR/manual_rem_euclid.rs:8:18
3+
|
4+
LL | let _: i32 = ((value % 4) + 4) % 4;
5+
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)`
6+
|
7+
= note: `-D clippy::manual-rem-euclid` implied by `-D warnings`
8+
9+
error: manual `rem_euclid` implementation
10+
--> $DIR/manual_rem_euclid.rs:9:18
11+
|
12+
LL | let _: i32 = (4 + (value % 4)) % 4;
13+
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)`
14+
15+
error: manual `rem_euclid` implementation
16+
--> $DIR/manual_rem_euclid.rs:10:18
17+
|
18+
LL | let _: i32 = (value % 4 + 4) % 4;
19+
| ^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)`
20+
21+
error: manual `rem_euclid` implementation
22+
--> $DIR/manual_rem_euclid.rs:11:18
23+
|
24+
LL | let _: i32 = (4 + value % 4) % 4;
25+
| ^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)`
26+
27+
error: manual `rem_euclid` implementation
28+
--> $DIR/manual_rem_euclid.rs:12:22
29+
|
30+
LL | let _: i32 = 1 + (4 + value % 4) % 4;
31+
| ^^^^^^^^^^^^^^^^^^^ help: consider using: `value.rem_euclid(4)`
32+
33+
error: aborting due to 5 previous errors
34+

0 commit comments

Comments
 (0)