Added identity_assign_op lint

clippy_lints/src/declared_lints.rs

@@ -615,6 +615,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::operators::FLOAT_CMP_INFO,
     crate::operators::FLOAT_CMP_CONST_INFO,
     crate::operators::FLOAT_EQUALITY_WITHOUT_ABS_INFO,
+    crate::operators::IDENTITY_ASSIGN_OP_INFO,
     crate::operators::IDENTITY_OP_INFO,
     crate::operators::IMPOSSIBLE_COMPARISONS_INFO,
     crate::operators::INEFFECTIVE_BIT_MASK_INFO,

clippy_lints/src/operators/identity_assign_op.rs

@@ -0,0 +1,109 @@
+use clippy_utils::consts::{ConstEvalCtxt, Constant};
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::eq_expr_value;
+use rustc_errors::Applicability;
+use rustc_hir::{BinOpKind, Expr, ExprKind, Node, Stmt, StmtKind};
+use rustc_lint::LateContext;
+
+use super::IDENTITY_ASSIGN_OP;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    stmt: &'tcx Stmt<'_>,
+    expr: &'tcx Expr<'_>,
+    op: BinOpKind,
+    left: &'tcx Expr<'_>,
+    right: &'tcx Expr<'_>,
+) {
+    if cx.typeck_results().type_dependent_def_id(expr.hir_id).is_some() {
+        return;
+    }
+
+    let is_identity = match op {
+        BinOpKind::Add => is_zero_or_one(cx, right, 0, true),
+
+        BinOpKind::Sub | BinOpKind::BitOr | BinOpKind::BitXor | BinOpKind::Shl | BinOpKind::Shr => {
+            is_zero_or_one(cx, right, 0, false)
+        },
+
+        BinOpKind::Mul | BinOpKind::Div => is_zero_or_one(cx, right, 1, false),
+
+        _ => false,
+    };
+
+    if is_identity && !is_part_of_series(cx, stmt, op, left) {
+        span_lint_and_sugg(
+            cx,
+            IDENTITY_ASSIGN_OP,
+            stmt.span,
+            "this operation has no effect",
+            "remove it",
+            String::new(),
+            Applicability::MachineApplicable,
+        );
+    }
+}
+
+fn is_part_of_series(cx: &LateContext<'_>, stmt: &Stmt<'_>, op: BinOpKind, left: &Expr<'_>) -> bool {
+    let Node::Block(block) = cx.tcx.parent_hir_node(stmt.hir_id) else {
+        return false;
+    };
+    let Some(index) = block.stmts.iter().position(|other| other.hir_id == stmt.hir_id) else {
+        return false;
+    };
+
+    (index > 0 && is_matching_assign_op(cx, &block.stmts[index - 1], op, left))
+        || (index < block.stmts.len() - 1 && is_matching_assign_op(cx, &block.stmts[index + 1], op, left))
+}
+
+fn is_matching_assign_op(cx: &LateContext<'_>, stmt: &Stmt<'_>, op: BinOpKind, left: &Expr<'_>) -> bool {
+    let StmtKind::Semi(expr) = stmt.kind else {
+        return false;
+    };
+    let ExprKind::AssignOp(other_op, other_left, _) = expr.kind else {
+        return false;
+    };
+    let other_op: BinOpKind = other_op.node.into();
+
+    other_op == op && eq_expr_value(cx, left.span.ctxt(), left, other_left)
+}
+
+fn is_zero_or_one(cx: &LateContext<'_>, expr: &Expr<'_>, expected: u128, negative_float_zero: bool) -> bool {
+    const F16_ZERO: u16 = 0.0_f16.to_bits();
+    const F16_NEGATIVE_ZERO: u16 = (-0.0_f16).to_bits();
+    const F16_ONE: u16 = 1.0_f16.to_bits();
+    const F32_ZERO: u32 = 0.0_f32.to_bits();
+    const F32_NEGATIVE_ZERO: u32 = (-0.0_f32).to_bits();
+    const F32_ONE: u32 = 1.0_f32.to_bits();
+    const F64_ZERO: u64 = 0.0_f64.to_bits();
+    const F64_NEGATIVE_ZERO: u64 = (-0.0_f64).to_bits();
+    const F64_ONE: u64 = 1.0_f64.to_bits();
+    const F128_ZERO: u128 = 0.0_f128.to_bits();
+    const F128_NEGATIVE_ZERO: u128 = (-0.0_f128).to_bits();
+    const F128_ONE: u128 = 1.0_f128.to_bits();
+
+    let (expected_f16, expected_f32, expected_f64, expected_f128) = match expected {
+        0 if negative_float_zero => (
+            F16_NEGATIVE_ZERO,
+            F32_NEGATIVE_ZERO,
+            F64_NEGATIVE_ZERO,
+            F128_NEGATIVE_ZERO,
+        ),
+        0 => (F16_ZERO, F32_ZERO, F64_ZERO, F128_ZERO),
+        1 => (F16_ONE, F32_ONE, F64_ONE, F128_ONE),
+        _ => return false,
+    };
+
+    let Some(value) = ConstEvalCtxt::new(cx).eval(expr).map(Constant::peel_refs) else {
+        return false;
+    };
+
+    match value {
+        Constant::Int(value) => value == expected,
+        Constant::F16(value) => value == expected_f16,
+        Constant::F32(value) => value.to_bits() == expected_f32,
+        Constant::F64(value) => value.to_bits() == expected_f64,
+        Constant::F128(value) => value == expected_f128,
+        _ => false,
+    }
+}

clippy_lints/src/operators/mod.rs

@@ -10,6 +10,7 @@ mod eq_op;
 mod erasing_op;
 mod float_cmp;
 mod float_equality_without_abs;
+mod identity_assign_op;
 mod identity_op;
 mod integer_division;
 mod integer_division_remainder_used;
@@ -31,7 +32,7 @@ pub(crate) mod arithmetic_side_effects;
 
 use clippy_config::Conf;
 use clippy_utils::msrvs::Msrv;
-use rustc_hir::{Body, Expr, ExprKind, UnOp};
+use rustc_hir::{Body, Expr, ExprKind, Stmt, StmtKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
 
@@ -518,6 +519,29 @@ declare_clippy_lint! {
     "float equality check without `.abs()`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for assignment operations with identity operands.
+    ///
+    /// ### Why is this bad?
+    /// These operations have no effect and can be removed for clarity.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let mut x = 1;
+    /// x += 0;
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let mut x = 1;
+    /// x;
+    /// ```
+    #[clippy::version = "1.97.0"]
+    pub IDENTITY_ASSIGN_OP,
+    pedantic,
+    "assignment operation with an identity operand"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for identity operations, e.g., `x + 0`.
@@ -1007,6 +1031,7 @@ impl_lint_pass!(Operators => [
     FLOAT_CMP,
     FLOAT_CMP_CONST,
     FLOAT_EQUALITY_WITHOUT_ABS,
+    IDENTITY_ASSIGN_OP,
     IDENTITY_OP,
     IMPOSSIBLE_COMPARISONS,
     INEFFECTIVE_BIT_MASK,
@@ -1112,6 +1137,16 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
         }
     }
 
+    fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
+        let StmtKind::Semi(e) = stmt.kind else { return };
+        let ExprKind::AssignOp(op, lhs, rhs) = e.kind else {
+            return;
+        };
+
+        let bin_op = op.node.into();
+        identity_assign_op::check(cx, stmt, e, bin_op, lhs, rhs);
+    }
+
     fn check_expr_post(&mut self, _: &LateContext<'_>, e: &Expr<'_>) {
         self.arithmetic_context.expr_post(e.hir_id);
     }

tests/ui/identity_assign_op.fixed

@@ -0,0 +1,177 @@
+#![warn(clippy::identity_assign_op)]
+#![allow(unused)]
+
+const ZERO_I64: i64 = 0;
+const ONE_I64: i64 = 1;
+const ZERO_U64: u64 = 0;
+const ONE_U64: u64 = 1;
+const ZERO_F32: f32 = 0.0;
+const NEGATIVE_ZERO_F32: f32 = -0.0;
+const ONE_F32: f32 = 1.0;
+const ZERO_F64: f64 = 0.0;
+const NEGATIVE_ZERO_F64: f64 = -0.0;
+const ONE_F64: f64 = 1.0;
+
+#[rustfmt::skip]
+fn test_identity_values() {
+    // Literals
+    let mut signed = 1_i64;
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+    let mut unsigned = 1_u64;
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+    let mut float32 = 1.0_f32;
+
+    float32 += 0.0; // no error
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+    let mut float64 = 1.0_f64;
+
+    float64 += 0.0; // no error
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+    let mut subtraction = 1.0_f32;
+    subtraction -= -0.0; // no error
+
+    // Constants
+    let mut signed = 1_i64;
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+    let mut unsigned = 1_u64;
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+    let mut float32 = 1.0_f32;
+
+    float32 += ZERO_F32; // no error
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+
+    let mut float64 = 1.0_f64;
+
+    float64 += ZERO_F64; // no error
+
+
+    //~^ identity_assign_op
+
+
+    //~^ identity_assign_op
+}
+
+fn test_non_identity_values() {
+    let mut value = 1_i64;
+
+    value += 1;
+    value *= 2;
+    value -= 1;
+    value <<= 1;
+}
+
+fn test_series() {
+    let mut series = 1_i64;
+
+    series += 0; // no error: part of a series
+    series += 1;
+    series += 2;
+
+    series <<= 0; // no error: part of a two-statement series
+    series <<= 1;
+
+    series *= 2;
+    series *= 1; // no error: at the end of a series
+}
+
+fn test_user_defined_operators() {
+    let mut custom = Custom(1);
+    custom += 0; // no error: user-defined operator
+    custom *= 1; // no error: user-defined operator
+}
+
+fn test_macros() {
+    let mut custom = Custom(1);
+    custom -= 0; // no error: macro-generated user-defined operator
+}
+
+struct Custom(i64);
+
+impl std::ops::AddAssign<i64> for Custom {
+    fn add_assign(&mut self, rhs: i64) {
+        self.0 += rhs + 1;
+    }
+}
+
+impl std::ops::MulAssign<i64> for Custom {
+    fn mul_assign(&mut self, rhs: i64) {
+        self.0 *= rhs + 1;
+    }
+}
+
+macro_rules! impl_sub_assign {
+    () => {
+        impl std::ops::SubAssign<i64> for Custom {
+            fn sub_assign(&mut self, rhs: i64) {
+                self.0 -= rhs + 1;
+            }
+        }
+    };
+}
+
+impl_sub_assign!();