Auto merge of #12178 - mdm:modulo-arithmetic-comparison-to-zero, r=llogiq

Don't warn about modulo arithmetic when comparing to zero

closes #12006

By default, don't warn about modulo arithmetic when comparing to zero. This behavior is configurable via `clippy.toml`.

See discussion [on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/.E2.9C.94.20Is.20issue.20.2312006.20worth.20implementing.3F)

changelog: [`modulo_arithmetic`]: By default don't lint when comparing the result of a modulo operation to zero.

CHANGELOG.md

@@ -5820,4 +5820,5 @@ Released 2018-09-13
 [`enforce-iter-loop-reborrow`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforce-iter-loop-reborrow
 [`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items
 [`pub-underscore-fields-behavior`]: https://doc.rust-lang.org/clippy/lint_configuration.html#pub-underscore-fields-behavior
+[`allow-comparison-to-zero`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-comparison-to-zero
 <!-- end autogenerated links to configuration documentation -->

book/src/lint_configuration.md

@@ -828,3 +828,13 @@ exported visibility, or whether they are marked as "pub".
 * [`pub_underscore_fields`](https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields)
 
 
+## `allow-comparison-to-zero`
+Don't lint when comparing the result of a modulo operation to zero.
+
+**Default Value:** `true`
+
+---
+**Affected lints:**
+* [`modulo_arithmetic`](https://rust-lang.github.io/rust-clippy/master/index.html#modulo_arithmetic)
+
+

clippy_config/src/conf.rs

@@ -567,6 +567,10 @@ pub fn get_configuration_metadata() -> Vec<ClippyConfiguration> {
     /// Lint "public" fields in a struct that are prefixed with an underscore based on their
     /// exported visibility, or whether they are marked as "pub".
     (pub_underscore_fields_behavior: PubUnderscoreFieldsBehaviour = PubUnderscoreFieldsBehaviour::PubliclyExported),
+    /// Lint: MODULO_ARITHMETIC.
+    ///
+    /// Don't lint when comparing the result of a modulo operation to zero.
+    (allow_comparison_to_zero: bool = true),
 }
 
 /// Search for the configuration file.

clippy_lints/src/lib.rs

@@ -577,6 +577,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
         check_private_items,
         pub_underscore_fields_behavior,
         ref allowed_duplicate_crates,
+        allow_comparison_to_zero,
 
         blacklisted_names: _,
         cyclomatic_complexity_threshold: _,
@@ -970,7 +971,12 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     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())));
     store.register_late_pass(move |_| Box::new(manual_retain::ManualRetain::new(msrv())));
-    store.register_late_pass(move |_| Box::new(operators::Operators::new(verbose_bit_mask_threshold)));
+    store.register_late_pass(move |_| {
+        Box::new(operators::Operators::new(
+            verbose_bit_mask_threshold,
+            allow_comparison_to_zero,
+        ))
+    });
     store.register_late_pass(|_| Box::<std_instead_of_core::StdReexports>::default());
     store.register_late_pass(move |_| Box::new(instant_subtraction::InstantSubtraction::new(msrv())));
     store.register_late_pass(|_| Box::new(partialeq_to_none::PartialeqToNone));

clippy_lints/src/operators/mod.rs

@@ -771,6 +771,7 @@
 pub struct Operators {
     arithmetic_context: numeric_arithmetic::Context,
     verbose_bit_mask_threshold: u64,
+    modulo_arithmetic_allow_comparison_to_zero: bool,
 }
 impl_lint_pass!(Operators => [
     ABSURD_EXTREME_COMPARISONS,
@@ -801,10 +802,11 @@ pub struct Operators {
     SELF_ASSIGNMENT,
 ]);
 impl Operators {
-    pub fn new(verbose_bit_mask_threshold: u64) -> Self {
+    pub fn new(verbose_bit_mask_threshold: u64, modulo_arithmetic_allow_comparison_to_zero: bool) -> Self {
         Self {
             arithmetic_context: numeric_arithmetic::Context::default(),
             verbose_bit_mask_threshold,
+            modulo_arithmetic_allow_comparison_to_zero,
         }
     }
 }
@@ -835,12 +837,19 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
                 cmp_owned::check(cx, op.node, lhs, rhs);
                 float_cmp::check(cx, e, op.node, lhs, rhs);
                 modulo_one::check(cx, e, op.node, rhs);
-                modulo_arithmetic::check(cx, e, op.node, lhs, rhs);
+                modulo_arithmetic::check(
+                    cx,
+                    e,
+                    op.node,
+                    lhs,
+                    rhs,
+                    self.modulo_arithmetic_allow_comparison_to_zero,
+                );
             },
             ExprKind::AssignOp(op, lhs, rhs) => {
                 self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs);
                 misrefactored_assign_op::check(cx, e, op.node, lhs, rhs);
-                modulo_arithmetic::check(cx, e, op.node, lhs, rhs);
+                modulo_arithmetic::check(cx, e, op.node, lhs, rhs, false);
             },
             ExprKind::Assign(lhs, rhs, _) => {
                 assign_op_pattern::check(cx, e, lhs, rhs);

clippy_lints/src/operators/modulo_arithmetic.rs

@@ -1,7 +1,7 @@
 use clippy_utils::consts::{constant, Constant};
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::sext;
-use rustc_hir::{BinOpKind, Expr};
+use rustc_hir::{BinOpKind, Expr, ExprKind, Node};
 use rustc_lint::LateContext;
 use rustc_middle::ty::{self, Ty};
 use std::fmt::Display;
@@ -14,8 +14,13 @@ pub(super) fn check<'tcx>(
     op: BinOpKind,
     lhs: &'tcx Expr<'_>,
     rhs: &'tcx Expr<'_>,
+    allow_comparison_to_zero: bool,
 ) {
     if op == BinOpKind::Rem {
+        if allow_comparison_to_zero && used_in_comparison_with_zero(cx, e) {
+            return;
+        }
+
         let lhs_operand = analyze_operand(lhs, cx, e);
         let rhs_operand = analyze_operand(rhs, cx, e);
         if let Some(lhs_operand) = lhs_operand
@@ -28,6 +33,26 @@ pub(super) fn check<'tcx>(
     };
 }
 
+fn used_in_comparison_with_zero(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    let Some(Node::Expr(parent_expr)) = cx.tcx.hir().find_parent(expr.hir_id) else {
+        return false;
+    };
+    let ExprKind::Binary(op, lhs, rhs) = parent_expr.kind else {
+        return false;
+    };
+
+    if op.node == BinOpKind::Eq || op.node == BinOpKind::Ne {
+        if let Some(Constant::Int(0)) = constant(cx, cx.typeck_results(), rhs) {
+            return true;
+        }
+        if let Some(Constant::Int(0)) = constant(cx, cx.typeck_results(), lhs) {
+            return true;
+        }
+    }
+
+    false
+}
+
 struct OperandInfo {
     string_representation: Option<String>,
     is_negative: bool,

tests/ui-toml/modulo_arithmetic/clippy.toml

@@ -0,0 +1 @@
+allow-comparison-to-zero = false

tests/ui-toml/modulo_arithmetic/modulo_arithmetic.rs

@@ -0,0 +1,10 @@
+#![warn(clippy::modulo_arithmetic)]
+
+fn main() {
+    let a = -1;
+    let b = 2;
+    let c = a % b == 0;
+    let c = a % b != 0;
+    let c = 0 == a % b;
+    let c = 0 != a % b;
+}

tests/ui-toml/modulo_arithmetic/modulo_arithmetic.stderr

@@ -0,0 +1,40 @@
+error: you are using modulo operator on types that might have different signs
+  --> $DIR/modulo_arithmetic.rs:6:13
+   |
+LL |     let c = a % b == 0;
+   |             ^^^^^
+   |
+   = note: double check for expected result especially when interoperating with different languages
+   = note: or consider using `rem_euclid` or similar function
+   = note: `-D clippy::modulo-arithmetic` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::modulo_arithmetic)]`
+
+error: you are using modulo operator on types that might have different signs
+  --> $DIR/modulo_arithmetic.rs:7:13
+   |
+LL |     let c = a % b != 0;
+   |             ^^^^^
+   |
+   = note: double check for expected result especially when interoperating with different languages
+   = note: or consider using `rem_euclid` or similar function
+
+error: you are using modulo operator on types that might have different signs
+  --> $DIR/modulo_arithmetic.rs:8:18
+   |
+LL |     let c = 0 == a % b;
+   |                  ^^^^^
+   |
+   = note: double check for expected result especially when interoperating with different languages
+   = note: or consider using `rem_euclid` or similar function
+
+error: you are using modulo operator on types that might have different signs
+  --> $DIR/modulo_arithmetic.rs:9:18
+   |
+LL |     let c = 0 != a % b;
+   |                  ^^^^^
+   |
+   = note: double check for expected result especially when interoperating with different languages
+   = note: or consider using `rem_euclid` or similar function
+
+error: aborting due to 4 previous errors
+

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

@@ -3,6 +3,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
            absolute-paths-max-segments
            accept-comment-above-attributes
            accept-comment-above-statement
+           allow-comparison-to-zero
            allow-dbg-in-tests
            allow-expect-in-tests
            allow-mixed-uninlined-format-args
@@ -80,6 +81,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
            absolute-paths-max-segments
            accept-comment-above-attributes
            accept-comment-above-statement
+           allow-comparison-to-zero
            allow-dbg-in-tests
            allow-expect-in-tests
            allow-mixed-uninlined-format-args
@@ -157,6 +159,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
            absolute-paths-max-segments
            accept-comment-above-attributes
            accept-comment-above-statement
+           allow-comparison-to-zero
            allow-dbg-in-tests
            allow-expect-in-tests
            allow-mixed-uninlined-format-args

tests/ui/modulo_arithmetic_integral.rs

@@ -114,4 +114,12 @@ fn main() {
     a_usize % b_usize;
     let mut a_usize: usize = 1;
     a_usize %= 2;
+
+    // No lint when comparing to zero
+    let a = -1;
+    let mut b = 2;
+    let c = a % b == 0;
+    let c = 0 == a % b;
+    let c = a % b != 0;
+    let c = 0 != a % b;
 }