Merge remote-tracking branch 'upstream/master' into rustup

Author: flip1995

clippy_lints/src/needless_pass_by_value.rs

@@ -40,9 +40,8 @@ declare_clippy_lint! {
     ///     assert_eq!(v.len(), 42);
     /// }
     /// ```
-    ///
+    /// should be
     /// ```rust
-    /// // should be
     /// fn foo(v: &[i32]) {
     ///     assert_eq!(v.len(), 42);
     /// }
@@ -159,26 +158,19 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue {
                 }
             }
 
+            //
             // * Exclude a type that is specifically bounded by `Borrow`.
             // * Exclude a type whose reference also fulfills its bound. (e.g., `std::convert::AsRef`,
             //   `serde::Serialize`)
             let (implements_borrow_trait, all_borrowable_trait) = {
-                let preds = preds
-                    .iter()
-                    .filter(|t| t.self_ty() == ty)
-                    .collect::<Vec<_>>();
+                let preds = preds.iter().filter(|t| t.self_ty() == ty).collect::<Vec<_>>();
 
                 (
                     preds.iter().any(|t| t.def_id() == borrow_trait),
                     !preds.is_empty() && {
                         let ty_empty_region = cx.tcx.mk_imm_ref(cx.tcx.lifetimes.re_root_empty, ty);
                         preds.iter().all(|t| {
-                            let ty_params = t
-                                .trait_ref
-                                .substs
-                                .iter()
-                                .skip(1)
-                                .collect::<Vec<_>>();
+                            let ty_params = t.trait_ref.substs.iter().skip(1).collect::<Vec<_>>();
                             implements_trait(cx, ty_empty_region, t.def_id(), &ty_params)
                         })
                     },

clippy_lints/src/suspicious_trait_impl.rs

@@ -64,26 +64,22 @@ impl<'tcx> LateLintPass<'tcx> for SuspiciousImpl {
                 | 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(
@@ -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 @@ impl<'tcx> Visitor<'tcx> for BinaryExprVisitor {
         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
     }

clippy_lints/src/utils/ast_utils.rs

@@ -147,9 +147,6 @@ fn run_ui_toml(config: &mut compiletest::Config) {
 }
 
 fn run_ui_cargo(config: &mut compiletest::Config) {
-    if cargo::is_rustc_test_suite() {
-        return;
-    }
     fn run_tests(
         config: &compiletest::Config,
         filter: &Option<String>,

tests/compile-test.rs

@@ -88,3 +88,33 @@ fn main() {}
 fn do_nothing(x: u32) -> u32 {
     x
 }
+
+struct MultipleBinops(u32);
+
+impl Add for MultipleBinops {
+    type Output = MultipleBinops;
+
+    // OK: multiple Binops in `add` impl
+    fn add(self, other: Self) -> Self::Output {
+        let mut result = self.0 + other.0;
+        if result >= u32::max_value() {
+            result -= u32::max_value();
+        }
+        MultipleBinops(result)
+    }
+}
+
+impl Mul for MultipleBinops {
+    type Output = MultipleBinops;
+
+    // OK: multiple Binops in `mul` impl
+    fn mul(self, other: Self) -> Self::Output {
+        let mut result: u32 = 0;
+        let size = std::cmp::max(self.0, other.0) as usize;
+        let mut v = vec![0; size + 1];
+        for i in 0..size + 1 {
+            result *= i as u32;
+        }
+        MultipleBinops(result)
+    }
+}