From 71c2daa60aa71446451d5fa9a2c43e9881e83f03 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Wed, 1 Jun 2022 09:50:33 -0400 Subject: [PATCH] Move `ModuloArithmetic` into `Operators` lint pass --- clippy_lints/src/lib.register_lints.rs | 2 +- clippy_lints/src/lib.register_restriction.rs | 2 +- clippy_lints/src/lib.rs | 2 - clippy_lints/src/operators/mod.rs | 26 +++++++ .../src/{ => operators}/modulo_arithmetic.rs | 74 +++++++------------ 5 files changed, 53 insertions(+), 53 deletions(-) rename clippy_lints/src/{ => operators}/modulo_arithmetic.rs (64%) diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 139b55e965f..fea98c08e4a 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/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, diff --git a/clippy_lints/src/lib.register_restriction.rs b/clippy_lints/src/lib.register_restriction.rs index e66dc67f3dc..970e9db4772 100644 --- a/clippy_lints/src/lib.register_restriction.rs +++ b/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), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4c4d149e177..724674471f2 100644 --- a/clippy_lints/src/lib.rs +++ b/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())); diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index e4aff267fef..7db38f0fcfb 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/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); diff --git a/clippy_lints/src/modulo_arithmetic.rs b/clippy_lints/src/operators/modulo_arithmetic.rs similarity index 64% rename from clippy_lints/src/modulo_arithmetic.rs rename to clippy_lints/src/operators/modulo_arithmetic.rs index 195b2e5c2ee..af4e74947f4 100644 --- a/clippy_lints/src/modulo_arithmetic.rs +++ b/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, @@ -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); - } - } - }; - }, - _ => {}, - } - } -}