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

suspicious_arithmetic_impl and OpAssign #5512

Closed
l0calh05t opened this issue Apr 23, 2020 · 4 comments
Closed

suspicious_arithmetic_impl and OpAssign #5512

l0calh05t opened this issue Apr 23, 2020 · 4 comments
Labels
C-bug Category: Clippy is not doing the correct thing C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages

Comments

@l0calh05t
Copy link

l0calh05t commented Apr 23, 2020

While the suspicious_arithmetic_impl is a bit overeager itself (see #3215), it also fails to detect the type of mistake it is supposed to detect when an Op-trait is implemented in terms of the corresponding OpAssign-trait. In the example below, Add has "accidentally" been implemented in terms of *=/MulAssign, but clippy 0.0.212 (4ee1206 2020-02-01) does not issue a warning:

#[derive(Clone, Debug)]
struct Foo(f32);

impl std::ops::Mul<&Foo> for &Foo {
	type Output = Foo;
	fn mul(self, other: &Foo) -> Foo {
		let mut ret = self.clone();
		ret.0 *= other.0;
		ret
	}
}

impl std::ops::Add<&Foo> for &Foo {
	type Output = Foo;
	fn add(self, other: &Foo) -> Foo {
		let mut ret = self.clone();
		ret.0 *= other.0; // oops, but no warning
		ret
	}
}

fn main() {
	println!("{:?}", &Foo(2.0) * &Foo(3.0));
	println!("{:?}", &Foo(2.0) + &Foo(3.0));
}
@flip1995 flip1995 added C-bug Category: Clippy is not doing the correct thing C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages labels Apr 24, 2020
@ThibsG
Copy link
Contributor

ThibsG commented Jul 7, 2020

This lint seems to be correctly triggered or am I missing something?
Playground

@l0calh05t
Copy link
Author

@ThibsG Doesn't trigger on your playground link either. Tried both debug and release and stable and nightly.

@ThibsG
Copy link
Contributor

ThibsG commented Jul 8, 2020

But if you do "Tools" then "Clippy" it triggers right?

@l0calh05t
Copy link
Author

l0calh05t commented Jul 8, 2020

It does, yes. Oddly, it does for me locally now too... closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages
Projects
None yet
Development

No branches or pull requests

3 participants