From 0fc61becb5f1b9a183311c8b053d996b8695a071 Mon Sep 17 00:00:00 2001 From: Ryan Wiedemann Date: Mon, 10 Aug 2020 07:30:55 -0600 Subject: [PATCH 1/5] 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. --- clippy_lints/src/suspicious_trait_impl.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index 4e335a0222f2..e026f27d9a7b 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -86,12 +86,18 @@ impl<'tcx> LateLintPass<'tcx> for SuspiciousImpl { cx, expr, binop.node, - &["Add", "Sub", "Mul", "Div"], + &["Add", "Sub", "Mul", "Div", "Rem", "BitAnd", "BitOr", "BitXor", "Shl", "Shr"], &[ hir::BinOpKind::Add, hir::BinOpKind::Sub, hir::BinOpKind::Mul, hir::BinOpKind::Div, + hir::BinOpKind::Rem, + hir::BinOpKind::BitAnd, + hir::BinOpKind::BitOr, + hir::BinOpKind::BitXor, + hir::BinOpKind::Shl, + hir::BinOpKind::Shr, ], ) { span_lint( From 616682deb72aa3760b1af3c701090e894f227ac7 Mon Sep 17 00:00:00 2001 From: Ryan Wiedemann Date: Mon, 10 Aug 2020 09:18:16 -0600 Subject: [PATCH 2/5] formatting --- clippy_lints/src/suspicious_trait_impl.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index e026f27d9a7b..596d4eb61417 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -86,7 +86,9 @@ impl<'tcx> LateLintPass<'tcx> for SuspiciousImpl { cx, expr, binop.node, - &["Add", "Sub", "Mul", "Div", "Rem", "BitAnd", "BitOr", "BitXor", "Shl", "Shr"], + &[ + "Add", "Sub", "Mul", "Div", "Rem", "BitAnd", "BitOr", "BitXor", "Shl", "Shr" + ], &[ hir::BinOpKind::Add, hir::BinOpKind::Sub, From c70581732de89a1b4f064818edeca9a18913ded2 Mon Sep 17 00:00:00 2001 From: Ryan Wiedemann Date: Mon, 10 Aug 2020 09:21:20 -0600 Subject: [PATCH 3/5] trailing comma Should have actually ran rustfmt on it, rather than attempting to fix it manually --- clippy_lints/src/suspicious_trait_impl.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index 596d4eb61417..3a688a7bbef3 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -87,7 +87,7 @@ impl<'tcx> LateLintPass<'tcx> for SuspiciousImpl { expr, binop.node, &[ - "Add", "Sub", "Mul", "Div", "Rem", "BitAnd", "BitOr", "BitXor", "Shl", "Shr" + "Add", "Sub", "Mul", "Div", "Rem", "BitAnd", "BitOr", "BitXor", "Shl", "Shr", ], &[ hir::BinOpKind::Add, From f4eeff99b6f2d5a01f7af1eae46e1b84525bf95a Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Wed, 12 Aug 2020 09:17:40 -0600 Subject: [PATCH 4/5] add tests for Rem, BitAnd, BitOr, BitXor, Shl, and Shr --- tests/ui/suspicious_arithmetic_impl.rs | 52 +++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/tests/ui/suspicious_arithmetic_impl.rs b/tests/ui/suspicious_arithmetic_impl.rs index 60c2f3ec9b65..5c280efac1a8 100644 --- a/tests/ui/suspicious_arithmetic_impl.rs +++ b/tests/ui/suspicious_arithmetic_impl.rs @@ -1,5 +1,7 @@ #![warn(clippy::suspicious_arithmetic_impl)] -use std::ops::{Add, AddAssign, BitOrAssign, Div, DivAssign, Mul, MulAssign, Sub}; +use std::ops::{ + Add, AddAssign, BitAnd, BitOr, BitOrAssign, BitXor, Div, DivAssign, Mul, MulAssign, Rem, Shl, Shr, Sub, +}; #[derive(Copy, Clone)] struct Foo(u32); @@ -61,6 +63,54 @@ impl Div for Foo { } } +impl Rem for Foo { + type Output = Foo; + + fn rem(self, other: Self) -> Self { + Foo(self.0 / other.0) + } +} + +impl BitAnd for Foo { + type Output = Foo; + + fn bitand(self, other: Self) -> Self { + Foo(self.0 | other.0) + } +} + +impl BitOr for Foo { + type Output = Foo; + + fn bitor(self, other: Self) -> Self { + Foo(self.0 ^ other.0) + } +} + +impl BitXor for Foo { + type Output = Foo; + + fn bitxor(self, other: Self) -> Self { + Foo(self.0 & other.0) + } +} + +impl Shl for Foo { + type Output = Foo; + + fn shl(self, other: Self) -> Self { + Foo(self.0 >> other.0) + } +} + +impl Shr for Foo { + type Output = Foo; + + fn shr(self, other: Self) -> Self { + Foo(self.0 << other.0) + } +} + struct Bar(i32); impl Add for Bar { From 7bd7a46331fbaa8b8ebbaacf178c988498df9f13 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Wed, 12 Aug 2020 10:49:12 -0600 Subject: [PATCH 5/5] run tests/ui/update-references.sh to update 'suspicious_arithmetic_impl.rs' --- tests/ui/suspicious_arithmetic_impl.stderr | 44 ++++++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/tests/ui/suspicious_arithmetic_impl.stderr b/tests/ui/suspicious_arithmetic_impl.stderr index 23d47e3f1ff0..388fc7400820 100644 --- a/tests/ui/suspicious_arithmetic_impl.stderr +++ b/tests/ui/suspicious_arithmetic_impl.stderr @@ -1,5 +1,5 @@ error: suspicious use of binary operator in `Add` impl - --> $DIR/suspicious_arithmetic_impl.rs:11:20 + --> $DIR/suspicious_arithmetic_impl.rs:13:20 | LL | Foo(self.0 - other.0) | ^ @@ -7,7 +7,7 @@ LL | Foo(self.0 - other.0) = note: `-D clippy::suspicious-arithmetic-impl` implied by `-D warnings` error: suspicious use of binary operator in `AddAssign` impl - --> $DIR/suspicious_arithmetic_impl.rs:17:23 + --> $DIR/suspicious_arithmetic_impl.rs:19:23 | LL | *self = *self - other; | ^ @@ -15,10 +15,46 @@ LL | *self = *self - other; = note: `#[deny(clippy::suspicious_op_assign_impl)]` on by default error: suspicious use of binary operator in `MulAssign` impl - --> $DIR/suspicious_arithmetic_impl.rs:30:16 + --> $DIR/suspicious_arithmetic_impl.rs:32:16 | LL | self.0 /= other.0; | ^^ -error: aborting due to 3 previous errors +error: suspicious use of binary operator in `Rem` impl + --> $DIR/suspicious_arithmetic_impl.rs:70:20 + | +LL | Foo(self.0 / other.0) + | ^ + +error: suspicious use of binary operator in `BitAnd` impl + --> $DIR/suspicious_arithmetic_impl.rs:78:20 + | +LL | Foo(self.0 | other.0) + | ^ + +error: suspicious use of binary operator in `BitOr` impl + --> $DIR/suspicious_arithmetic_impl.rs:86:20 + | +LL | Foo(self.0 ^ other.0) + | ^ + +error: suspicious use of binary operator in `BitXor` impl + --> $DIR/suspicious_arithmetic_impl.rs:94:20 + | +LL | Foo(self.0 & other.0) + | ^ + +error: suspicious use of binary operator in `Shl` impl + --> $DIR/suspicious_arithmetic_impl.rs:102:20 + | +LL | Foo(self.0 >> other.0) + | ^^ + +error: suspicious use of binary operator in `Shr` impl + --> $DIR/suspicious_arithmetic_impl.rs:110:20 + | +LL | Foo(self.0 << other.0) + | ^^ + +error: aborting due to 9 previous errors