Trigger modulo_one lint also on -1.

Author: mlegner

clippy_lints/src/misc.rs

@@ -18,7 +18,7 @@ use crate::utils::sugg::Sugg;
 use crate::utils::{
     get_item_name, get_parent_expr, higher, implements_trait, in_constant, is_integer_const, iter_input_pats,
     last_path_segment, match_qpath, match_trait_method, paths, snippet, snippet_opt, span_lint, span_lint_and_sugg,
-    span_lint_and_then, span_lint_hir_and_then, SpanlessEq,
+    span_lint_and_then, span_lint_hir_and_then, SpanlessEq, unsext,
 };
 
 declare_clippy_lint! {
@@ -139,23 +139,26 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for getting the remainder of a division by one.
+    /// **What it does:** Checks for getting the remainder of a division by one or minus
+    /// one.
     ///
-    /// **Why is this bad?** The result can only ever be zero. No one will write
-    /// such code deliberately, unless trying to win an Underhanded Rust
-    /// Contest. Even for that contest, it's probably a bad idea. Use something more
-    /// underhanded.
+    /// **Why is this bad?** The result for a divisor of one can only ever be zero; for
+    /// minus one it can cause panic/overflow (if the left operand is the minimal value of
+    /// the respective integer type) or results in zero. No one will write such code
+    /// deliberately, unless trying to win an Underhanded Rust Contest. Even for that
+    /// contest, it's probably a bad idea. Use something more underhanded.
     ///
     /// **Known problems:** None.
     ///
     /// **Example:**
     /// ```rust
     /// # let x = 1;
     /// let a = x % 1;
+    /// let a = x % -1;
     /// ```
     pub MODULO_ONE,
     correctness,
-    "taking a number modulo 1, which always returns 0"
+    "taking a number modulo +/-1, which can either panic/overflow or always returns 0"
 }
 
 declare_clippy_lint! {
@@ -429,8 +432,17 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints {
                         }
                         diag.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`");
                     });
-                } else if op == BinOpKind::Rem && is_integer_const(cx, right, 1) {
-                    span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
+                } else if op == BinOpKind::Rem {
+                    if is_integer_const(cx, right, 1) {
+                        span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
+                    }
+
+                    if let ty::Int(ity) = cx.typeck_results().expr_ty(right).kind() {
+                        if is_integer_const(cx, right, unsext(cx.tcx, -1, *ity)) {
+                            span_lint(cx, MODULO_ONE, expr.span,
+                                "any number modulo -1 will panic/overflow or result in 0");
+                        }
+                    };
                 }
             },
             _ => {},

tests/ui/modulo_one.rs

@@ -2,13 +2,22 @@
 #![allow(clippy::no_effect, clippy::unnecessary_operation)]
 
 static STATIC_ONE: usize = 2 - 1;
+static STATIC_NEG_ONE: i64 = 1 - 2;
 
 fn main() {
     10 % 1;
+    10 % -1;
     10 % 2;
+    i32::MIN % (-1); // also caught by rustc
 
     const ONE: u32 = 1 * 1;
+    const NEG_ONE: i64 = 1 - 2;
+    const INT_MIN: i64 = i64::MIN;
 
     2 % ONE;
-    5 % STATIC_ONE;
+    5 % STATIC_ONE; // NOT caught by lint
+    2 % NEG_ONE;
+    5 % STATIC_NEG_ONE; // NOT caught by lint
+    INT_MIN % NEG_ONE; // also caught by rustc
+    INT_MIN % STATIC_NEG_ONE; // ONLY caught by rustc
 }

tests/ui/modulo_one.stderr

@@ -1,30 +1,74 @@
+error: this arithmetic operation will overflow
+  --> $DIR/modulo_one.rs:11:5
+   |
+LL |     i32::MIN % (-1); // also caught by rustc
+   |     ^^^^^^^^^^^^^^^ attempt to compute the remainder of `i32::MIN % -1_i32`, which would overflow
+   |
+   = note: `#[deny(arithmetic_overflow)]` on by default
+
+error: this arithmetic operation will overflow
+  --> $DIR/modulo_one.rs:21:5
+   |
+LL |     INT_MIN % NEG_ONE; // also caught by rustc
+   |     ^^^^^^^^^^^^^^^^^ attempt to compute the remainder of `i64::MIN % -1_i64`, which would overflow
+
+error: this arithmetic operation will overflow
+  --> $DIR/modulo_one.rs:22:5
+   |
+LL |     INT_MIN % STATIC_NEG_ONE; // ONLY caught by rustc
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^ attempt to compute the remainder of `i64::MIN % -1_i64`, which would overflow
+
 error: any number modulo 1 will be 0
-  --> $DIR/modulo_one.rs:7:5
+  --> $DIR/modulo_one.rs:8:5
    |
 LL |     10 % 1;
    |     ^^^^^^
    |
    = note: `-D clippy::modulo-one` implied by `-D warnings`
 
+error: any number modulo -1 will panic/overflow or result in 0
+  --> $DIR/modulo_one.rs:9:5
+   |
+LL |     10 % -1;
+   |     ^^^^^^^
+
+error: any number modulo -1 will panic/overflow or result in 0
+  --> $DIR/modulo_one.rs:11:5
+   |
+LL |     i32::MIN % (-1); // also caught by rustc
+   |     ^^^^^^^^^^^^^^^
+
 error: the operation is ineffective. Consider reducing it to `1`
-  --> $DIR/modulo_one.rs:10:22
+  --> $DIR/modulo_one.rs:13:22
    |
 LL |     const ONE: u32 = 1 * 1;
    |                      ^^^^^
    |
    = note: `-D clippy::identity-op` implied by `-D warnings`
 
 error: the operation is ineffective. Consider reducing it to `1`
-  --> $DIR/modulo_one.rs:10:22
+  --> $DIR/modulo_one.rs:13:22
    |
 LL |     const ONE: u32 = 1 * 1;
    |                      ^^^^^
 
 error: any number modulo 1 will be 0
-  --> $DIR/modulo_one.rs:12:5
+  --> $DIR/modulo_one.rs:17:5
    |
 LL |     2 % ONE;
    |     ^^^^^^^
 
-error: aborting due to 4 previous errors
+error: any number modulo -1 will panic/overflow or result in 0
+  --> $DIR/modulo_one.rs:19:5
+   |
+LL |     2 % NEG_ONE;
+   |     ^^^^^^^^^^^
+
+error: any number modulo -1 will panic/overflow or result in 0
+  --> $DIR/modulo_one.rs:21:5
+   |
+LL |     INT_MIN % NEG_ONE; // also caught by rustc
+   |     ^^^^^^^^^^^^^^^^^
+
+error: aborting due to 11 previous errors