Skip to content

Commit e456c28

Browse files
mdmmdm
committedJan 25, 2024
Don't warn about modulo arithmetic when comparing to zero
Add lint configuration for `modulo_arithmetic` Collect meta-data
1 parent 900a5aa commit e456c28

File tree

11 files changed

+122
-5
lines changed

11 files changed

+122
-5
lines changed
 

Diff for: ‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5819,4 +5819,5 @@ Released 2018-09-13
58195819
[`enforce-iter-loop-reborrow`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforce-iter-loop-reborrow
58205820
[`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items
58215821
[`pub-underscore-fields-behavior`]: https://doc.rust-lang.org/clippy/lint_configuration.html#pub-underscore-fields-behavior
5822+
[`allow-comparison-to-zero`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-comparison-to-zero
58225823
<!-- end autogenerated links to configuration documentation -->

Diff for: ‎book/src/lint_configuration.md

+10
Original file line numberDiff line numberDiff line change
@@ -828,3 +828,13 @@ exported visibility, or whether they are marked as "pub".
828828
* [`pub_underscore_fields`](https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields)
829829

830830

831+
## `allow-comparison-to-zero`
832+
Don't lint when comparing the result of a modulo operation to zero.
833+
834+
**Default Value:** `true`
835+
836+
---
837+
**Affected lints:**
838+
* [`modulo_arithmetic`](https://rust-lang.github.io/rust-clippy/master/index.html#modulo_arithmetic)
839+
840+

Diff for: ‎clippy_config/src/conf.rs

+4
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,10 @@ define_Conf! {
567567
/// Lint "public" fields in a struct that are prefixed with an underscore based on their
568568
/// exported visibility, or whether they are marked as "pub".
569569
(pub_underscore_fields_behavior: PubUnderscoreFieldsBehaviour = PubUnderscoreFieldsBehaviour::PubliclyExported),
570+
/// Lint: MODULO_ARITHMETIC.
571+
///
572+
/// Don't lint when comparing the result of a modulo operation to zero.
573+
(allow_comparison_to_zero: bool = true),
570574
}
571575

572576
/// Search for the configuration file.

Diff for: ‎clippy_lints/src/lib.rs

+7-1
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
575575
check_private_items,
576576
pub_underscore_fields_behavior,
577577
ref allowed_duplicate_crates,
578+
allow_comparison_to_zero,
578579

579580
blacklisted_names: _,
580581
cyclomatic_complexity_threshold: _,
@@ -968,7 +969,12 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
968969
store.register_late_pass(|_| Box::new(default_instead_of_iter_empty::DefaultIterEmpty));
969970
store.register_late_pass(move |_| Box::new(manual_rem_euclid::ManualRemEuclid::new(msrv())));
970971
store.register_late_pass(move |_| Box::new(manual_retain::ManualRetain::new(msrv())));
971-
store.register_late_pass(move |_| Box::new(operators::Operators::new(verbose_bit_mask_threshold)));
972+
store.register_late_pass(move |_| {
973+
Box::new(operators::Operators::new(
974+
verbose_bit_mask_threshold,
975+
allow_comparison_to_zero,
976+
))
977+
});
972978
store.register_late_pass(|_| Box::<std_instead_of_core::StdReexports>::default());
973979
store.register_late_pass(move |_| Box::new(instant_subtraction::InstantSubtraction::new(msrv())));
974980
store.register_late_pass(|_| Box::new(partialeq_to_none::PartialeqToNone));

Diff for: ‎clippy_lints/src/operators/mod.rs

+12-3
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,7 @@ declare_clippy_lint! {
771771
pub struct Operators {
772772
arithmetic_context: numeric_arithmetic::Context,
773773
verbose_bit_mask_threshold: u64,
774+
modulo_arithmetic_allow_comparison_to_zero: bool,
774775
}
775776
impl_lint_pass!(Operators => [
776777
ABSURD_EXTREME_COMPARISONS,
@@ -801,10 +802,11 @@ impl_lint_pass!(Operators => [
801802
SELF_ASSIGNMENT,
802803
]);
803804
impl Operators {
804-
pub fn new(verbose_bit_mask_threshold: u64) -> Self {
805+
pub fn new(verbose_bit_mask_threshold: u64, modulo_arithmetic_allow_comparison_to_zero: bool) -> Self {
805806
Self {
806807
arithmetic_context: numeric_arithmetic::Context::default(),
807808
verbose_bit_mask_threshold,
809+
modulo_arithmetic_allow_comparison_to_zero,
808810
}
809811
}
810812
}
@@ -835,12 +837,19 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
835837
cmp_owned::check(cx, op.node, lhs, rhs);
836838
float_cmp::check(cx, e, op.node, lhs, rhs);
837839
modulo_one::check(cx, e, op.node, rhs);
838-
modulo_arithmetic::check(cx, e, op.node, lhs, rhs);
840+
modulo_arithmetic::check(
841+
cx,
842+
e,
843+
op.node,
844+
lhs,
845+
rhs,
846+
self.modulo_arithmetic_allow_comparison_to_zero,
847+
);
839848
},
840849
ExprKind::AssignOp(op, lhs, rhs) => {
841850
self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);
842851
misrefactored_assign_op::check(cx, e, op.node, lhs, rhs);
843-
modulo_arithmetic::check(cx, e, op.node, lhs, rhs);
852+
modulo_arithmetic::check(cx, e, op.node, lhs, rhs, false);
844853
},
845854
ExprKind::Assign(lhs, rhs, _) => {
846855
assign_op_pattern::check(cx, e, lhs, rhs);

Diff for: ‎clippy_lints/src/operators/modulo_arithmetic.rs

+26-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::consts::{constant, Constant};
22
use clippy_utils::diagnostics::span_lint_and_then;
33
use clippy_utils::sext;
4-
use rustc_hir::{BinOpKind, Expr};
4+
use rustc_hir::{BinOpKind, Expr, ExprKind, Node};
55
use rustc_lint::LateContext;
66
use rustc_middle::ty::{self, Ty};
77
use std::fmt::Display;
@@ -14,8 +14,13 @@ pub(super) fn check<'tcx>(
1414
op: BinOpKind,
1515
lhs: &'tcx Expr<'_>,
1616
rhs: &'tcx Expr<'_>,
17+
allow_comparison_to_zero: bool,
1718
) {
1819
if op == BinOpKind::Rem {
20+
if allow_comparison_to_zero && used_in_comparison_with_zero(cx, e) {
21+
return;
22+
}
23+
1924
let lhs_operand = analyze_operand(lhs, cx, e);
2025
let rhs_operand = analyze_operand(rhs, cx, e);
2126
if let Some(lhs_operand) = lhs_operand
@@ -28,6 +33,26 @@ pub(super) fn check<'tcx>(
2833
};
2934
}
3035

36+
fn used_in_comparison_with_zero(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
37+
let Some(Node::Expr(parent_expr)) = cx.tcx.hir().find_parent(expr.hir_id) else {
38+
return false;
39+
};
40+
let ExprKind::Binary(op, lhs, rhs) = parent_expr.kind else {
41+
return false;
42+
};
43+
44+
if op.node == BinOpKind::Eq || op.node == BinOpKind::Ne {
45+
if let Some(Constant::Int(0)) = constant(cx, cx.typeck_results(), rhs) {
46+
return true;
47+
}
48+
if let Some(Constant::Int(0)) = constant(cx, cx.typeck_results(), lhs) {
49+
return true;
50+
}
51+
}
52+
53+
false
54+
}
55+
3156
struct OperandInfo {
3257
string_representation: Option<String>,
3358
is_negative: bool,

Diff for: ‎tests/ui-toml/modulo_arithmetic/clippy.toml

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
allow-comparison-to-zero = false

Diff for: ‎tests/ui-toml/modulo_arithmetic/modulo_arithmetic.rs

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#![warn(clippy::modulo_arithmetic)]
2+
3+
fn main() {
4+
let a = -1;
5+
let b = 2;
6+
let c = a % b == 0;
7+
let c = a % b != 0;
8+
let c = 0 == a % b;
9+
let c = 0 != a % b;
10+
}

Diff for: ‎tests/ui-toml/modulo_arithmetic/modulo_arithmetic.stderr

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
error: you are using modulo operator on types that might have different signs
2+
--> $DIR/modulo_arithmetic.rs:6:13
3+
|
4+
LL | let c = a % b == 0;
5+
| ^^^^^
6+
|
7+
= note: double check for expected result especially when interoperating with different languages
8+
= note: or consider using `rem_euclid` or similar function
9+
= note: `-D clippy::modulo-arithmetic` implied by `-D warnings`
10+
= help: to override `-D warnings` add `#[allow(clippy::modulo_arithmetic)]`
11+
12+
error: you are using modulo operator on types that might have different signs
13+
--> $DIR/modulo_arithmetic.rs:7:13
14+
|
15+
LL | let c = a % b != 0;
16+
| ^^^^^
17+
|
18+
= note: double check for expected result especially when interoperating with different languages
19+
= note: or consider using `rem_euclid` or similar function
20+
21+
error: you are using modulo operator on types that might have different signs
22+
--> $DIR/modulo_arithmetic.rs:8:18
23+
|
24+
LL | let c = 0 == a % b;
25+
| ^^^^^
26+
|
27+
= note: double check for expected result especially when interoperating with different languages
28+
= note: or consider using `rem_euclid` or similar function
29+
30+
error: you are using modulo operator on types that might have different signs
31+
--> $DIR/modulo_arithmetic.rs:9:18
32+
|
33+
LL | let c = 0 != a % b;
34+
| ^^^^^
35+
|
36+
= note: double check for expected result especially when interoperating with different languages
37+
= note: or consider using `rem_euclid` or similar function
38+
39+
error: aborting due to 4 previous errors
40+

Diff for: ‎tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

+3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
33
absolute-paths-max-segments
44
accept-comment-above-attributes
55
accept-comment-above-statement
6+
allow-comparison-to-zero
67
allow-dbg-in-tests
78
allow-expect-in-tests
89
allow-mixed-uninlined-format-args
@@ -80,6 +81,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
8081
absolute-paths-max-segments
8182
accept-comment-above-attributes
8283
accept-comment-above-statement
84+
allow-comparison-to-zero
8385
allow-dbg-in-tests
8486
allow-expect-in-tests
8587
allow-mixed-uninlined-format-args
@@ -157,6 +159,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
157159
absolute-paths-max-segments
158160
accept-comment-above-attributes
159161
accept-comment-above-statement
162+
allow-comparison-to-zero
160163
allow-dbg-in-tests
161164
allow-expect-in-tests
162165
allow-mixed-uninlined-format-args

Diff for: ‎tests/ui/modulo_arithmetic_integral.rs

+8
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,12 @@ fn main() {
114114
a_usize % b_usize;
115115
let mut a_usize: usize = 1;
116116
a_usize %= 2;
117+
118+
// No lint when comparing to zero
119+
let a = -1;
120+
let mut b = 2;
121+
let c = a % b == 0;
122+
let c = 0 == a % b;
123+
let c = a % b != 0;
124+
let c = 0 != a % b;
117125
}

0 commit comments

Comments
 (0)