Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions clippy_lints/src/operators/assign_op_pattern.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<'_>| {
Expand All @@ -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(
Expand Down
39 changes: 36 additions & 3 deletions tests/ui/assign_ops.fixed
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
}
41 changes: 37 additions & 4 deletions tests/ui/assign_ops.rs
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
}
50 changes: 37 additions & 13 deletions tests/ui/assign_ops.stderr
Original file line number Diff line number Diff line change
@@ -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`
Expand All @@ -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

Loading