squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Auto merge of #7163 - mgacek8:issue7110_needless_collect_with_type_annotations, r=flip1995

Browse source 
needless_collect: Lint cases with type annotations for indirect usage and recognize `BinaryHeap`

fixes #7110
changelog: needless_collect: Lint cases with type annotations for indirect usage and recognize `BinaryHeap`.
This commit is contained in:
bors 2021-05-05 15:43:02 +00:00
commit 0baf6bf226
4 changed files with 112 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
clippy_lints/src/loops/needless_collect.rs
View file

@ -7,9 +7,10 @@ use clippy_utils::{is_trait_method, path_to_local_id, paths};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_block, walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{Block, Expr, ExprKind, GenericArg, HirId, Local, Pat, PatKind, QPath, StmtKind};
use rustc_hir::{Block, Expr, ExprKind, GenericArg, GenericArgs, HirId, Local, Pat, PatKind, QPath, StmtKind, Ty};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use rustc_span::symbol::{sym, Ident};
use rustc_span::{MultiSpan, Span};
@ -26,7 +27,7 @@ fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCont
if chain_method.ident.name == sym!(collect) && is_trait_method(cx, &args[0], sym::Iterator);
if let Some(generic_args) = chain_method.args;
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
let ty = cx.typeck_results().node_type(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)
|| is_type_diagnostic_item(cx, ty, sym::vecdeque_type)
|| match_type(cx, ty, &paths::BTREEMAP)
@ -58,20 +59,33 @@ 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 get_hir_id<'tcx>(ty: Option<&Ty<'tcx>>, method_args: Option<&GenericArgs<'tcx>>) -> Option<HirId> {
if let Some(ty) = ty {
return Some(ty.hir_id);
}
if let Some(generic_args) = method_args {
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0) {
return Some(ty.hir_id);
}
}
None
}
if let ExprKind::Block(block, _) = expr.kind {
for stmt in block.stmts {
if_chain! {
if let StmtKind::Local(
Local { pat: Pat { hir_id: pat_id, kind: PatKind::Binding(_, _, ident, .. ), .. },
init: Some(init_expr), .. }
init: Some(init_expr), ty, .. }
) = stmt.kind;
if let ExprKind::MethodCall(method_name, collect_span, &[ref iter_source], ..) = init_expr.kind;
if method_name.ident.name == sym!(collect) && is_trait_method(cx, init_expr, sym::Iterator);
if let Some(generic_args) = method_name.args;
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
if let ty = cx.typeck_results().node_type(ty.hir_id);
if let Some(hir_id) = get_hir_id(*ty, method_name.args);
if let Some(ty) = cx.typeck_results().node_type_opt(hir_id);
if is_type_diagnostic_item(cx, ty, sym::vec_type) ||
is_type_diagnostic_item(cx, ty, sym::vecdeque_type) ||
is_type_diagnostic_item(cx, ty, sym::BinaryHeap) ||
match_type(cx, ty, &paths::LINKED_LIST);
if let Some(iter_calls) = detect_iter_and_into_iters(block, *ident);
if let [iter_call] = &*iter_calls;

6
tests/ui/copy_iterator.rs
View file

@ -16,8 +16,6 @@ impl Iterator for Countdown {
fn main() {
let my_iterator = Countdown(5);
let a: Vec<_> = my_iterator.take(1).collect();
assert_eq!(a.len(), 1);
let b: Vec<_> = my_iterator.collect();
assert_eq!(b.len(), 5);
assert_eq!(my_iterator.take(1).count(), 1);
assert_eq!(my_iterator.count(), 5);
}

34
tests/ui/needless_collect_indirect.rs
View file

@ -1,4 +1,4 @@
use std::collections::{HashMap, VecDeque};
use std::collections::{BinaryHeap, HashMap, LinkedList, VecDeque};
fn main() {
let sample = [1; 5];
@ -43,3 +43,35 @@ fn main() {
.collect::<Vec<_>>();
}
}
mod issue7110 {
// #7110 - lint for type annotation cases
use super::*;
fn lint_vec(string: &str) -> usize {
let buffer: Vec<&str> = string.split('/').collect();
buffer.len()
}
fn lint_vec_deque() -> usize {
let sample = [1; 5];
let indirect_len: VecDeque<_> = sample.iter().collect();
indirect_len.len()
}
fn lint_linked_list() -> usize {
let sample = [1; 5];
let indirect_len: LinkedList<_> = sample.iter().collect();
indirect_len.len()
}
fn lint_binary_heap() -> usize {
let sample = [1; 5];
let indirect_len: BinaryHeap<_> = sample.iter().collect();
indirect_len.len()
}
fn dont_lint(string: &str) -> usize {
let buffer: Vec<&str> = string.split('/').collect();
for buff in &buffer {
println!("{}", buff);
}
buffer.len()
}
}

58
tests/ui/needless_collect_indirect.stderr
View file

@ -69,5 +69,61 @@ LL |
LL | sample.into_iter().any(|x| x == a);
|
error: aborting due to 5 previous errors
error: avoid using `collect()` when not needed
--> $DIR/needless_collect_indirect.rs:52:51
|
LL | let buffer: Vec<&str> = string.split('/').collect();
| ^^^^^^^
LL | buffer.len()
| ------------ the iterator could be used here instead
|
help: take the original Iterator's count instead of collecting it and finding the length
|
LL |
LL | string.split('/').count()
|
error: avoid using `collect()` when not needed
--> $DIR/needless_collect_indirect.rs:57:55
|
LL | let indirect_len: VecDeque<_> = sample.iter().collect();
| ^^^^^^^
LL | indirect_len.len()
| ------------------ the iterator could be used here instead
|
help: take the original Iterator's count instead of collecting it and finding the length
|
LL |
LL | sample.iter().count()
|
error: avoid using `collect()` when not needed
--> $DIR/needless_collect_indirect.rs:62:57
|
LL | let indirect_len: LinkedList<_> = sample.iter().collect();
| ^^^^^^^
LL | indirect_len.len()
| ------------------ the iterator could be used here instead
|
help: take the original Iterator's count instead of collecting it and finding the length
|
LL |
LL | sample.iter().count()
|
error: avoid using `collect()` when not needed
--> $DIR/needless_collect_indirect.rs:67:57
|
LL | let indirect_len: BinaryHeap<_> = sample.iter().collect();
| ^^^^^^^
LL | indirect_len.len()
| ------------------ the iterator could be used here instead
|
help: take the original Iterator's count instead of collecting it and finding the length
|
LL |
LL | sample.iter().count()
|
error: aborting due to 9 previous errors