Merge commit '09bd400243ed6f7059fedc0c1623aae3792521d6' into clippyup

[rust.git] / src / tools / clippy / clippy_lints / src / suspicious_trait_impl.rs

diff --git a/src/tools/clippy/clippy_lints/src/suspicious_trait_impl.rs b/src/tools/clippy/clippy_lints/src/suspicious_trait_impl.rs
index 6d1d083fa8d40a145f45ecaabc13c8fc41f4c7b2..4e335a0222f2067c3fd33e05f9bed56c5fa91bff 100644 (file)
--- a/src/tools/clippy/clippy_lints/src/suspicious_trait_impl.rs
+++ b/src/tools/clippy/clippy_lints/src/suspicious_trait_impl.rs
@@ -64,26 +64,22 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
                 | hir::BinOpKind::Gt => return,
                 _ => {},
             }
-            // Check if the binary expression is part of another bi/unary expression
-            // or operator assignment as a child node
-            let mut parent_expr = cx.tcx.hir().get_parent_node(expr.hir_id);
-            while parent_expr != hir::CRATE_HIR_ID {
-                if let hir::Node::Expr(e) = cx.tcx.hir().get(parent_expr) {
-                    match e.kind {
-                        hir::ExprKind::Binary(..)
-                        | hir::ExprKind::Unary(hir::UnOp::UnNot | hir::UnOp::UnNeg, _)
-                        | hir::ExprKind::AssignOp(..) => return,
-                        _ => {},
+
+            // Check for more than one binary operation in the implemented function
+            // Linting when multiple operations are involved can result in false positives
+            if_chain! {
+                let parent_fn = cx.tcx.hir().get_parent_item(expr.hir_id);
+                if let hir::Node::ImplItem(impl_item) = cx.tcx.hir().get(parent_fn);
+                if let hir::ImplItemKind::Fn(_, body_id) = impl_item.kind;
+                let body = cx.tcx.hir().body(body_id);
+                let mut visitor = BinaryExprVisitor { nb_binops: 0 };
+
+                then {
+                    walk_expr(&mut visitor, &body.value);
+                    if visitor.nb_binops > 1 {
+                        return;
                     }
                 }
-                parent_expr = cx.tcx.hir().get_parent_node(parent_expr);
-            }
-            // as a parent node
-            let mut visitor = BinaryExprVisitor { in_binary_expr: false };
-            walk_expr(&mut visitor, expr);
-
-            if visitor.in_binary_expr {
-                return;
             }
 
             if let Some(impl_trait) = check_binop(
@@ -102,7 +98,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
                     cx,
                     SUSPICIOUS_ARITHMETIC_IMPL,
                     binop.span,
-                    &format!(r#"Suspicious use of binary operator in `{}` impl"#, impl_trait),
+                    &format!("suspicious use of binary operator in `{}` impl", impl_trait),
                 );
             }
 
@@ -139,7 +135,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
                     cx,
                     SUSPICIOUS_OP_ASSIGN_IMPL,
                     binop.span,
-                    &format!(r#"Suspicious use of binary operator in `{}` impl"#, impl_trait),
+                    &format!("suspicious use of binary operator in `{}` impl", impl_trait),
                 );
             }
         }
@@ -181,7 +177,7 @@ fn check_binop(
 }
 
 struct BinaryExprVisitor {
-    in_binary_expr: bool,
+    nb_binops: u32,
 }
 
 impl<'tcx> Visitor<'tcx> for BinaryExprVisitor {
@@ -191,12 +187,13 @@ fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
         match expr.kind {
             hir::ExprKind::Binary(..)
             | hir::ExprKind::Unary(hir::UnOp::UnNot | hir::UnOp::UnNeg, _)
-            | hir::ExprKind::AssignOp(..) => self.in_binary_expr = true,
+            | hir::ExprKind::AssignOp(..) => self.nb_binops += 1,
             _ => {},
         }
 
         walk_expr(self, expr);
     }
+
     fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
         NestedVisitorMap::None
     }