Review: use span_note_and_lint, present offending expression in message

Author: mgr-inz-rafal

clippy_lints/src/modulo_arithmetic.rs

@@ -1,5 +1,5 @@
 use crate::consts::{constant, Constant};
-use crate::utils::{sext, span_help_and_lint};
+use crate::utils::{sext, span_note_and_lint};
 use if_chain::if_chain;
 use rustc::declare_lint_pass;
 use rustc::hir::*;
@@ -30,22 +30,23 @@ declare_clippy_lint! {
 
 declare_lint_pass!(ModuloArithmetic => [MODULO_ARITHMETIC]);
 
-fn is_negative(operand: &Expr, cx: &LateContext<'_, '_>, expr: &Expr) -> Option<bool> {
+fn is_negative(operand: &Expr, cx: &LateContext<'_, '_>, expr: &Expr) -> Option<(bool, Option<String>)> {
     match constant(cx, cx.tables, operand) {
         Some((Constant::Int(v), _)) => match cx.tables.expr_ty(expr).kind {
             ty::Int(ity) => {
-                return Some(sext(cx.tcx, v, ity) < 0);
+                let value = sext(cx.tcx, v, ity);
+                return Some((value < 0, Some(value.to_string())));
             },
             ty::Uint(_) => {
-                return Some(false);
+                return Some((false, None));
             },
             _ => {},
         },
         Some((Constant::F32(f), _)) => {
-            return Some(f < 0.0);
+            return Some((f < 0.0, Some(format!("{:.3}", f))));
         },
         Some((Constant::F64(f), _)) => {
-            return Some(f < 0.0);
+            return Some((f < 0.0, Some(format!("{:.3}", f))));
         },
         _ => {},
     }
@@ -68,15 +69,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ModuloArithmetic {
                         if_chain! {
                             if let Some(lhs_negative) = lhs_negative;
                             if let Some(rhs_negative) = rhs_negative;
-                            if lhs_negative ^ rhs_negative;
+                            if lhs_negative.0 ^ rhs_negative.0;
                             then
                             {
-                                span_help_and_lint(
+                                span_note_and_lint(
                                     cx,
                                     MODULO_ARITHMETIC,
                                     expr.span,
-                                    "modulo arithemtic detected",
-                                    "modulo operands have different signs, double check all modulo arithemtic results when interoperating with different languages",
+                                    &format!(
+                                        "you are using modulo operator on constants with different signs: `{} % {}`", lhs_negative.1.unwrap(), rhs_negative.1.unwrap()
+                                    ),
+                                    expr.span,
+                                    "double check for expected result especially when interoperating with different languages",
                                 );
                                 return;
                             }
@@ -85,12 +89,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ModuloArithmetic {
                         // Operands are not consts, fallback to the 'might have different signs' lint
                         let lhs_type = cx.tables.expr_ty(lhs);
                         if might_have_negative_value(lhs_type) {
-                            span_help_and_lint(
+                            span_note_and_lint(
                                 cx,
                                 MODULO_ARITHMETIC,
                                 expr.span,
-                                "modulo arithemtic detected",
-                                "modulo operands might have different signs, double check all modulo arithemtic results when interoperating with different languages",
+                                "you are using modulo operator on types that might have different signs",
+                                expr.span,
+                                "double check for expected result especially when interoperating with different languages",
                             );
                         }
                     }

tests/ui/modulo_arithmetic.rs

@@ -18,6 +18,7 @@ fn main() {
     (1 + 2) % (1 - 2);
     (1.1 - 2.3) % (1.1 + 2.3);
     (1.1 + 2.3) % (1.1 - 2.3);
+    35 * (7 - 4 * 2) % (-500 * -600);
 
     -1i8 % 2i8;
     1i8 % -2i8;