Auto merge of #4575 - Manishearth:suggestions, r=oli-obk

Make more tests rustfixable

Burning through #3630

changelog: Improve suggestions for many lints in preparation for `cargo fix --clippy`

r? @phansch @yaahc

Cargo.toml

@@ -46,7 +46,7 @@ rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"}
 
 [dev-dependencies]
 cargo_metadata = "0.8.0"
-compiletest_rs = { version = "0.3.22", features = ["tmp"] }
+compiletest_rs = { version = "0.3.23", features = ["tmp"] }
 lazy_static = "1.0"
 clippy-mini-macro-test = { version = "0.2", path = "mini-macro" }
 serde = { version = "1.0", features = ["derive"] }

clippy_lints/src/cognitive_complexity.rs

@@ -112,8 +112,7 @@ impl<'tcx> Visitor<'tcx> for CCHelper {
         walk_expr(self, e);
         match e.node {
             ExprKind::Match(_, ref arms, _) => {
-                let arms_n: u64 = arms.iter().map(|arm| arm.pats.len() as u64).sum();
-                if arms_n > 1 {
+                if arms.len() > 1 {
                     self.cc += 1;
                 }
                 self.cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64;

clippy_lints/src/copies.rs

@@ -193,7 +193,7 @@ fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr) {
             (min_index..=max_index).all(|index| arms[index].guard.is_none()) &&
                 SpanlessEq::new(cx).eq_expr(&lhs.body, &rhs.body) &&
                 // all patterns should have the same bindings
-                same_bindings(cx, &bindings(cx, &lhs.pats[0]), &bindings(cx, &rhs.pats[0]))
+                same_bindings(cx, &bindings(cx, &lhs.pat), &bindings(cx, &rhs.pat))
         };
 
         let indexed_arms: Vec<(usize, &Arm)> = arms.iter().enumerate().collect();
@@ -213,27 +213,22 @@ fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr) {
                     // span for the whole pattern, the suggestion is only shown when there is only
                     // one pattern. The user should know about `|` if they are already using it…
 
-                    if i.pats.len() == 1 && j.pats.len() == 1 {
-                        let lhs = snippet(cx, i.pats[0].span, "<pat1>");
-                        let rhs = snippet(cx, j.pats[0].span, "<pat2>");
-
-                        if let PatKind::Wild = j.pats[0].node {
-                            // if the last arm is _, then i could be integrated into _
-                            // note that i.pats[0] cannot be _, because that would mean that we're
-                            // hiding all the subsequent arms, and rust won't compile
-                            db.span_note(
-                                i.body.span,
-                                &format!(
-                                    "`{}` has the same arm body as the `_` wildcard, consider removing it`",
-                                    lhs
-                                ),
-                            );
-                        } else {
-                            db.span_help(
-                                i.pats[0].span,
-                                &format!("consider refactoring into `{} | {}`", lhs, rhs),
-                            );
-                        }
+                    let lhs = snippet(cx, i.pat.span, "<pat1>");
+                    let rhs = snippet(cx, j.pat.span, "<pat2>");
+
+                    if let PatKind::Wild = j.pat.node {
+                        // if the last arm is _, then i could be integrated into _
+                        // note that i.pat cannot be _, because that would mean that we're
+                        // hiding all the subsequent arms, and rust won't compile
+                        db.span_note(
+                            i.body.span,
+                            &format!(
+                                "`{}` has the same arm body as the `_` wildcard, consider removing it`",
+                                lhs
+                            ),
+                        );
+                    } else {
+                        db.span_help(i.pat.span, &format!("consider refactoring into `{} | {}`", lhs, rhs));
                     }
                 },
             );

clippy_lints/src/entry.rs

@@ -1,5 +1,6 @@
 use crate::utils::SpanlessEq;
-use crate::utils::{get_item_name, higher, match_type, paths, snippet, snippet_opt, span_lint_and_then, walk_ptrs_ty};
+use crate::utils::{get_item_name, higher, match_type, paths, snippet, snippet_opt};
+use crate::utils::{snippet_with_applicability, span_lint_and_then, walk_ptrs_ty};
 use if_chain::if_chain;
 use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
 use rustc::hir::*;
@@ -64,6 +65,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for HashMapPass {
                             } else {
                                 true
                             }
+                        // XXXManishearth we can also check for if/else blocks containing `None`.
                     };
 
                     let mut visitor = InsertVisitor {
@@ -145,10 +147,11 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> {
                 span_lint_and_then(self.cx, MAP_ENTRY, self.span,
                                    &format!("usage of `contains_key` followed by `insert` on a `{}`", self.ty), |db| {
                     if self.sole_expr {
-                        let help = format!("{}.entry({}).or_insert({})",
-                                           snippet(self.cx, self.map.span, "map"),
-                                           snippet(self.cx, params[1].span, ".."),
-                                           snippet(self.cx, params[2].span, ".."));
+                        let mut app = Applicability::MachineApplicable;
+                        let help = format!("{}.entry({}).or_insert({});",
+                                           snippet_with_applicability(self.cx, self.map.span, "map", &mut app),
+                                           snippet_with_applicability(self.cx, params[1].span, "..", &mut app),
+                                           snippet_with_applicability(self.cx, params[2].span, "..", &mut app));
 
                         db.span_suggestion(
                             self.span,
@@ -158,15 +161,13 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> {
                         );
                     }
                     else {
-                        let help = format!("{}.entry({})",
+                        let help = format!("consider using `{}.entry({})`",
                                            snippet(self.cx, self.map.span, "map"),
                                            snippet(self.cx, params[1].span, ".."));
 
-                        db.span_suggestion(
+                        db.span_label(
                             self.span,
-                            "consider using",
-                            help,
-                            Applicability::MachineApplicable, // snippet
+                            &help,
                         );
                     }
                 });

clippy_lints/src/format.rs

@@ -84,9 +84,8 @@ fn on_argumentv1_new<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, arm
         if let ExprKind::Path(ref qpath) = args[1].node;
         if let Some(did) = resolve_node(cx, qpath, args[1].hir_id).opt_def_id();
         if match_def_path(cx, did, &paths::DISPLAY_FMT_METHOD);
-        if arms[0].pats.len() == 1;
         // check `(arg0,)` in match block
-        if let PatKind::Tuple(ref pats, None) = arms[0].pats[0].node;
+        if let PatKind::Tuple(ref pats, None) = arms[0].pat.node;
         if pats.len() == 1;
         then {
             let ty = walk_ptrs_ty(cx.tables.pat_ty(&pats[0]));

clippy_lints/src/infallible_destructuring_match.rs

@@ -47,8 +47,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InfallibleDestructingMatch {
         if_chain! {
             if let Some(ref expr) = local.init;
             if let ExprKind::Match(ref target, ref arms, MatchSource::Normal) = expr.node;
-            if arms.len() == 1 && arms[0].pats.len() == 1 && arms[0].guard.is_none();
-            if let PatKind::TupleStruct(QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pats[0].node;
+            if arms.len() == 1 && arms[0].guard.is_none();
+            if let PatKind::TupleStruct(QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pat.node;
             if args.len() == 1;
             if let Some(arg) = get_arg_name(&args[0]);
             let body = remove_blocks(&arms[0].body);

clippy_lints/src/loops.rs

@@ -517,9 +517,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops {
                     match *source {
                         MatchSource::Normal | MatchSource::IfLetDesugar { .. } => {
                             if arms.len() == 2
-                                && arms[0].pats.len() == 1
                                 && arms[0].guard.is_none()
-                                && arms[1].pats.len() == 1
                                 && arms[1].guard.is_none()
                                 && is_simple_break_expr(&arms[1].body)
                             {
@@ -541,7 +539,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops {
                                     "try",
                                     format!(
                                         "while let {} = {} {{ .. }}",
-                                        snippet_with_applicability(cx, arms[0].pats[0].span, "..", &mut applicability),
+                                        snippet_with_applicability(cx, arms[0].pat.span, "..", &mut applicability),
                                         snippet_with_applicability(cx, matchexpr.span, "..", &mut applicability),
                                     ),
                                     applicability,
@@ -554,7 +552,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops {
             }
         }
         if let ExprKind::Match(ref match_expr, ref arms, MatchSource::WhileLetDesugar) = expr.node {
-            let pat = &arms[0].pats[0].node;
+            let pat = &arms[0].pat.node;
             if let (
                 &PatKind::TupleStruct(ref qpath, ref pat_args, _),
                 &ExprKind::MethodCall(ref method_path, _, ref method_args),
@@ -2429,12 +2427,17 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr, cx: &LateContext<'a, 'tcx>
                     let contains_arg = snippet(cx, args[1].span, "??");
                     let span = shorten_needless_collect_span(expr);
                     span_lint_and_then(cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, |db| {
+                        let (arg, pred) = if contains_arg.starts_with('&') {
+                            ("x", &contains_arg[1..])
+                        } else {
+                            ("&x", &*contains_arg)
+                        };
                         db.span_suggestion(
                             span,
                             "replace with",
                             format!(
-                                ".any(|&x| x == {})",
-                                if contains_arg.starts_with('&') { &contains_arg[1..] } else { &contains_arg }
+                                ".any(|{}| x == {})",
+                                arg, pred
                             ),
                             Applicability::MachineApplicable,
                         );

clippy_lints/src/map_unit_fn.rs

@@ -217,15 +217,15 @@ fn lint_map_unit_fn(cx: &LateContext<'_, '_>, stmt: &hir::Stmt, expr: &hir::Expr
     if is_unit_function(cx, fn_arg) {
         let msg = suggestion_msg("function", map_type);
         let suggestion = format!(
-            "if let {0}({1}) = {2} {{ {3}(...) }}",
+            "if let {0}({binding}) = {1} {{ {2}({binding}) }}",
             variant,
-            let_binding_name(cx, var_arg),
             snippet(cx, var_arg.span, "_"),
-            snippet(cx, fn_arg.span, "_")
+            snippet(cx, fn_arg.span, "_"),
+            binding = let_binding_name(cx, var_arg)
         );
 
         span_lint_and_then(cx, lint, expr.span, &msg, |db| {
-            db.span_suggestion(stmt.span, "try this", suggestion, Applicability::Unspecified);
+            db.span_suggestion(stmt.span, "try this", suggestion, Applicability::MachineApplicable);
         });
     } else if let Some((binding, closure_expr)) = unit_closure(cx, fn_arg) {
         let msg = suggestion_msg("closure", map_type);
@@ -250,9 +250,9 @@ fn lint_map_unit_fn(cx: &LateContext<'_, '_>, stmt: &hir::Stmt, expr: &hir::Expr
                     "if let {0}({1}) = {2} {{ ... }}",
                     variant,
                     snippet(cx, binding.pat.span, "_"),
-                    snippet(cx, var_arg.span, "_")
+                    snippet(cx, var_arg.span, "_"),
                 );
-                db.span_suggestion(stmt.span, "try this", suggestion, Applicability::Unspecified);
+                db.span_suggestion(stmt.span, "try this", suggestion, Applicability::HasPlaceholders);
             }
         });
     }