integer_arithmetic: check all/only overflowing ops

Make the `integer_arithmetic` lint detect all the operations that are
defined as being capable of overflow in the Rust Reference.

Changes:

* Disallow bit shifting (`<<`, `>>`). They overflow if the RHS equals or
  exceeds the bit width.
* Allow unsigned division and remainder (they cannot overflow).

Author: michaelsproul

clippy_lints/src/arithmetic.rs

@@ -2,14 +2,20 @@ use crate::consts::constant_simple;
 use crate::utils::span_lint;
 use rustc_hir as hir;
 use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::Span;
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for plain integer arithmetic.
+    /// **What it does:** Checks for integer arithmetic operations that could overflow.
     ///
-    /// **Why is this bad?** This is only checked against overflow in debug builds.
-    /// In some applications one wants explicitly checked, wrapping or saturating
+    /// Specifically, checks for any operators (`+`, `-`, `*`, `<<`, etc) which are capable
+    /// of overflowing according to the [Rust
+    /// Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow).
+    /// No bounds analysis or sophisticated reasoning is attempted.
+    ///
+    /// **Why is this bad?** Integer overflow will trigger a panic in debug builds or will wrap in
+    /// release mode. In some applications one wants explicitly checked, wrapping or saturating
     /// arithmetic.
     ///
     /// **Known problems:** None.
@@ -21,7 +27,7 @@ declare_clippy_lint! {
     /// ```
     pub INTEGER_ARITHMETIC,
     restriction,
-    "any integer arithmetic statement"
+    "any integer arithmetic expression which could overflow"
 }
 
 declare_clippy_lint! {
@@ -71,8 +77,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic {
                     | hir::BinOpKind::BitAnd
                     | hir::BinOpKind::BitOr
                     | hir::BinOpKind::BitXor
-                    | hir::BinOpKind::Shl
-                    | hir::BinOpKind::Shr
                     | hir::BinOpKind::Eq
                     | hir::BinOpKind::Lt
                     | hir::BinOpKind::Le
@@ -84,6 +88,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic {
 
                 let (l_ty, r_ty) = (cx.tables.expr_ty(l), cx.tables.expr_ty(r));
                 if l_ty.peel_refs().is_integral() && r_ty.peel_refs().is_integral() {
+                    // Allow unsigned integer division and modulo (it can't overflow)
+                    if let ty::Uint(_) = l_ty.kind {
+                        if op.node == hir::BinOpKind::Div || op.node == hir::BinOpKind::Rem {
+                            return;
+                        }
+                    }
                     span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected");
                     self.expr_span = Some(expr.span);
                 } else if l_ty.peel_refs().is_floating_point() && r_ty.peel_refs().is_floating_point() {

src/lintlist/mod.rs

@@ -846,7 +846,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
     Lint {
         name: "integer_arithmetic",
         group: "restriction",
-        desc: "any integer arithmetic statement",
+        desc: "any integer arithmetic expression which could overflow",
         deprecation: None,
         module: "arithmetic",
     },

tests/ui/integer_arithmetic.rs

@@ -18,6 +18,8 @@ fn main() {
     i / 2; // no error, this is part of the expression in the preceding line
     i - 2 + 2 - i;
     -i;
+    i >> 1;
+    i << 1;
 
     // no error, overflows are checked by `overflowing_literals`
     -1;
@@ -26,18 +28,16 @@ fn main() {
     i & 1; // no wrapping
     i | 1;
     i ^ 1;
-    i >> 1;
-    i << 1;
 
     i += 1;
     i -= 1;
     i *= 2;
     i /= 2;
     i %= 2;
-
-    // no errors
     i <<= 3;
     i >>= 2;
+
+    // no errors
     i |= 1;
     i &= 1;
     i ^= i;
@@ -73,8 +73,6 @@ fn main() {
             1 + 1
         };
     }
-
-
 }
 
 // warn on references as well! (#5328)
@@ -84,6 +82,42 @@ pub fn int_arith_ref() {
     &3 + &1;
 }
 
+pub fn unsigned() {
+    let mut i = 1000u64;
+
+    // should error
+    i + 2;
+    i - 2;
+    i * 2;
+    i << 2;
+    i >> i;
+
+    // should error
+    i += 2;
+    i -= 2;
+    i *= i;
+    i <<= 2;
+    i >>= 2;
+
+    // should not error
+    i | 3;
+    i & 2;
+    i ^ 4;
+    i / 2;
+    i % 5;
+
+    // should not error
+    i |= 1;
+    i &= 1;
+    i ^= i;
+    i /= i;
+    i %= 6;
+
+    // should not error
+    !i;
+    !(!i);
+}
+
 pub fn foo(x: &i32) -> i32 {
     let a = 5;
     a + x

tests/ui/integer_arithmetic.stderr

@@ -31,6 +31,18 @@ error: integer arithmetic detected
 LL |     -i;
    |     ^^
 
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:21:5
+   |
+LL |     i >> 1;
+   |     ^^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:22:5
+   |
+LL |     i << 1;
+   |     ^^^^^^
+
 error: integer arithmetic detected
   --> $DIR/integer_arithmetic.rs:32:5
    |
@@ -62,46 +74,118 @@ LL |     i %= 2;
    |     ^^^^^^
 
 error: integer arithmetic detected
-  --> $DIR/integer_arithmetic.rs:82:5
+  --> $DIR/integer_arithmetic.rs:37:5
+   |
+LL |     i <<= 3;
+   |     ^^^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:38:5
+   |
+LL |     i >>= 2;
+   |     ^^^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:80:5
    |
 LL |     3 + &1;
    |     ^^^^^^
 
 error: integer arithmetic detected
-  --> $DIR/integer_arithmetic.rs:83:5
+  --> $DIR/integer_arithmetic.rs:81:5
    |
 LL |     &3 + 1;
    |     ^^^^^^
 
 error: integer arithmetic detected
-  --> $DIR/integer_arithmetic.rs:84:5
+  --> $DIR/integer_arithmetic.rs:82:5
    |
 LL |     &3 + &1;
    |     ^^^^^^^
 
 error: integer arithmetic detected
   --> $DIR/integer_arithmetic.rs:89:5
    |
-LL |     a + x
+LL |     i + 2;
+   |     ^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:90:5
+   |
+LL |     i - 2;
+   |     ^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:91:5
+   |
+LL |     i * 2;
    |     ^^^^^
 
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:92:5
+   |
+LL |     i << 2;
+   |     ^^^^^^
+
 error: integer arithmetic detected
   --> $DIR/integer_arithmetic.rs:93:5
    |
+LL |     i >> i;
+   |     ^^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:96:5
+   |
+LL |     i += 2;
+   |     ^^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:97:5
+   |
+LL |     i -= 2;
+   |     ^^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:98:5
+   |
+LL |     i *= i;
+   |     ^^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:99:5
+   |
+LL |     i <<= 2;
+   |     ^^^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:100:5
+   |
+LL |     i >>= 2;
+   |     ^^^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:123:5
+   |
+LL |     a + x
+   |     ^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:127:5
+   |
 LL |     x + y
    |     ^^^^^
 
 error: integer arithmetic detected
-  --> $DIR/integer_arithmetic.rs:97:5
+  --> $DIR/integer_arithmetic.rs:131:5
    |
 LL |     x + y
    |     ^^^^^
 
 error: integer arithmetic detected
-  --> $DIR/integer_arithmetic.rs:101:5
+  --> $DIR/integer_arithmetic.rs:135:5
    |
 LL |     (&x + &y)
    |     ^^^^^^^^^
 
-error: aborting due to 17 previous errors
+error: aborting due to 31 previous errors