Skip to content

Commit f191e91

Browse files
committed
Add new lint (modulo_arithmetic)
1 parent 0fec590 commit f191e91

File tree

5 files changed

+163
-2
lines changed

5 files changed

+163
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 4 additions & 0 deletions
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

Lines changed: 149 additions & 0 deletions
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

Lines changed: 8 additions & 1 deletion
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",

0 commit comments

Comments
 (0)