Skip to content

Commit

Permalink
Don't warn about modulo arithmetic when comparing to zero
Browse files Browse the repository at this point in the history
Add lint configuration for `modulo_arithmetic`

Collect meta-data
  • Loading branch information
mdm committed Jan 25, 2024
1 parent 900a5aa commit 7af81b7
Show file tree
Hide file tree
Showing 11 changed files with 121 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5819,4 +5819,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 -->
10 changes: 10 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)


4 changes: 4 additions & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,10 @@ define_Conf! {
/// 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.
Expand Down
8 changes: 7 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,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: _,
Expand Down Expand Up @@ -968,7 +969,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));
Expand Down
15 changes: 12 additions & 3 deletions clippy_lints/src/operators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,7 @@ declare_clippy_lint! {
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,
Expand Down Expand Up @@ -801,10 +802,11 @@ impl_lint_pass!(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,
}
}
}
Expand Down Expand Up @@ -835,12 +837,19 @@ 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);
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);
Expand Down
27 changes: 26 additions & 1 deletion clippy_lints/src/operators/modulo_arithmetic.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/modulo_arithmetic/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allow-comparison-to-zero = false
10 changes: 10 additions & 0 deletions tests/ui-toml/modulo_arithmetic/modulo_arithmetic.rs
Original file line number Diff line number Diff line change
@@ -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;
}
40 changes: 40 additions & 0 deletions tests/ui-toml/modulo_arithmetic/modulo_arithmetic.stderr
Original file line number Diff line number Diff line change
@@ -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

2 changes: 2 additions & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/modulo_arithmetic_integral.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit 7af81b7

Please sign in to comment.