From 061b2f3057515ecb49e147fa6f54d07fd988005c Mon Sep 17 00:00:00 2001 From: Josh Holmer Date: Fri, 31 Aug 2018 18:26:04 -0400 Subject: [PATCH] Apply applicability --- clippy_lints/src/loops.rs | 60 ++++++++++++++++++-------------- tests/ui/needless_collect.stderr | 16 ++++----- 2 files changed, 41 insertions(+), 35 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 3b886875d..0c737d948 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -14,6 +14,7 @@ use rustc::middle::mem_categorization::Categorization; use rustc::middle::mem_categorization::cmt_; use rustc::ty::{self, Ty}; use rustc::ty::subst::Subst; +use rustc_errors::Applicability; use std::collections::{HashMap, HashSet}; use std::iter::{once, Iterator}; use syntax::ast; @@ -2267,6 +2268,8 @@ impl<'a, 'tcx> Visitor<'tcx> for VarCollectorVisitor<'a, 'tcx> { } } +const NEEDLESS_COLLECT_MSG: &str = "avoid using `collect()` when not needed"; + fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr, cx: &LateContext<'a, 'tcx>) { if let ExprKind::MethodCall(ref method, _, ref args) = expr.node { if let ExprKind::MethodCall(ref chain_method, _, _) = args[0].node { @@ -2281,38 +2284,41 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr, cx: &LateContext<'a, 'tcx> match_type(cx, ty, &paths::BTREEMAP) || match_type(cx, ty, &paths::HASHMAP) { if method.ident.name == "len" { - span_lint_and_sugg( - cx, - NEEDLESS_COLLECT, - shorten_needless_collect_span(expr), - "you are collecting an iterator to check its length", - "consider replacing with", - ".count()".to_string(), - ); + let span = shorten_needless_collect_span(expr); + span_lint_and_then(cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, |db| { + db.span_suggestion_with_applicability( + span, + "replace with", + ".count()".to_string(), + Applicability::MachineApplicable, + ); + }); } if method.ident.name == "is_empty" { - span_lint_and_sugg( - cx, - NEEDLESS_COLLECT, - shorten_needless_collect_span(expr), - "you are collecting an iterator to check if it is empty", - "consider replacing with", - ".next().is_none()".to_string(), - ); + let span = shorten_needless_collect_span(expr); + span_lint_and_then(cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, |db| { + db.span_suggestion_with_applicability( + span, + "replace with", + ".next().is_none()".to_string(), + Applicability::MachineApplicable, + ); + }); } if method.ident.name == "contains" { let contains_arg = snippet(cx, args[1].span, "??"); - span_lint_and_sugg( - cx, - NEEDLESS_COLLECT, - shorten_needless_collect_span(expr), - "you are collecting an iterator to check if contains an element", - "consider replacing with", - format!( - ".any(|&x| x == {})", - if contains_arg.starts_with('&') { &contains_arg[1..] } else { &contains_arg } - ), - ); + let span = shorten_needless_collect_span(expr); + span_lint_and_then(cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, |db| { + db.span_suggestion_with_applicability( + span, + "replace with", + format!( + ".any(|&x| x == {})", + if contains_arg.starts_with('&') { &contains_arg[1..] } else { &contains_arg } + ), + Applicability::MachineApplicable, + ); + }); } } } diff --git a/tests/ui/needless_collect.stderr b/tests/ui/needless_collect.stderr index 1eca733ea..c2dd0879f 100644 --- a/tests/ui/needless_collect.stderr +++ b/tests/ui/needless_collect.stderr @@ -1,28 +1,28 @@ -error: you are collecting an iterator to check its length +error: avoid using `collect()` when not needed --> $DIR/needless_collect.rs:7:28 | 7 | let len = sample.iter().collect::>().len(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider replacing with: `.count()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.count()` | = note: `-D needless-collect` implied by `-D warnings` -error: you are collecting an iterator to check if it is empty +error: avoid using `collect()` when not needed --> $DIR/needless_collect.rs:8:21 | 8 | if sample.iter().collect::>().is_empty() { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider replacing with: `.next().is_none()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.next().is_none()` -error: you are collecting an iterator to check if contains an element +error: avoid using `collect()` when not needed --> $DIR/needless_collect.rs:11:27 | 11 | sample.iter().cloned().collect::>().contains(&1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider replacing with: `.any(|&x| x == 1)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.any(|&x| x == 1)` -error: you are collecting an iterator to check its length +error: avoid using `collect()` when not needed --> $DIR/needless_collect.rs:12:34 | 12 | sample.iter().map(|x| (x, x)).collect::>().len(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider replacing with: `.count()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.count()` error: aborting due to 4 previous errors