Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assign_op_pattern misleading on wrongly implemented assign impl. #2268

Closed
letheed opened this issue Dec 11, 2017 · 3 comments
Closed

assign_op_pattern misleading on wrongly implemented assign impl. #2268

letheed opened this issue Dec 11, 2017 · 3 comments
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. good-first-issue These issues are a good way to get started with Clippy L-correctness Lint: Belongs in the correctness lint group T-middle Type: Probably requires verifiying types

Comments

@letheed
Copy link

letheed commented Dec 11, 2017

impl SubAssign for Foo {
    fn sub_assign(&mut self, rhs: Self) {
        *self = *self + rhs; // Copy-paste facepalm
    }
}

Triggers assign_op_pattern, which suggest replace it with: *self += rhs.

Mistake caught but left me wondering for a minute. Had to find this to figure out something else must be going on.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 11, 2017

duplicate of #1239

@oli-obk oli-obk closed this as completed Dec 11, 2017
@oli-obk oli-obk reopened this Dec 11, 2017
@oli-obk
Copy link
Contributor

oli-obk commented Dec 11, 2017

Oh, this is SubAssign not AddAssign, nevermind me

@oli-obk oli-obk added L-correctness Lint: Belongs in the correctness lint group E-medium Call for participation: Medium difficulty level problem and requires some initial experience. good-first-issue These issues are a good way to get started with Clippy A-lint Area: New lints T-middle Type: Probably requires verifiying types labels Dec 11, 2017
@Ryan1729
Copy link
Contributor

Given that #2458, which addresses this, landed some time ago, and that #5820 which reduces the false positive rate on the suspicious_arithmetic_impl lint, recently landed, can this issue be closed?

#2458 does mention that the suspicious_arithmetic_impl lint only covers the Add, Sub, Mul and Div, traits, and this is still true currently. Do we want to extend this lint to cover the rest of the binary operations that can be overloaded? Specifically those are BitAnd, BitOr, BitXor, Rem, Shl, and Shr.

@oli-obk oli-obk closed this as completed Aug 10, 2020
Ryan1729 added a commit to Ryan1729/rust-clippy that referenced this issue Aug 10, 2020
In rust-lang#2268 I idly mused that the other user-overloadable operations could be added to this lint. Knowing that the lint was arguably incomplete was gnawing at the back of my mind, so I figured that I might as well make this PR, particularly given the change needed was so small.
Ryan1729 added a commit to Ryan1729/rust-clippy that referenced this issue Aug 12, 2020
In rust-lang#2268 I idly mused that the other user-overloadable operations could be added to this lint. Knowing that the lint was arguably incomplete was gnawing at the back of my mind, so I figured that I might as well make this PR, particularly given the change needed was so small.
Ryan1729 added a commit to Ryan1729/rust-clippy that referenced this issue Aug 12, 2020
In rust-lang#2268 I idly mused that the other user-overloadable operations could be added to this lint. Knowing that the lint was arguably incomplete was gnawing at the back of my mind, so I figured that I might as well make this PR, particularly given the change needed was so small.
bors added a commit that referenced this issue Aug 12, 2020
Add the other overloadable operations to suspicious_arithmetic_impl

In #2268 I idly mused that the other user-overloadable operations could be added to this lint. Knowing that the lint was arguably incomplete was gnawing at the back of my mind, so I figured that I might as well make this PR, particularly given the change needed was so small.

changelog: Start warning on suspicious implementations of the `BitAnd`, `BitOr`, `BitXor`, `Rem`, `Shl`, and `Shr` traits.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. good-first-issue These issues are a good way to get started with Clippy L-correctness Lint: Belongs in the correctness lint group T-middle Type: Probably requires verifiying types
Projects
None yet
Development

No branches or pull requests

4 participants