Better check for assign_op_pattern in const context (#15532)

flip1995 · web-flow · commit b43e62584e74 · 2025-08-22T15:51:07.000Z
`assign_op_pattern` can be used in a `const` context if the trait definition as well as the implementation of the corresponding `Assign` pattern is `const` as well. The lint was temporarily deactivated in the compiler repository for all `const` contexts to avoid false positives when operators were potentially constified. This reenables it when appropriate now that the repositories have been synced. Closes #15285 changelog: [`assign_op_pattern`]: trigger in `const` contexts when appropriate r? @flip1995
diff --git a/clippy_lints/src/operators/assign_op_pattern.rs b/clippy_lints/src/operators/assign_op_pattern.rs
@@ -1,5 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::msrvs::Msrv;
+use clippy_utils::qualify_min_const_fn::is_stable_const_fn;
 use clippy_utils::source::SpanRangeExt;
 use clippy_utils::ty::implements_trait;
 use clippy_utils::visitors::for_each_expr_without_closures;
@@ -20,7 +21,7 @@ pub(super) fn check<'tcx>(
     expr: &'tcx hir::Expr<'_>,
     assignee: &'tcx hir::Expr<'_>,
     e: &'tcx hir::Expr<'_>,
-    _msrv: Msrv,
+    msrv: Msrv,
 ) {
     if let hir::ExprKind::Binary(op, l, r) = &e.kind {
         let lint = |assignee: &hir::Expr<'_>, rhs: &hir::Expr<'_>| {
@@ -43,10 +44,28 @@ pub(super) fn check<'tcx>(
                     }
                 }
 
-                // Skip if the trait is not stable in const contexts
-                // FIXME: reintroduce a better check after this is merged back into Clippy
+                // Skip if the trait or the implementation is not stable in const contexts
                 if is_in_const_context(cx) {
-                    return;
+                    if cx
+                        .tcx
+                        .associated_item_def_ids(trait_id)
+                        .first()
+                        .is_none_or(|binop_id| !is_stable_const_fn(cx, *binop_id, msrv))
+                    {
+                        return;
+                    }
+
+                    let impls = cx.tcx.non_blanket_impls_for_ty(trait_id, rty).collect::<Vec<_>>();
+                    if impls.is_empty()
+                        || impls.into_iter().any(|impl_id| {
+                            cx.tcx
+                                .associated_item_def_ids(impl_id)
+                                .first()
+                                .is_none_or(|fn_id| !is_stable_const_fn(cx, *fn_id, msrv))
+                        })
+                    {
+                        return;
+                    }
                 }
 
                 span_lint_and_then(
diff --git a/tests/ui/assign_ops.fixed b/tests/ui/assign_ops.fixed
@@ -1,8 +1,9 @@
 #![allow(clippy::useless_vec)]
 #![warn(clippy::assign_op_pattern)]
-#![feature(const_trait_impl, const_ops)]
+#![feature(const_ops)]
+#![feature(const_trait_impl)]
 
-use core::num::Wrapping;
+use std::num::Wrapping;
 use std::ops::{Mul, MulAssign};
 
 fn main() {
@@ -82,6 +83,18 @@ mod issue14871 {
     pub trait Number: Copy + Add<Self, Output = Self> + AddAssign {
         const ZERO: Self;
         const ONE: Self;
+
+        fn non_constant(value: usize) -> Self {
+            let mut res = Self::ZERO;
+            let mut count = 0;
+            while count < value {
+                res += Self::ONE;
+                //~^ assign_op_pattern
+                count += 1;
+                //~^ assign_op_pattern
+            }
+            res
+        }
     }
 
     #[rustfmt::skip] // rustfmt doesn't understand the order of pub const on traits (yet)
@@ -91,16 +104,36 @@ mod issue14871 {
 
     impl<T> const NumberConstants for T
     where
-        T: Number + [const] core::ops::Add,
+        T: Number + [const] std::ops::Add,
     {
         fn constant(value: usize) -> Self {
             let mut res = Self::ZERO;
             let mut count = 0;
             while count < value {
                 res = res + Self::ONE;
                 count += 1;
+                //~^ assign_op_pattern
             }
             res
         }
     }
+
+    pub struct S;
+
+    impl const std::ops::Add for S {
+        type Output = S;
+        fn add(self, _rhs: S) -> S {
+            S
+        }
+    }
+
+    impl const std::ops::AddAssign for S {
+        fn add_assign(&mut self, rhs: S) {}
+    }
+
+    const fn do_add() {
+        let mut s = S;
+        s += S;
+        //~^ assign_op_pattern
+    }
 }
diff --git a/tests/ui/assign_ops.rs b/tests/ui/assign_ops.rs
@@ -1,8 +1,9 @@
 #![allow(clippy::useless_vec)]
 #![warn(clippy::assign_op_pattern)]
-#![feature(const_trait_impl, const_ops)]
+#![feature(const_ops)]
+#![feature(const_trait_impl)]
 
-use core::num::Wrapping;
+use std::num::Wrapping;
 use std::ops::{Mul, MulAssign};
 
 fn main() {
@@ -82,6 +83,18 @@ mod issue14871 {
     pub trait Number: Copy + Add<Self, Output = Self> + AddAssign {
         const ZERO: Self;
         const ONE: Self;
+
+        fn non_constant(value: usize) -> Self {
+            let mut res = Self::ZERO;
+            let mut count = 0;
+            while count < value {
+                res = res + Self::ONE;
+                //~^ assign_op_pattern
+                count = count + 1;
+                //~^ assign_op_pattern
+            }
+            res
+        }
     }
 
     #[rustfmt::skip] // rustfmt doesn't understand the order of pub const on traits (yet)
@@ -91,16 +104,36 @@ mod issue14871 {
 
     impl<T> const NumberConstants for T
     where
-        T: Number + [const] core::ops::Add,
+        T: Number + [const] std::ops::Add,
     {
         fn constant(value: usize) -> Self {
             let mut res = Self::ZERO;
             let mut count = 0;
             while count < value {
                 res = res + Self::ONE;
-                count += 1;
+                count = count + 1;
+                //~^ assign_op_pattern
             }
             res
         }
     }
+
+    pub struct S;
+
+    impl const std::ops::Add for S {
+        type Output = S;
+        fn add(self, _rhs: S) -> S {
+            S
+        }
+    }
+
+    impl const std::ops::AddAssign for S {
+        fn add_assign(&mut self, rhs: S) {}
+    }
+
+    const fn do_add() {
+        let mut s = S;
+        s = s + S;
+        //~^ assign_op_pattern
+    }
 }
diff --git a/tests/ui/assign_ops.stderr b/tests/ui/assign_ops.stderr
@@ -1,5 +1,5 @@
 error: manual implementation of an assign operation
-  --> tests/ui/assign_ops.rs:10:5
+  --> tests/ui/assign_ops.rs:11:5
    |
 LL |     a = a + 1;
    |     ^^^^^^^^^ help: replace it with: `a += 1`
@@ -8,70 +8,94 @@ LL |     a = a + 1;
    = help: to override `-D warnings` add `#[allow(clippy::assign_op_pattern)]`
 
 error: manual implementation of an assign operation
-  --> tests/ui/assign_ops.rs:12:5
+  --> tests/ui/assign_ops.rs:13:5
    |
 LL |     a = 1 + a;
    |     ^^^^^^^^^ help: replace it with: `a += 1`
 
 error: manual implementation of an assign operation
-  --> tests/ui/assign_ops.rs:14:5
+  --> tests/ui/assign_ops.rs:15:5
    |
 LL |     a = a - 1;
    |     ^^^^^^^^^ help: replace it with: `a -= 1`
 
 error: manual implementation of an assign operation
-  --> tests/ui/assign_ops.rs:16:5
+  --> tests/ui/assign_ops.rs:17:5
    |
 LL |     a = a * 99;
    |     ^^^^^^^^^^ help: replace it with: `a *= 99`
 
 error: manual implementation of an assign operation
-  --> tests/ui/assign_ops.rs:18:5
+  --> tests/ui/assign_ops.rs:19:5
    |
 LL |     a = 42 * a;
    |     ^^^^^^^^^^ help: replace it with: `a *= 42`
 
 error: manual implementation of an assign operation
-  --> tests/ui/assign_ops.rs:20:5
+  --> tests/ui/assign_ops.rs:21:5
    |
 LL |     a = a / 2;
    |     ^^^^^^^^^ help: replace it with: `a /= 2`
 
 error: manual implementation of an assign operation
-  --> tests/ui/assign_ops.rs:22:5
+  --> tests/ui/assign_ops.rs:23:5
    |
 LL |     a = a % 5;
    |     ^^^^^^^^^ help: replace it with: `a %= 5`
 
 error: manual implementation of an assign operation
-  --> tests/ui/assign_ops.rs:24:5
+  --> tests/ui/assign_ops.rs:25:5
    |
 LL |     a = a & 1;
    |     ^^^^^^^^^ help: replace it with: `a &= 1`
 
 error: manual implementation of an assign operation
-  --> tests/ui/assign_ops.rs:31:5
+  --> tests/ui/assign_ops.rs:32:5
    |
 LL |     s = s + "bla";
    |     ^^^^^^^^^^^^^ help: replace it with: `s += "bla"`
 
 error: manual implementation of an assign operation
-  --> tests/ui/assign_ops.rs:36:5
+  --> tests/ui/assign_ops.rs:37:5
    |
 LL |     a = a + Wrapping(1u32);
    |     ^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a += Wrapping(1u32)`
 
 error: manual implementation of an assign operation
-  --> tests/ui/assign_ops.rs:39:5
+  --> tests/ui/assign_ops.rs:40:5
    |
 LL |     v[0] = v[0] + v[1];
    |     ^^^^^^^^^^^^^^^^^^ help: replace it with: `v[0] += v[1]`
 
 error: manual implementation of an assign operation
-  --> tests/ui/assign_ops.rs:52:5
+  --> tests/ui/assign_ops.rs:53:5
    |
 LL |     buf = buf + cows.clone();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `buf += cows.clone()`
 
-error: aborting due to 12 previous errors
+error: manual implementation of an assign operation
+  --> tests/ui/assign_ops.rs:91:17
+   |
+LL |                 res = res + Self::ONE;
+   |                 ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `res += Self::ONE`
+
+error: manual implementation of an assign operation
+  --> tests/ui/assign_ops.rs:93:17
+   |
+LL |                 count = count + 1;
+   |                 ^^^^^^^^^^^^^^^^^ help: replace it with: `count += 1`
+
+error: manual implementation of an assign operation
+  --> tests/ui/assign_ops.rs:114:17
+   |
+LL |                 count = count + 1;
+   |                 ^^^^^^^^^^^^^^^^^ help: replace it with: `count += 1`
+
+error: manual implementation of an assign operation
+  --> tests/ui/assign_ops.rs:136:9
+   |
+LL |         s = s + S;
+   |         ^^^^^^^^^ help: replace it with: `s += S`
+
+error: aborting due to 16 previous errors
 
