diff options
| author | Samuel Tardieu <sam@rfc1149.net> | 2025-07-16 15:37:55 +0200 |
|---|---|---|
| committer | Samuel Tardieu <sam@rfc1149.net> | 2025-08-22 15:33:16 +0200 |
| commit | 86beeccb6519032d1bc067a05f87d797162d137d (patch) | |
| tree | 56f63942d666af1beb53837f8dfb85c1bef71635 | |
| parent | 877967959ae8da9814df4f2614971f4d784bf53f (diff) | |
| download | rust-86beeccb6519032d1bc067a05f87d797162d137d.tar.gz rust-86beeccb6519032d1bc067a05f87d797162d137d.zip | |
Better check for `assign_op_pattern` in `const` context
`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.
| -rw-r--r-- | clippy_lints/src/operators/assign_op_pattern.rs | 27 | ||||
| -rw-r--r-- | tests/ui/assign_ops.fixed | 39 | ||||
| -rw-r--r-- | tests/ui/assign_ops.rs | 41 | ||||
| -rw-r--r-- | tests/ui/assign_ops.stderr | 50 |
4 files changed, 133 insertions, 24 deletions
diff --git a/clippy_lints/src/operators/assign_op_pattern.rs b/clippy_lints/src/operators/assign_op_pattern.rs index 7317c62df7f..2d303e40bd1 100644 --- 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 index 3754b9dfe74..2046d089d6a 100644 --- 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,7 +104,7 @@ 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; @@ -99,8 +112,28 @@ mod issue14871 { 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 index 0b878d4f490..f83a40f5547 100644 --- 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 index c5e698b3ee1..a4fca04893c 100644 --- 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 |
