Skip to content

Commit 8e493c6

Browse files
committed
Auto merge of #4867 - mgr-inz-rafal:modulo_arithmetic, r=flip1995
Modulo arithmetic changelog: Added modulo_arithmetic lint
2 parents 0fec590 + a208906 commit 8e493c6

11 files changed

+728
-2
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1186,6 +1186,7 @@ Released 2018-09-13
11861186
[`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
11871187
[`module_inception`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_inception
11881188
[`module_name_repetitions`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions
1189+
[`modulo_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#modulo_arithmetic
11891190
[`modulo_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#modulo_one
11901191
[`multiple_crate_versions`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions
11911192
[`multiple_inherent_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_inherent_impl

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
88

9-
[There are 341 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
9+
[There are 342 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
1010

1111
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1212

clippy_lints/src/lib.rs

+4
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ pub mod misc_early;
239239
pub mod missing_const_for_fn;
240240
pub mod missing_doc;
241241
pub mod missing_inline;
242+
pub mod modulo_arithmetic;
242243
pub mod mul_add;
243244
pub mod multiple_crate_versions;
244245
pub mod mut_key;
@@ -667,6 +668,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
667668
&missing_const_for_fn::MISSING_CONST_FOR_FN,
668669
&missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS,
669670
&missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS,
671+
&modulo_arithmetic::MODULO_ARITHMETIC,
670672
&mul_add::MANUAL_MUL_ADD,
671673
&multiple_crate_versions::MULTIPLE_CRATE_VERSIONS,
672674
&mut_key::MUTABLE_KEY_TYPE,
@@ -943,6 +945,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
943945
store.register_late_pass(|| box comparison_chain::ComparisonChain);
944946
store.register_late_pass(|| box mul_add::MulAddCheck);
945947
store.register_late_pass(|| box mut_key::MutableKeyType);
948+
store.register_late_pass(|| box modulo_arithmetic::ModuloArithmetic);
946949
store.register_early_pass(|| box reference::DerefAddrOf);
947950
store.register_early_pass(|| box reference::RefInDeref);
948951
store.register_early_pass(|| box double_parens::DoubleParens);
@@ -1003,6 +1006,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
10031006
LintId::of(&misc::FLOAT_CMP_CONST),
10041007
LintId::of(&missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS),
10051008
LintId::of(&missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS),
1009+
LintId::of(&modulo_arithmetic::MODULO_ARITHMETIC),
10061010
LintId::of(&panic_unimplemented::PANIC),
10071011
LintId::of(&panic_unimplemented::TODO),
10081012
LintId::of(&panic_unimplemented::UNIMPLEMENTED),

clippy_lints/src/modulo_arithmetic.rs

+149
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
use crate::consts::{constant, Constant};
2+
use crate::utils::{sext, span_lint_and_then};
3+
use if_chain::if_chain;
4+
use rustc::declare_lint_pass;
5+
use rustc::hir::*;
6+
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
7+
use rustc::ty::{self};
8+
use rustc_session::declare_tool_lint;
9+
use std::fmt::Display;
10+
11+
declare_clippy_lint! {
12+
/// **What it does:** Checks for modulo arithemtic.
13+
///
14+
/// **Why is this bad?** The results of modulo (%) operation might differ
15+
/// depending on the language, when negative numbers are involved.
16+
/// If you interop with different languages it might be beneficial
17+
/// to double check all places that use modulo arithmetic.
18+
///
19+
/// For example, in Rust `17 % -3 = 2`, but in Python `17 % -3 = -1`.
20+
///
21+
/// **Known problems:** None.
22+
///
23+
/// **Example:**
24+
/// ```rust
25+
/// let x = -17 % 3;
26+
/// ```
27+
pub MODULO_ARITHMETIC,
28+
restriction,
29+
"any modulo arithmetic statement"
30+
}
31+
32+
declare_lint_pass!(ModuloArithmetic => [MODULO_ARITHMETIC]);
33+
34+
struct OperandInfo {
35+
string_representation: Option<String>,
36+
is_negative: bool,
37+
is_integral: bool,
38+
}
39+
40+
fn analyze_operand(operand: &Expr<'_>, cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> Option<OperandInfo> {
41+
match constant(cx, cx.tables, operand) {
42+
Some((Constant::Int(v), _)) => match cx.tables.expr_ty(expr).kind {
43+
ty::Int(ity) => {
44+
let value = sext(cx.tcx, v, ity);
45+
return Some(OperandInfo {
46+
string_representation: Some(value.to_string()),
47+
is_negative: value < 0,
48+
is_integral: true,
49+
});
50+
},
51+
ty::Uint(_) => {
52+
return Some(OperandInfo {
53+
string_representation: None,
54+
is_negative: false,
55+
is_integral: true,
56+
});
57+
},
58+
_ => {},
59+
},
60+
Some((Constant::F32(f), _)) => {
61+
return Some(floating_point_operand_info(&f));
62+
},
63+
Some((Constant::F64(f), _)) => {
64+
return Some(floating_point_operand_info(&f));
65+
},
66+
_ => {},
67+
}
68+
None
69+
}
70+
71+
fn floating_point_operand_info<T: Display + PartialOrd + From<f32>>(f: &T) -> OperandInfo {
72+
OperandInfo {
73+
string_representation: Some(format!("{:.3}", *f)),
74+
is_negative: *f < 0.0.into(),
75+
is_integral: false,
76+
}
77+
}
78+
79+
fn might_have_negative_value(t: &ty::TyS<'_>) -> bool {
80+
t.is_signed() || t.is_floating_point()
81+
}
82+
83+
fn check_const_operands<'a, 'tcx>(
84+
cx: &LateContext<'a, 'tcx>,
85+
expr: &'tcx Expr<'_>,
86+
lhs_operand: &OperandInfo,
87+
rhs_operand: &OperandInfo,
88+
) {
89+
if lhs_operand.is_negative ^ rhs_operand.is_negative {
90+
span_lint_and_then(
91+
cx,
92+
MODULO_ARITHMETIC,
93+
expr.span,
94+
&format!(
95+
"you are using modulo operator on constants with different signs: `{} % {}`",
96+
lhs_operand.string_representation.as_ref().unwrap(),
97+
rhs_operand.string_representation.as_ref().unwrap()
98+
),
99+
|db| {
100+
db.note("double check for expected result especially when interoperating with different languages");
101+
if lhs_operand.is_integral {
102+
db.note("or consider using `rem_euclid` or similar function");
103+
}
104+
},
105+
);
106+
}
107+
}
108+
109+
fn check_non_const_operands<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>, operand: &Expr<'_>) {
110+
let operand_type = cx.tables.expr_ty(operand);
111+
if might_have_negative_value(operand_type) {
112+
span_lint_and_then(
113+
cx,
114+
MODULO_ARITHMETIC,
115+
expr.span,
116+
"you are using modulo operator on types that might have different signs",
117+
|db| {
118+
db.note("double check for expected result especially when interoperating with different languages");
119+
if operand_type.is_integral() {
120+
db.note("or consider using `rem_euclid` or similar function");
121+
}
122+
},
123+
);
124+
}
125+
}
126+
127+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ModuloArithmetic {
128+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
129+
match &expr.kind {
130+
ExprKind::Binary(op, lhs, rhs) | ExprKind::AssignOp(op, lhs, rhs) => {
131+
if let BinOpKind::Rem = op.node {
132+
let lhs_operand = analyze_operand(lhs, cx, expr);
133+
let rhs_operand = analyze_operand(rhs, cx, expr);
134+
if_chain! {
135+
if let Some(lhs_operand) = lhs_operand;
136+
if let Some(rhs_operand) = rhs_operand;
137+
then {
138+
check_const_operands(cx, expr, &lhs_operand, &rhs_operand);
139+
}
140+
else {
141+
check_non_const_operands(cx, expr, lhs);
142+
}
143+
}
144+
};
145+
},
146+
_ => {},
147+
}
148+
}
149+
}

src/lintlist/mod.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub use lint::Lint;
66
pub use lint::LINT_LEVELS;
77

88
// begin lint list, do not remove this comment, it’s used in `update_lints`
9-
pub const ALL_LINTS: [Lint; 341] = [
9+
pub const ALL_LINTS: [Lint; 342] = [
1010
Lint {
1111
name: "absurd_extreme_comparisons",
1212
group: "correctness",
@@ -1183,6 +1183,13 @@ pub const ALL_LINTS: [Lint; 341] = [
11831183
deprecation: None,
11841184
module: "enum_variants",
11851185
},
1186+
Lint {
1187+
name: "modulo_arithmetic",
1188+
group: "restriction",
1189+
desc: "any modulo arithmetic statement",
1190+
deprecation: None,
1191+
module: "modulo_arithmetic",
1192+
},
11861193
Lint {
11871194
name: "modulo_one",
11881195
group: "correctness",

tests/ui/modulo_arithmetic_float.rs

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
#![warn(clippy::modulo_arithmetic)]
2+
#![allow(
3+
unused,
4+
clippy::shadow_reuse,
5+
clippy::shadow_unrelated,
6+
clippy::no_effect,
7+
clippy::unnecessary_operation,
8+
clippy::modulo_one
9+
)]
10+
11+
fn main() {
12+
// Lint when both sides are const and of the opposite sign
13+
-1.6 % 2.1;
14+
1.6 % -2.1;
15+
(1.1 - 2.3) % (1.1 + 2.3);
16+
(1.1 + 2.3) % (1.1 - 2.3);
17+
18+
// Lint on floating point numbers
19+
let a_f32: f32 = -1.6;
20+
let mut b_f32: f32 = 2.1;
21+
a_f32 % b_f32;
22+
b_f32 % a_f32;
23+
b_f32 %= a_f32;
24+
25+
let a_f64: f64 = -1.6;
26+
let mut b_f64: f64 = 2.1;
27+
a_f64 % b_f64;
28+
b_f64 % a_f64;
29+
b_f64 %= a_f64;
30+
31+
// No lint when both sides are const and of the same sign
32+
1.6 % 2.1;
33+
-1.6 % -2.1;
34+
(1.1 + 2.3) % (-1.1 + 2.3);
35+
(-1.1 - 2.3) % (1.1 - 2.3);
36+
}
+83
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
error: you are using modulo operator on constants with different signs: `-1.600 % 2.100`
2+
--> $DIR/modulo_arithmetic_float.rs:13:5
3+
|
4+
LL | -1.6 % 2.1;
5+
| ^^^^^^^^^^
6+
|
7+
= note: `-D clippy::modulo-arithmetic` implied by `-D warnings`
8+
= note: double check for expected result especially when interoperating with different languages
9+
10+
error: you are using modulo operator on constants with different signs: `1.600 % -2.100`
11+
--> $DIR/modulo_arithmetic_float.rs:14:5
12+
|
13+
LL | 1.6 % -2.1;
14+
| ^^^^^^^^^^
15+
|
16+
= note: double check for expected result especially when interoperating with different languages
17+
18+
error: you are using modulo operator on constants with different signs: `-1.200 % 3.400`
19+
--> $DIR/modulo_arithmetic_float.rs:15:5
20+
|
21+
LL | (1.1 - 2.3) % (1.1 + 2.3);
22+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
23+
|
24+
= note: double check for expected result especially when interoperating with different languages
25+
26+
error: you are using modulo operator on constants with different signs: `3.400 % -1.200`
27+
--> $DIR/modulo_arithmetic_float.rs:16:5
28+
|
29+
LL | (1.1 + 2.3) % (1.1 - 2.3);
30+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
31+
|
32+
= note: double check for expected result especially when interoperating with different languages
33+
34+
error: you are using modulo operator on types that might have different signs
35+
--> $DIR/modulo_arithmetic_float.rs:21:5
36+
|
37+
LL | a_f32 % b_f32;
38+
| ^^^^^^^^^^^^^
39+
|
40+
= note: double check for expected result especially when interoperating with different languages
41+
42+
error: you are using modulo operator on types that might have different signs
43+
--> $DIR/modulo_arithmetic_float.rs:22:5
44+
|
45+
LL | b_f32 % a_f32;
46+
| ^^^^^^^^^^^^^
47+
|
48+
= note: double check for expected result especially when interoperating with different languages
49+
50+
error: you are using modulo operator on types that might have different signs
51+
--> $DIR/modulo_arithmetic_float.rs:23:5
52+
|
53+
LL | b_f32 %= a_f32;
54+
| ^^^^^^^^^^^^^^
55+
|
56+
= note: double check for expected result especially when interoperating with different languages
57+
58+
error: you are using modulo operator on types that might have different signs
59+
--> $DIR/modulo_arithmetic_float.rs:27:5
60+
|
61+
LL | a_f64 % b_f64;
62+
| ^^^^^^^^^^^^^
63+
|
64+
= note: double check for expected result especially when interoperating with different languages
65+
66+
error: you are using modulo operator on types that might have different signs
67+
--> $DIR/modulo_arithmetic_float.rs:28:5
68+
|
69+
LL | b_f64 % a_f64;
70+
| ^^^^^^^^^^^^^
71+
|
72+
= note: double check for expected result especially when interoperating with different languages
73+
74+
error: you are using modulo operator on types that might have different signs
75+
--> $DIR/modulo_arithmetic_float.rs:29:5
76+
|
77+
LL | b_f64 %= a_f64;
78+
| ^^^^^^^^^^^^^^
79+
|
80+
= note: double check for expected result especially when interoperating with different languages
81+
82+
error: aborting due to 10 previous errors
83+

0 commit comments

Comments
 (0)