From 23df4a0183e0d954d47db98824295411d50f742e Mon Sep 17 00:00:00 2001
From: Michael Sproul <michael@sigmaprime.io>
Date: Mon, 13 Apr 2020 13:11:19 +1000
Subject: [PATCH] Disallow bit-shifting in `integer_arithmetic` lint

With this change, the lint checks all operations that are defined as
being capable of overflow in the Rust Reference.
---
 clippy_lints/src/arithmetic.rs     | 18 ++++++++------
 src/lintlist/mod.rs                |  2 +-
 tests/ui/integer_arithmetic.rs     | 10 +++-----
 tests/ui/integer_arithmetic.stderr | 40 ++++++++++++++++++++++++------
 4 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/clippy_lints/src/arithmetic.rs b/clippy_lints/src/arithmetic.rs
index a138c9d3545..6cbe10a5352 100644
--- a/clippy_lints/src/arithmetic.rs
+++ b/clippy_lints/src/arithmetic.rs
@@ -6,11 +6,17 @@ 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 which could overflow or panic.
     ///
-    /// **Why is this bad?** This is only checked against overflow in debug builds.
-    /// In some applications one wants explicitly checked, wrapping or saturating
-    /// arithmetic.
+    /// 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),
+    /// or which can panic (`/`, `%`). 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. Division by zero will cause a panic in either 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 or panic"
 }
 
 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
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index edebaff9f14..8712f07e9e0 100644
--- a/src/lintlist/mod.rs
+++ b/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 or panic",
         deprecation: None,
         module: "arithmetic",
     },
diff --git a/tests/ui/integer_arithmetic.rs b/tests/ui/integer_arithmetic.rs
index 2fe32c6ace8..7b1b64f390a 100644
--- a/tests/ui/integer_arithmetic.rs
+++ b/tests/ui/integer_arithmetic.rs
@@ -17,6 +17,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;
@@ -25,18 +27,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;
@@ -72,8 +72,6 @@ fn main() {
             1 + 1
         };
     }
-
-
 }
 
 // warn on references as well! (#5328)
diff --git a/tests/ui/integer_arithmetic.stderr b/tests/ui/integer_arithmetic.stderr
index 64c44d7ecc7..83e8a9cde3f 100644
--- a/tests/ui/integer_arithmetic.stderr
+++ b/tests/ui/integer_arithmetic.stderr
@@ -31,6 +31,18 @@ error: integer arithmetic detected
 LL |     -i;
    |     ^^
 
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:20:5
+   |
+LL |     i >> 1;
+   |     ^^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:21:5
+   |
+LL |     i << 1;
+   |     ^^^^^^
+
 error: integer arithmetic detected
   --> $DIR/integer_arithmetic.rs:31:5
    |
@@ -62,46 +74,58 @@ LL |     i %= 2;
    |     ^^^^^^
 
 error: integer arithmetic detected
-  --> $DIR/integer_arithmetic.rs:81:5
+  --> $DIR/integer_arithmetic.rs:36:5
+   |
+LL |     i <<= 3;
+   |     ^^^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:37:5
+   |
+LL |     i >>= 2;
+   |     ^^^^^^^
+
+error: integer arithmetic detected
+  --> $DIR/integer_arithmetic.rs:79:5
    |
 LL |     3 + &1;
    |     ^^^^^^
 
 error: integer arithmetic detected
-  --> $DIR/integer_arithmetic.rs:82:5
+  --> $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:88:5
+  --> $DIR/integer_arithmetic.rs:86:5
    |
 LL |     a + x
    |     ^^^^^
 
 error: integer arithmetic detected
-  --> $DIR/integer_arithmetic.rs:92:5
+  --> $DIR/integer_arithmetic.rs:90:5
    |
 LL |     x + y
    |     ^^^^^
 
 error: integer arithmetic detected
-  --> $DIR/integer_arithmetic.rs:96:5
+  --> $DIR/integer_arithmetic.rs:94:5
    |
 LL |     x + y
    |     ^^^^^
 
 error: integer arithmetic detected
-  --> $DIR/integer_arithmetic.rs:100:5
+  --> $DIR/integer_arithmetic.rs:98:5
    |
 LL |     (&x + &y)
    |     ^^^^^^^^^
 
-error: aborting due to 17 previous errors
+error: aborting due to 21 previous errors