Move ModuloArithmetic into Operators lint pass

Author: Jarcho

clippy_lints/src/lib.register_lints.rs

@@ -379,7 +379,6 @@ store.register_lints(&[
     mixed_read_write_in_expression::MIXED_READ_WRITE_IN_EXPRESSION,
     module_style::MOD_MODULE_FILES,
     module_style::SELF_NAMED_MODULE_FILES,
-    modulo_arithmetic::MODULO_ARITHMETIC,
     mut_key::MUTABLE_KEY_TYPE,
     mut_mut::MUT_MUT,
     mut_mutex_lock::MUT_MUTEX_LOCK,
@@ -434,6 +433,7 @@ store.register_lints(&[
     operators::INTEGER_ARITHMETIC,
     operators::INTEGER_DIVISION,
     operators::MISREFACTORED_ASSIGN_OP,
+    operators::MODULO_ARITHMETIC,
     operators::MODULO_ONE,
     operators::OP_REF,
     operators::VERBOSE_BIT_MASK,

clippy_lints/src/lib.register_restriction.rs

@@ -47,11 +47,11 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
     LintId::of(mixed_read_write_in_expression::MIXED_READ_WRITE_IN_EXPRESSION),
     LintId::of(module_style::MOD_MODULE_FILES),
     LintId::of(module_style::SELF_NAMED_MODULE_FILES),
-    LintId::of(modulo_arithmetic::MODULO_ARITHMETIC),
     LintId::of(operators::FLOAT_ARITHMETIC),
     LintId::of(operators::FLOAT_CMP_CONST),
     LintId::of(operators::INTEGER_ARITHMETIC),
     LintId::of(operators::INTEGER_DIVISION),
+    LintId::of(operators::MODULO_ARITHMETIC),
     LintId::of(panic_in_result_fn::PANIC_IN_RESULT_FN),
     LintId::of(panic_unimplemented::PANIC),
     LintId::of(panic_unimplemented::TODO),

clippy_lints/src/lib.rs

@@ -295,7 +295,6 @@ mod missing_enforced_import_rename;
 mod missing_inline;
 mod mixed_read_write_in_expression;
 mod module_style;
-mod modulo_arithmetic;
 mod mut_key;
 mod mut_mut;
 mod mut_mutex_lock;
@@ -738,7 +737,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(move || Box::new(trait_bounds::TraitBounds::new(max_trait_bounds)));
     store.register_late_pass(|| Box::new(comparison_chain::ComparisonChain));
     store.register_late_pass(|| Box::new(mut_key::MutableKeyType));
-    store.register_late_pass(|| Box::new(modulo_arithmetic::ModuloArithmetic));
     store.register_early_pass(|| Box::new(reference::DerefAddrOf));
     store.register_early_pass(|| Box::new(double_parens::DoubleParens));
     store.register_late_pass(|| Box::new(format_impl::FormatImpl::new()));

clippy_lints/src/operators/mod.rs

@@ -16,6 +16,7 @@ mod float_equality_without_abs;
 mod identity_op;
 mod integer_division;
 mod misrefactored_assign_op;
+mod modulo_arithmetic;
 mod modulo_one;
 mod numeric_arithmetic;
 mod op_ref;
@@ -618,6 +619,28 @@ declare_clippy_lint! {
     "taking a number modulo +/-1, which can either panic/overflow or always returns 0"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for modulo arithmetic.
+    ///
+    /// ### Why is this bad?
+    /// The results of modulo (%) operation might differ
+    /// depending on the language, when negative numbers are involved.
+    /// If you interop with different languages it might be beneficial
+    /// to double check all places that use modulo arithmetic.
+    ///
+    /// For example, in Rust `17 % -3 = 2`, but in Python `17 % -3 = -1`.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let x = -17 % 3;
+    /// ```
+    #[clippy::version = "1.42.0"]
+    pub MODULO_ARITHMETIC,
+    restriction,
+    "any modulo arithmetic statement"
+}
+
 pub struct Operators {
     arithmetic_context: numeric_arithmetic::Context,
     verbose_bit_mask_threshold: u64,
@@ -644,6 +667,7 @@ impl_lint_pass!(Operators => [
     FLOAT_CMP,
     FLOAT_CMP_CONST,
     MODULO_ONE,
+    MODULO_ARITHMETIC,
 ]);
 impl Operators {
     pub fn new(verbose_bit_mask_threshold: u64) -> Self {
@@ -678,10 +702,12 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
                 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);
             },
             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);
             },
             ExprKind::Assign(lhs, rhs, _) => {
                 assign_op_pattern::check(cx, e, lhs, rhs);

clippy_lints/src/modulo_arithmetic.rs renamed to clippy_lints/src/operators/modulo_arithmetic.rs

@@ -2,35 +2,35 @@ use clippy_utils::consts::{constant, Constant};
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::sext;
 use if_chain::if_chain;
-use rustc_hir::{BinOpKind, Expr, ExprKind};
-use rustc_lint::{LateContext, LateLintPass};
+use rustc_hir::{BinOpKind, Expr};
+use rustc_lint::LateContext;
 use rustc_middle::ty::{self, Ty};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
 use std::fmt::Display;
 
-declare_clippy_lint! {
-    /// ### What it does
-    /// Checks for modulo arithmetic.
-    ///
-    /// ### Why is this bad?
-    /// The results of modulo (%) operation might differ
-    /// depending on the language, when negative numbers are involved.
-    /// If you interop with different languages it might be beneficial
-    /// to double check all places that use modulo arithmetic.
-    ///
-    /// For example, in Rust `17 % -3 = 2`, but in Python `17 % -3 = -1`.
-    ///
-    /// ### Example
-    /// ```rust
-    /// let x = -17 % 3;
-    /// ```
-    #[clippy::version = "1.42.0"]
-    pub MODULO_ARITHMETIC,
-    restriction,
-    "any modulo arithmetic statement"
-}
+use super::MODULO_ARITHMETIC;
 
-declare_lint_pass!(ModuloArithmetic => [MODULO_ARITHMETIC]);
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    e: &'tcx Expr<'_>,
+    op: BinOpKind,
+    lhs: &'tcx Expr<'_>,
+    rhs: &'tcx Expr<'_>,
+) {
+    if op == BinOpKind::Rem {
+        let lhs_operand = analyze_operand(lhs, cx, e);
+        let rhs_operand = analyze_operand(rhs, cx, e);
+        if_chain! {
+            if let Some(lhs_operand) = lhs_operand;
+            if let Some(rhs_operand) = rhs_operand;
+            then {
+                check_const_operands(cx, e, &lhs_operand, &rhs_operand);
+            }
+            else {
+                check_non_const_operands(cx, e, lhs);
+            }
+        }
+    };
+}
 
 struct OperandInfo {
     string_representation: Option<String>,
@@ -124,27 +124,3 @@ fn check_non_const_operands<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>,
         );
     }
 }
-
-impl<'tcx> LateLintPass<'tcx> for ModuloArithmetic {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        match &expr.kind {
-            ExprKind::Binary(op, lhs, rhs) | ExprKind::AssignOp(op, lhs, rhs) => {
-                if op.node == BinOpKind::Rem {
-                    let lhs_operand = analyze_operand(lhs, cx, expr);
-                    let rhs_operand = analyze_operand(rhs, cx, expr);
-                    if_chain! {
-                        if let Some(lhs_operand) = lhs_operand;
-                        if let Some(rhs_operand) = rhs_operand;
-                        then {
-                            check_const_operands(cx, expr, &lhs_operand, &rhs_operand);
-                        }
-                        else {
-                            check_non_const_operands(cx, expr, lhs);
-                        }
-                    }
-                };
-            },
-            _ => {},
-        }
-    }
-}