Skip to content

Commit

Permalink
Add new lint (modulo_arithmetic)
Browse files Browse the repository at this point in the history
  • Loading branch information
mgr-inz-rafal committed Dec 28, 2019
1 parent 0fec590 commit f191e91
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -1186,6 +1186,7 @@ Released 2018-09-13
[`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
[`module_inception`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_inception
[`module_name_repetitions`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions
[`modulo_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#modulo_arithmetic
[`modulo_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#modulo_one
[`multiple_crate_versions`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_crate_versions
[`multiple_inherent_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#multiple_inherent_impl
Expand Down
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -6,7 +6,7 @@

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

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

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

Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Expand Up @@ -239,6 +239,7 @@ pub mod misc_early;
pub mod missing_const_for_fn;
pub mod missing_doc;
pub mod missing_inline;
pub mod modulo_arithmetic;
pub mod mul_add;
pub mod multiple_crate_versions;
pub mod mut_key;
Expand Down Expand Up @@ -667,6 +668,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
&missing_const_for_fn::MISSING_CONST_FOR_FN,
&missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS,
&missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS,
&modulo_arithmetic::MODULO_ARITHMETIC,
&mul_add::MANUAL_MUL_ADD,
&multiple_crate_versions::MULTIPLE_CRATE_VERSIONS,
&mut_key::MUTABLE_KEY_TYPE,
Expand Down Expand Up @@ -943,6 +945,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
store.register_late_pass(|| box comparison_chain::ComparisonChain);
store.register_late_pass(|| box mul_add::MulAddCheck);
store.register_late_pass(|| box mut_key::MutableKeyType);
store.register_late_pass(|| box modulo_arithmetic::ModuloArithmetic);
store.register_early_pass(|| box reference::DerefAddrOf);
store.register_early_pass(|| box reference::RefInDeref);
store.register_early_pass(|| box double_parens::DoubleParens);
Expand Down Expand Up @@ -1003,6 +1006,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&misc::FLOAT_CMP_CONST),
LintId::of(&missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS),
LintId::of(&missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS),
LintId::of(&modulo_arithmetic::MODULO_ARITHMETIC),
LintId::of(&panic_unimplemented::PANIC),
LintId::of(&panic_unimplemented::TODO),
LintId::of(&panic_unimplemented::UNIMPLEMENTED),
Expand Down
149 changes: 149 additions & 0 deletions clippy_lints/src/modulo_arithmetic.rs
@@ -0,0 +1,149 @@
use crate::consts::{constant, Constant};
use crate::utils::{sext, span_lint_and_then};
use if_chain::if_chain;
use rustc::declare_lint_pass;
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::ty::{self};
use rustc_session::declare_tool_lint;
use std::fmt::Display;

declare_clippy_lint! {
/// **What it does:** Checks for modulo arithemtic.
///
/// **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`.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let x = -17 % 3;
/// ```
pub MODULO_ARITHMETIC,
restriction,
"any modulo arithmetic statement"
}

declare_lint_pass!(ModuloArithmetic => [MODULO_ARITHMETIC]);

struct OperandInfo {
string_representation: Option<String>,
is_negative: bool,
is_integral: bool,
}

fn analyze_operand(operand: &Expr, cx: &LateContext<'_, '_>, expr: &Expr) -> Option<OperandInfo> {
match constant(cx, cx.tables, operand) {
Some((Constant::Int(v), _)) => match cx.tables.expr_ty(expr).kind {
ty::Int(ity) => {
let value = sext(cx.tcx, v, ity);
return Some(OperandInfo {
string_representation: Some(value.to_string()),
is_negative: value < 0,
is_integral: true,
});
},
ty::Uint(_) => {
return Some(OperandInfo {
string_representation: None,
is_negative: false,
is_integral: true,
});
},
_ => {},
},
Some((Constant::F32(f), _)) => {
return Some(floating_point_operand_info(&f));
},
Some((Constant::F64(f), _)) => {
return Some(floating_point_operand_info(&f));
},
_ => {},
}
None
}

fn floating_point_operand_info<T: Display + PartialOrd + From<f32>>(f: &T) -> OperandInfo {
OperandInfo {
string_representation: Some(format!("{:.3}", *f)),
is_negative: *f < 0.0.into(),
is_integral: false,
}
}

fn might_have_negative_value(t: &ty::TyS<'_>) -> bool {
t.is_signed() || t.is_floating_point()
}

fn check_const_operands<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
expr: &'tcx Expr,
lhs_operand: &OperandInfo,
rhs_operand: &OperandInfo,
) {
if lhs_operand.is_negative ^ rhs_operand.is_negative {
span_lint_and_then(
cx,
MODULO_ARITHMETIC,
expr.span,
&format!(
"you are using modulo operator on constants with different signs: `{} % {}`",
lhs_operand.string_representation.as_ref().unwrap(),
rhs_operand.string_representation.as_ref().unwrap()
),
|db| {
db.note("double check for expected result especially when interoperating with different languages");
if lhs_operand.is_integral {
db.note("or consider using `rem_euclid` or similar function");
}
},
);
}
}

fn check_non_const_operands<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, operand: &Expr) {
let operand_type = cx.tables.expr_ty(operand);
if might_have_negative_value(operand_type) {
span_lint_and_then(
cx,
MODULO_ARITHMETIC,
expr.span,
"you are using modulo operator on types that might have different signs",
|db| {
db.note("double check for expected result especially when interoperating with different languages");
if operand_type.is_integral() {
db.note("or consider using `rem_euclid` or similar function");
}
},
);
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ModuloArithmetic {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
match &expr.kind {
ExprKind::Binary(op, lhs, rhs) | ExprKind::AssignOp(op, lhs, rhs) => {
if let BinOpKind::Rem = op.node {
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);
}
}
};
},
_ => {},
}
}
}
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Expand Up @@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;

// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 341] = [
pub const ALL_LINTS: [Lint; 342] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -1183,6 +1183,13 @@ pub const ALL_LINTS: [Lint; 341] = [
deprecation: None,
module: "enum_variants",
},
Lint {
name: "modulo_arithmetic",
group: "restriction",
desc: "any modulo arithmetic statement",
deprecation: None,
module: "modulo_arithmetic",
},
Lint {
name: "modulo_one",
group: "correctness",
Expand Down

0 comments on commit f191e91

Please sign in to comment.