From b1faaaeb0cfb3310c2679f22a33d5ec855f8aaf8 Mon Sep 17 00:00:00 2001 From: Mateusz Gacek <96mateusz.gacek@gmail.com> Date: Mon, 3 May 2021 13:36:08 -0700 Subject: [PATCH] needless_collect: Lint cases with type annotations --- clippy_lints/src/loops/needless_collect.rs | 23 ++++++++--- tests/ui/needless_collect_indirect.rs | 29 +++++++++++++- tests/ui/needless_collect_indirect.stderr | 44 +++++++++++++++++++++- 3 files changed, 89 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/loops/needless_collect.rs b/clippy_lints/src/loops/needless_collect.rs index 4d73aef76e8..293dd8b6b79 100644 --- a/clippy_lints/src/loops/needless_collect.rs +++ b/clippy_lints/src/loops/needless_collect.rs @@ -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}; @@ -58,18 +59,30 @@ 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 { + 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 ty = cx.typeck_results().node_type(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::LINKED_LIST); diff --git a/tests/ui/needless_collect_indirect.rs b/tests/ui/needless_collect_indirect.rs index 0918a6868ab..2647c6401a4 100644 --- a/tests/ui/needless_collect_indirect.rs +++ b/tests/ui/needless_collect_indirect.rs @@ -1,4 +1,4 @@ -use std::collections::{HashMap, VecDeque}; +use std::collections::{HashMap, LinkedList, VecDeque}; fn main() { let sample = [1; 5]; @@ -43,3 +43,30 @@ fn main() { .collect::>(); } } + +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 dont_lint(string: &str) -> usize { + let buffer: Vec<&str> = string.split('/').collect(); + for buff in &buffer { + println!("{}", buff); + } + buffer.len() + } +} diff --git a/tests/ui/needless_collect_indirect.stderr b/tests/ui/needless_collect_indirect.stderr index c773b841f3b..642beb2865a 100644 --- a/tests/ui/needless_collect_indirect.stderr +++ b/tests/ui/needless_collect_indirect.stderr @@ -69,5 +69,47 @@ 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: aborting due to 8 previous errors