needless_collect: For BTreeMap and HashMap lint only is_empty

- `len` might produce different results than `count`
- they don't have `contain` but `contains_key` method
This commit is contained in:
Mateusz Gacek 2021-05-05 12:08:24 -07:00
parent 182a1853c3
commit a21607d9b5
4 changed files with 63 additions and 33 deletions

View file

@ -10,7 +10,6 @@ use rustc_hir::intravisit::{walk_block, walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{Block, Expr, ExprKind, GenericArg, GenericArgs, HirId, Local, Pat, PatKind, QPath, StmtKind, Ty}; use rustc_hir::{Block, Expr, ExprKind, GenericArg, GenericArgs, HirId, Local, Pat, PatKind, QPath, StmtKind, Ty};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::hir::map::Map; use rustc_middle::hir::map::Map;
use rustc_span::symbol::{sym, Ident}; use rustc_span::symbol::{sym, Ident};
use rustc_span::{MultiSpan, Span}; use rustc_span::{MultiSpan, Span};
@ -28,13 +27,14 @@ fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCont
if let Some(generic_args) = chain_method.args; if let Some(generic_args) = chain_method.args;
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0); if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
if let Some(ty) = cx.typeck_results().node_type_opt(ty.hir_id); if let Some(ty) = cx.typeck_results().node_type_opt(ty.hir_id);
if is_type_diagnostic_item(cx, ty, sym::vec_type) then {
|| is_type_diagnostic_item(cx, ty, sym::vecdeque_type) let is_empty_sugg = Some("next().is_none()".to_string());
|| match_type(cx, ty, &paths::BTREEMAP) let method_name = &*method.ident.name.as_str();
|| is_type_diagnostic_item(cx, ty, sym::hashmap_type); let sugg = if is_type_diagnostic_item(cx, ty, sym::vec_type) ||
if let Some(sugg) = match &*method.ident.name.as_str() { is_type_diagnostic_item(cx, ty, sym::vecdeque_type) {
match method_name {
"len" => Some("count()".to_string()), "len" => Some("count()".to_string()),
"is_empty" => Some("next().is_none()".to_string()), "is_empty" => is_empty_sugg,
"contains" => { "contains" => {
let contains_arg = snippet(cx, args[1].span, "??"); let contains_arg = snippet(cx, args[1].span, "??");
let (arg, pred) = contains_arg let (arg, pred) = contains_arg
@ -43,8 +43,19 @@ fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCont
Some(format!("any(|{}| x == {})", arg, pred)) Some(format!("any(|{}| x == {})", arg, pred))
} }
_ => None, _ => None,
}
}
else if match_type(cx, ty, &paths::BTREEMAP) ||
is_type_diagnostic_item(cx, ty, sym::hashmap_type) {
match method_name {
"is_empty" => is_empty_sugg,
_ => None,
}
}
else {
None
}; };
then { if let Some(sugg) = sugg {
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
NEEDLESS_COLLECT, NEEDLESS_COLLECT,
@ -57,6 +68,7 @@ fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCont
} }
} }
} }
}
fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) { fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
fn get_hir_id<'tcx>(ty: Option<&Ty<'tcx>>, method_args: Option<&GenericArgs<'tcx>>) -> Option<HirId> { fn get_hir_id<'tcx>(ty: Option<&Ty<'tcx>>, method_args: Option<&GenericArgs<'tcx>>) -> Option<HirId> {

View file

@ -2,7 +2,7 @@
#![allow(unused, clippy::suspicious_map, clippy::iter_count)] #![allow(unused, clippy::suspicious_map, clippy::iter_count)]
use std::collections::{BTreeSet, HashMap, HashSet}; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
#[warn(clippy::needless_collect)] #[warn(clippy::needless_collect)]
#[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)] #[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)]
@ -13,7 +13,13 @@ fn main() {
// Empty // Empty
} }
sample.iter().cloned().any(|x| x == 1); sample.iter().cloned().any(|x| x == 1);
sample.iter().map(|x| (x, x)).count(); // #7164 HashMap's and BTreeMap's `len` usage should not be linted
sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().len();
sample.iter().map(|x| (x, x)).collect::<BTreeMap<_, _>>().len();
sample.iter().map(|x| (x, x)).next().is_none();
sample.iter().map(|x| (x, x)).next().is_none();
// Notice the `HashSet`--this should not be linted // Notice the `HashSet`--this should not be linted
sample.iter().collect::<HashSet<_>>().len(); sample.iter().collect::<HashSet<_>>().len();
// Neither should this // Neither should this

View file

@ -2,7 +2,7 @@
#![allow(unused, clippy::suspicious_map, clippy::iter_count)] #![allow(unused, clippy::suspicious_map, clippy::iter_count)]
use std::collections::{BTreeSet, HashMap, HashSet}; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
#[warn(clippy::needless_collect)] #[warn(clippy::needless_collect)]
#[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)] #[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)]
@ -13,7 +13,13 @@ fn main() {
// Empty // Empty
} }
sample.iter().cloned().collect::<Vec<_>>().contains(&1); sample.iter().cloned().collect::<Vec<_>>().contains(&1);
// #7164 HashMap's and BTreeMap's `len` usage should not be linted
sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().len(); sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().len();
sample.iter().map(|x| (x, x)).collect::<BTreeMap<_, _>>().len();
sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().is_empty();
sample.iter().map(|x| (x, x)).collect::<BTreeMap<_, _>>().is_empty();
// Notice the `HashSet`--this should not be linted // Notice the `HashSet`--this should not be linted
sample.iter().collect::<HashSet<_>>().len(); sample.iter().collect::<HashSet<_>>().len();
// Neither should this // Neither should this

View file

@ -19,10 +19,16 @@ LL | sample.iter().cloned().collect::<Vec<_>>().contains(&1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == 1)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == 1)`
error: avoid using `collect()` when not needed error: avoid using `collect()` when not needed
--> $DIR/needless_collect.rs:16:35 --> $DIR/needless_collect.rs:20:35
| |
LL | sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().len(); LL | sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().is_empty();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`
error: aborting due to 4 previous errors error: avoid using `collect()` when not needed
--> $DIR/needless_collect.rs:21:35
|
LL | sample.iter().map(|x| (x, x)).collect::<BTreeMap<_, _>>().is_empty();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`
error: aborting due to 5 previous errors