[arithmetic-side-effects] Consider negative numbers and add more tests

Author: c410-f3r

clippy_lints/src/operators/arithmetic_side_effects.rs

@@ -2,7 +2,7 @@ use super::ARITHMETIC_SIDE_EFFECTS;
 use clippy_utils::{
     consts::{constant, constant_simple},
     diagnostics::span_lint,
-    peel_hir_expr_refs,
+    peel_hir_expr_refs, peel_hir_expr_unary,
 };
 use rustc_ast as ast;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
@@ -98,8 +98,11 @@ impl ArithmeticSideEffects {
     }
 
     /// If `expr` is not a literal integer like `1`, returns `None`.
+    ///
+    /// Returns the absolute value of the expression, if this is an integer literal.
     fn literal_integer(expr: &hir::Expr<'_>) -> Option<u128> {
-        if let hir::ExprKind::Lit(ref lit) = expr.kind && let ast::LitKind::Int(n, _) = lit.node {
+        let actual = peel_hir_expr_unary(expr).0;
+        if let hir::ExprKind::Lit(ref lit) = actual.kind && let ast::LitKind::Int(n, _) = lit.node {
             Some(n)
         }
         else {
@@ -123,12 +126,12 @@ impl ArithmeticSideEffects {
         if !matches!(
             op.node,
             hir::BinOpKind::Add
-                | hir::BinOpKind::Sub
-                | hir::BinOpKind::Mul
                 | hir::BinOpKind::Div
+                | hir::BinOpKind::Mul
                 | hir::BinOpKind::Rem
                 | hir::BinOpKind::Shl
                 | hir::BinOpKind::Shr
+                | hir::BinOpKind::Sub
         ) {
             return;
         };

clippy_utils/src/lib.rs

@@ -2258,6 +2258,18 @@ pub fn peel_n_hir_expr_refs<'a>(expr: &'a Expr<'a>, count: usize) -> (&'a Expr<'
     (e, count - remaining)
 }
 
+/// Peels off all unary operators of an expression. Returns the underlying expression and the number
+/// of operators removed.
+pub fn peel_hir_expr_unary<'a>(expr: &'a Expr<'a>) -> (&'a Expr<'a>, usize) {
+    let mut count: usize = 0;
+    let mut curr_expr = expr;
+    while let ExprKind::Unary(_, local_expr) = curr_expr.kind {
+        count = count.wrapping_add(1);
+        curr_expr = local_expr;
+    }
+    (curr_expr, count)
+}
+
 /// Peels off all references on the expression. Returns the underlying expression and the number of
 /// references removed.
 pub fn peel_hir_expr_refs<'a>(expr: &'a Expr<'a>) -> (&'a Expr<'a>, usize) {

tests/ui-toml/arithmetic_side_effects_allowed/arithmetic_side_effects_allowed.rs

@@ -107,7 +107,7 @@ fn rhs_is_different() {
 fn unary() {
     // is explicitly on the list
     let _ = -OutOfNames;
-    // is specifically on the list
+    // is explicitly on the list
     let _ = -Foo;
     // not on the list
     let _ = -Bar;

tests/ui/arithmetic_side_effects.rs

@@ -2,6 +2,7 @@
     clippy::assign_op_pattern,
     clippy::erasing_op,
     clippy::identity_op,
+    clippy::no_effect,
     clippy::op_ref,
     clippy::unnecessary_owned_empty_strings,
     arithmetic_overflow,
@@ -125,6 +126,18 @@ pub fn non_overflowing_ops_or_ops_already_handled_by_the_compiler_should_not_tri
     _n *= &0;
     _n *= 1;
     _n *= &1;
+    _n += -0;
+    _n += &-0;
+    _n -= -0;
+    _n -= &-0;
+    _n /= -99;
+    _n /= &-99;
+    _n %= -99;
+    _n %= &-99;
+    _n *= -0;
+    _n *= &-0;
+    _n *= -1;
+    _n *= &-1;
 
     // Binary
     _n = _n + 0;
@@ -158,7 +171,7 @@ pub fn non_overflowing_ops_or_ops_already_handled_by_the_compiler_should_not_tri
     _n = -&i32::MIN;
 }
 
-pub fn runtime_ops() {
+pub fn unknown_ops_or_runtime_ops_that_can_overflow() {
     let mut _n = i32::MAX;
 
     // Assign
@@ -172,6 +185,16 @@ pub fn runtime_ops() {
     _n %= &0;
     _n *= 2;
     _n *= &2;
+    _n += -1;
+    _n += &-1;
+    _n -= -1;
+    _n -= &-1;
+    _n /= -0;
+    _n /= &-0;
+    _n %= -0;
+    _n %= &-0;
+    _n *= -2;
+    _n *= &-2;
 
     // Binary
     _n = _n + 1;
@@ -225,4 +248,46 @@ pub fn runtime_ops() {
     _n = -&_n;
 }
 
+// Copied and pasted from the `integer_arithmetic` lint for comparison.
+pub fn integer_arithmetic() {
+    let mut i = 1i32;
+    let mut var1 = 0i32;
+    let mut var2 = -1i32;
+
+    1 + i;
+    i * 2;
+    1 % i / 2;
+    i - 2 + 2 - i;
+    -i;
+    i >> 1;
+    i << 1;
+
+    -1;
+    -(-1);
+
+    i & 1;
+    i | 1;
+    i ^ 1;
+
+    i += 1;
+    i -= 1;
+    i *= 2;
+    i /= 2;
+    i /= 0;
+    i /= -1;
+    i /= var1;
+    i /= var2;
+    i %= 2;
+    i %= 0;
+    i %= -1;
+    i %= var1;
+    i %= var2;
+    i <<= 3;
+    i >>= 2;
+
+    i |= 1;
+    i &= 1;
+    i ^= i;
+}
+
 fn main() {}