Merge pull request #1318 from oli-obk/op_assign_false_positive

don't lint on x = x + y inside an AddAssign impl

diff --git a/Cargo.toml b/Cargo.toml
index 5167218ff4a84cb0f0aaad4d3b79ac5f8ba407b3..0b1d7fd8671b651bed7f361d2f189915cd681bcb 100644 (file)
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -29,7 +29,7 @@ clippy_lints = { version = "0.0.104", path = "clippy_lints" }
 # end automatic update
 
 [dev-dependencies]
-compiletest_rs = "0.2.1"
+compiletest_rs = "0.2.5"
 lazy_static = "0.1.15"
 regex = "0.1.71"
 rustc-serialize = "0.3"

diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs
index ef6898cef885611182d3a68c03f52b624593271f..5f5cddc64714b73c99add74553ad15a9389bd478 100644 (file)
--- a/clippy_lints/src/assign_ops.rs
+++ b/clippy_lints/src/assign_ops.rs
@@ -1,5 +1,6 @@
 use rustc::hir;
 use rustc::lint::*;
+use syntax::ast;
 use utils::{span_lint_and_then, snippet_opt, SpanlessEq, get_trait_def_id, implements_trait};
 use utils::{higher, sugg};
 
@@ -135,6 +136,19 @@ macro_rules! ops {
                                         } else {
                                             return; // useless if the trait doesn't exist
                                         };
+                                        // check that we are not inside an `impl AssignOp` of this exact operation
+                                        let parent_fn = cx.tcx.map.get_parent(e.id);
+                                        let parent_impl = cx.tcx.map.get_parent(parent_fn);
+                                        // the crate node is the only one that is not in the map
+                                        if parent_impl != ast::CRATE_NODE_ID {
+                                            if let hir::map::Node::NodeItem(item) = cx.tcx.map.get(parent_impl) {
+                                                if let hir::Item_::ItemImpl(_, _, _, Some(ref trait_ref), _, _) = item.node {
+                                                    if trait_ref.path.def.def_id() == trait_id {
+                                                        return;
+                                                    }
+                                                }
+                                            }
+                                        }
                                         implements_trait($cx, $ty, trait_id, vec![$rty])
                                     },)*
                                     _ => false,

diff --git a/tests/compile-fail/assign_ops2.rs b/tests/compile-fail/assign_ops2.rs
index 1537232bf1828dd326feeba8fb40d988474e5b13..e8549c01bc93268c7d4ff431a017150f3b14249b 100644 (file)
--- a/tests/compile-fail/assign_ops2.rs
+++ b/tests/compile-fail/assign_ops2.rs
@@ -34,3 +34,24 @@ fn main() {
     a %= 42 % a;
     a <<= 6 << a;
 }
+
+// check that we don't lint on op assign impls, because that's just the way to impl them
+
+use std::ops::{Mul, MulAssign};
+
+#[derive(Copy, Clone, Debug, PartialEq)]
+pub struct Wrap(i64);
+
+impl Mul<i64> for Wrap {
+    type Output = Self;
+
+    fn mul(self, rhs: i64) -> Self {
+        Wrap(self.0 * rhs)
+    }
+}
+
+impl MulAssign<i64> for Wrap {
+    fn mul_assign(&mut self, rhs: i64) {
+        *self = *self * rhs
+    }
+}