Auto merge of #9483 - c410-f3r:arith, r=Jarcho

[arithmetic-side-effects] Finish non-overflowing ops

Extends https://github.com/rust-lang/rust-clippy/pull/9474 to also take into consideration "raw" binary operations. For example, `let a = b / 2` and `let a = 1 * b` won't trigger the lint.

changelog: [arithmetic-side-effects] Finish non-overflowing ops

clippy_lints/src/operators/arithmetic_side_effects.rs

@@ -42,26 +42,23 @@ impl ArithmeticSideEffects {
         }
     }
 
-    /// Checks assign operators (+=, -=, *=, /=) of integers in a non-constant environment that
-    /// won't overflow.
-    fn has_valid_assign_op(op: &Spanned<hir::BinOpKind>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool {
-        if !Self::is_literal_integer(rhs, rhs_refs) {
-            return false;
-        }
+    /// Assuming that `expr` is a literal integer, checks operators (+=, -=, *, /) in a
+    /// non-constant environment that won't overflow.
+    fn has_valid_op(op: &Spanned<hir::BinOpKind>, expr: &hir::Expr<'_>) -> bool {
         if let hir::BinOpKind::Add | hir::BinOpKind::Sub = op.node
-            && let hir::ExprKind::Lit(ref lit) = rhs.kind
+            && let hir::ExprKind::Lit(ref lit) = expr.kind
             && let ast::LitKind::Int(0, _) = lit.node
         {
             return true;
         }
         if let hir::BinOpKind::Div | hir::BinOpKind::Rem = op.node
-            && let hir::ExprKind::Lit(ref lit) = rhs.kind
+            && let hir::ExprKind::Lit(ref lit) = expr.kind
             && !matches!(lit.node, ast::LitKind::Int(0, _))
         {
             return true;
         }
         if let hir::BinOpKind::Mul = op.node
-            && let hir::ExprKind::Lit(ref lit) = rhs.kind
+            && let hir::ExprKind::Lit(ref lit) = expr.kind
             && let ast::LitKind::Int(0 | 1, _) = lit.node
         {
             return true;
@@ -69,12 +66,6 @@ impl ArithmeticSideEffects {
         false
     }
 
-    /// Checks "raw" binary operators (+, -, *, /) of integers in a non-constant environment
-    /// already handled by the CTFE.
-    fn has_valid_bin_op(lhs: &hir::Expr<'_>, lhs_refs: Ty<'_>, rhs: &hir::Expr<'_>, rhs_refs: Ty<'_>) -> bool {
-        Self::is_literal_integer(lhs, lhs_refs) && Self::is_literal_integer(rhs, rhs_refs)
-    }
-
     /// Checks if the given `expr` has any of the inner `allowed` elements.
     fn is_allowed_ty(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
         self.allowed.contains(
@@ -95,7 +86,8 @@ impl ArithmeticSideEffects {
     }
 
     fn issue_lint(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
-        span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, "arithmetic detected");
+        let msg = "arithmetic operation that can potentially result in unexpected side-effects";
+        span_lint(cx, ARITHMETIC_SIDE_EFFECTS, expr.span, msg);
         self.expr_span = Some(expr.span);
     }
 
@@ -127,13 +119,18 @@ impl ArithmeticSideEffects {
         if self.is_allowed_ty(cx, lhs) || self.is_allowed_ty(cx, rhs) {
             return;
         }
-        let lhs_refs = cx.typeck_results().expr_ty(lhs).peel_refs();
-        let rhs_refs = cx.typeck_results().expr_ty(rhs).peel_refs();
-        let has_valid_assign_op = Self::has_valid_assign_op(op, rhs, rhs_refs);
-        if has_valid_assign_op || Self::has_valid_bin_op(lhs, lhs_refs, rhs, rhs_refs) {
-            return;
+        let has_valid_op = match (
+            Self::is_literal_integer(lhs, cx.typeck_results().expr_ty(lhs).peel_refs()),
+            Self::is_literal_integer(rhs, cx.typeck_results().expr_ty(rhs).peel_refs()),
+        ) {
+            (true, true) => true,
+            (true, false) => Self::has_valid_op(op, lhs),
+            (false, true) => Self::has_valid_op(op, rhs),
+            (false, false) => false,
+        };
+        if !has_valid_op {
+            self.issue_lint(cx, expr);
         }
-        self.issue_lint(cx, expr);
     }
 }
 

tests/ui/arithmetic_side_effects.rs

@@ -1,7 +1,10 @@
 #![allow(
     clippy::assign_op_pattern,
-    unconditional_panic,
-    clippy::unnecessary_owned_empty_strings
+    clippy::erasing_op,
+    clippy::identity_op,
+    clippy::unnecessary_owned_empty_strings,
+    arithmetic_overflow,
+    unconditional_panic
 )]
 #![feature(inline_const, saturating_int_impl)]
 #![warn(clippy::arithmetic_side_effects)]
@@ -30,7 +33,7 @@ pub fn hard_coded_allowed() {
 }
 
 #[rustfmt::skip]
-pub fn non_overflowing_ops() {
+pub fn non_overflowing_const_ops() {
     const _: i32 = { let mut n = 1; n += 1; n };
     let _ = const { let mut n = 1; n += 1; n };
 
@@ -41,33 +44,54 @@ pub fn non_overflowing_ops() {
     let _ = const { let mut n = 1; n = 1 + n; n };
 
     const _: i32 = 1 + 1;
-    let _ = 1 + 1;
     let _ = const { 1 + 1 };
+}
 
-    let mut _a = 1;
-    _a += 0;
-    _a -= 0;
-    _a /= 99;
-    _a %= 99;
-    _a *= 0;
-    _a *= 1;
+pub fn non_overflowing_runtime_ops() {
+    let mut _n = i32::MAX;
+
+    // Assign
+    _n += 0;
+    _n -= 0;
+    _n /= 99;
+    _n %= 99;
+    _n *= 0;
+    _n *= 1;
+
+    // Binary
+    _n = _n + 0;
+    _n = 0 + _n;
+    _n = _n - 0;
+    _n = 0 - _n;
+    _n = _n / 99;
+    _n = _n % 99;
+    _n = _n * 0;
+    _n = 0 * _n;
+    _n = _n * 1;
+    _n = 1 * _n;
+    _n = 23 + 85;
 }
 
 #[rustfmt::skip]
-pub fn overflowing_ops() {
-    let mut _a = 1; _a += 1;
+pub fn overflowing_runtime_ops() {
+    let mut _n = i32::MAX;
 
-    let mut _b = 1; _b = _b + 1;
+    // Assign
+    _n += 1;
+    _n -= 1;
+    _n /= 0;
+    _n %= 0;
+    _n *= 2;
 
-    let mut _c = 1; _c = 1 + _c;
-
-    let mut _a = 1;
-    _a += 1;
-    _a -= 1;
-    _a /= 0;
-    _a %= 0;
-    _a *= 2;
-    _a *= 3;
+    // Binary
+    _n = _n + 1;
+    _n = 1 + _n;
+    _n = _n - 1;
+    _n = 1 - _n;
+    _n = _n / 0;
+    _n = _n % 0;
+    _n = _n * 2;
+    _n = 2 * _n;
 }
 
 fn main() {}

tests/ui/arithmetic_side_effects.stderr

@@ -1,58 +1,82 @@
-error: arithmetic detected
-  --> $DIR/arithmetic_side_effects.rs:58:21
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:80:5
    |
-LL |     let mut _a = 1; _a += 1;
-   |                     ^^^^^^^
+LL |     _n += 1;
+   |     ^^^^^^^
    |
    = note: `-D clippy::arithmetic-side-effects` implied by `-D warnings`
 
-error: arithmetic detected
-  --> $DIR/arithmetic_side_effects.rs:60:26
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:81:5
    |
-LL |     let mut _b = 1; _b = _b + 1;
-   |                          ^^^^^^
-
-error: arithmetic detected
-  --> $DIR/arithmetic_side_effects.rs:62:26
-   |
-LL |     let mut _c = 1; _c = 1 + _c;
-   |                          ^^^^^^
-
-error: arithmetic detected
-  --> $DIR/arithmetic_side_effects.rs:65:5
-   |
-LL |     _a += 1;
+LL |     _n -= 1;
    |     ^^^^^^^
 
-error: arithmetic detected
-  --> $DIR/arithmetic_side_effects.rs:66:5
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:82:5
    |
-LL |     _a -= 1;
+LL |     _n /= 0;
    |     ^^^^^^^
 
-error: arithmetic detected
-  --> $DIR/arithmetic_side_effects.rs:67:5
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:83:5
    |
-LL |     _a /= 0;
+LL |     _n %= 0;
    |     ^^^^^^^
 
-error: arithmetic detected
-  --> $DIR/arithmetic_side_effects.rs:68:5
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:84:5
    |
-LL |     _a %= 0;
+LL |     _n *= 2;
    |     ^^^^^^^
 
-error: arithmetic detected
-  --> $DIR/arithmetic_side_effects.rs:69:5
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:87:10
    |
-LL |     _a *= 2;
-   |     ^^^^^^^
+LL |     _n = _n + 1;
+   |          ^^^^^^
 
-error: arithmetic detected
-  --> $DIR/arithmetic_side_effects.rs:70:5
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:88:10
    |
-LL |     _a *= 3;
-   |     ^^^^^^^
+LL |     _n = 1 + _n;
+   |          ^^^^^^
 
-error: aborting due to 9 previous errors
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:89:10
+   |
+LL |     _n = _n - 1;
+   |          ^^^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:90:10
+   |
+LL |     _n = 1 - _n;
+   |          ^^^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:91:10
+   |
+LL |     _n = _n / 0;
+   |          ^^^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:92:10
+   |
+LL |     _n = _n % 0;
+   |          ^^^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:93:10
+   |
+LL |     _n = _n * 2;
+   |          ^^^^^^
+
+error: arithmetic operation that can potentially result in unexpected side-effects
+  --> $DIR/arithmetic_side_effects.rs:94:10
+   |
+LL |     _n = 2 * _n;
+   |          ^^^^^^
+
+error: aborting due to 13 previous errors