mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Lint needless_collect on non-std collection types

Browse Source
This commit is contained in:
Jason Newcomb 2022-04-20 16:10:18 -04:00
parent b0dcb862c6
commit 8bfc8bc5e0
9 changed files with 408 additions and 151 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
clippy_lints/src/methods/collapsible_str_replace.rs
View File

@ -23,7 +23,7 @@ pub(super) fn check<'tcx>(
// If the parent node's `to` argument is the same as the `to` argument
// of the last replace call in the current chain, don't lint as it was already linted
if let Some(parent) = get_parent_expr(cx, expr)
&& let Some(("replace", _, [current_from, current_to], _)) = method_call(parent)
&& let Some(("replace", _, [current_from, current_to], _, _)) = method_call(parent)
&& eq_expr_value(cx, to, current_to)
&& from_kind == cx.typeck_results().expr_ty(current_from).peel_refs().kind()
{
@ -48,7 +48,7 @@ fn collect_replace_calls<'tcx>(
let mut from_args = VecDeque::new();
let _: Option<()> = for_each_expr(expr, |e| {
if let Some(("replace", _, [from, to], _)) = method_call(e) {
if let Some(("replace", _, [from, to], _, _)) = method_call(e) {
if eq_expr_value(cx, to_arg, to) && cx.typeck_results().expr_ty(from).peel_refs().is_char() {
methods.push_front(e);
from_args.push_front(from);
@ -78,7 +78,7 @@ fn check_consecutive_replace_calls<'tcx>(
.collect();
let app = Applicability::MachineApplicable;
let earliest_replace_call = replace_methods.methods.front().unwrap();
if let Some((_, _, [..], span_lo)) = method_call(earliest_replace_call) {
if let Some((_, _, [..], span_lo, _)) = method_call(earliest_replace_call) {
span_lint_and_sugg(
cx,
COLLAPSIBLE_STR_REPLACE,

2
clippy_lints/src/methods/manual_str_repeat.rs
View File

@ -59,10 +59,8 @@ pub(super) fn check(
if let ExprKind::Call(repeat_fn, [repeat_arg]) = take_self_arg.kind;
if is_path_diagnostic_item(cx, repeat_fn, sym::iter_repeat);
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(collect_expr), sym::String);
if let Some(collect_id) = cx.typeck_results().type_dependent_def_id(collect_expr.hir_id);
if let Some(take_id) = cx.typeck_results().type_dependent_def_id(take_expr.hir_id);
if let Some(iter_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
if cx.tcx.trait_of_item(collect_id) == Some(iter_trait_id);
if cx.tcx.trait_of_item(take_id) == Some(iter_trait_id);
if let Some(repeat_kind) = parse_repeat_arg(cx, repeat_arg);
let ctxt = collect_expr.span.ctxt();

15
clippy_lints/src/methods/map_collect_result_unit.rs
View File

@ -1,5 +1,4 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_trait_method;
use clippy_utils::source::snippet;
use clippy_utils::ty::is_type_diagnostic_item;
use if_chain::if_chain;
@ -11,18 +10,10 @@
use super::MAP_COLLECT_RESULT_UNIT;
pub(super) fn check(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
iter: &hir::Expr<'_>,
map_fn: &hir::Expr<'_>,
collect_recv: &hir::Expr<'_>,
) {
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, iter: &hir::Expr<'_>, map_fn: &hir::Expr<'_>) {
// return of collect `Result<(),_>`
let collect_ret_ty = cx.typeck_results().expr_ty(expr);
if_chain! {
// called on Iterator
if is_trait_method(cx, collect_recv, sym::Iterator);
// return of collect `Result<(),_>`
let collect_ret_ty = cx.typeck_results().expr_ty(expr);
if is_type_diagnostic_item(cx, collect_ret_ty, sym::Result);
if let ty::Adt(_, substs) = collect_ret_ty.kind();
if let Some(result_t) = substs.types().next();

95
clippy_lints/src/methods/mod.rs
View File

@ -3296,11 +3296,11 @@ pub fn new(
/// Extracts a method call name, args, and `Span` of the method name.
fn method_call<'tcx>(
recv: &'tcx hir::Expr<'tcx>,
) -> Option<(&'tcx str, &'tcx hir::Expr<'tcx>, &'tcx [hir::Expr<'tcx>], Span)> {
if let ExprKind::MethodCall(path, receiver, args, _) = recv.kind {
) -> Option<(&'tcx str, &'tcx hir::Expr<'tcx>, &'tcx [hir::Expr<'tcx>], Span, Span)> {
if let ExprKind::MethodCall(path, receiver, args, call_span) = recv.kind {
if !args.iter().any(|e| e.span.from_expansion()) && !receiver.span.from_expansion() {
let name = path.ident.name.as_str();
return Some((name, receiver, args, path.ident.span));
return Some((name, receiver, args, path.ident.span, call_span));
}
}
None
@ -3341,8 +3341,6 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
},
_ => (),
}
needless_collect::check(expr, cx);
}
#[allow(clippy::too_many_lines)]
@ -3488,7 +3486,7 @@ fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>
impl Methods {
#[allow(clippy::too_many_lines)]
fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some((name, recv, args, span)) = method_call(expr) {
if let Some((name, recv, args, span, call_span)) = method_call(expr) {
match (name, args) {
("add" | "offset" | "sub" | "wrapping_offset" | "wrapping_add" | "wrapping_sub", [_arg]) => {
zst_offset::check(cx, expr, recv);
@ -3507,28 +3505,31 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
("as_ref", []) => useless_asref::check(cx, expr, "as_ref", recv),
("assume_init", []) => uninit_assumed_init::check(cx, expr, recv),
("cloned", []) => cloned_instead_of_copied::check(cx, expr, recv, span, self.msrv),
("collect", []) => match method_call(recv) {
Some((name @ ("cloned" | "copied"), recv2, [], _)) => {
iter_cloned_collect::check(cx, name, expr, recv2);
},
Some(("map", m_recv, [m_arg], _)) => {
map_collect_result_unit::check(cx, expr, m_recv, m_arg, recv);
},
Some(("take", take_self_arg, [take_arg], _)) => {
if meets_msrv(self.msrv, msrvs::STR_REPEAT) {
manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg);
}
},
_ => {},
("collect", []) if is_trait_method(cx, expr, sym::Iterator) => {
needless_collect::check(cx, span, expr, recv, call_span);
match method_call(recv) {
Some((name @ ("cloned" | "copied"), recv2, [], _, _)) => {
iter_cloned_collect::check(cx, name, expr, recv2);
},
Some(("map", m_recv, [m_arg], _, _)) => {
map_collect_result_unit::check(cx, expr, m_recv, m_arg);
},
Some(("take", take_self_arg, [take_arg], _, _)) => {
if meets_msrv(self.msrv, msrvs::STR_REPEAT) {
manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg);
}
},
_ => {},
}
},
("count", []) if is_trait_method(cx, expr, sym::Iterator) => match method_call(recv) {
Some(("cloned", recv2, [], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, true, false),
Some((name2 @ ("into_iter" | "iter" | "iter_mut"), recv2, [], _)) => {
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, true, false),
Some((name2 @ ("into_iter" | "iter" | "iter_mut"), recv2, [], _, _)) => {
iter_count::check(cx, expr, recv2, name2);
},
Some(("map", _, [arg], _)) => suspicious_map::check(cx, expr, recv, arg),
Some(("filter", recv2, [arg], _)) => bytecount::check(cx, expr, recv2, arg),
Some(("bytes", recv2, [], _)) => bytes_count_to_len::check(cx, expr, recv, recv2),
Some(("map", _, [arg], _, _)) => suspicious_map::check(cx, expr, recv, arg),
Some(("filter", recv2, [arg], _, _)) => bytecount::check(cx, expr, recv2, arg),
Some(("bytes", recv2, [], _, _)) => bytes_count_to_len::check(cx, expr, recv, recv2),
_ => {},
},
("drain", [arg]) => {
@ -3540,8 +3541,8 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
}
},
("expect", [_]) => match method_call(recv) {
Some(("ok", recv, [], _)) => ok_expect::check(cx, expr, recv),
Some(("err", recv, [], err_span)) => err_expect::check(cx, expr, recv, self.msrv, span, err_span),
Some(("ok", recv, [], _, _)) => ok_expect::check(cx, expr, recv),
Some(("err", recv, [], err_span, _)) => err_expect::check(cx, expr, recv, self.msrv, span, err_span),
_ => expect_used::check(cx, expr, recv, false, self.allow_expect_in_tests),
},
("expect_err", [_]) => expect_used::check(cx, expr, recv, true, self.allow_expect_in_tests),
@ -3561,13 +3562,13 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
flat_map_option::check(cx, expr, arg, span);
},
("flatten", []) => match method_call(recv) {
Some(("map", recv, [map_arg], map_span)) => map_flatten::check(cx, expr, recv, map_arg, map_span),
Some(("cloned", recv2, [], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, true),
Some(("map", recv, [map_arg], map_span, _)) => map_flatten::check(cx, expr, recv, map_arg, map_span),
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, true),
_ => {},
},
("fold", [init, acc]) => unnecessary_fold::check(cx, expr, init, acc, span),
("for_each", [_]) => {
if let Some(("inspect", _, [_], span2)) = method_call(recv) {
if let Some(("inspect", _, [_], span2, _)) = method_call(recv) {
inspect_for_each::check(cx, expr, span2);
}
},
@ -3587,12 +3588,12 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
iter_on_single_or_empty_collections::check(cx, expr, name, recv);
},
("join", [join_arg]) => {
if let Some(("collect", _, _, span)) = method_call(recv) {
if let Some(("collect", _, _, span, _)) = method_call(recv) {
unnecessary_join::check(cx, expr, recv, join_arg, span);
}
},
("last", []) | ("skip", [_]) => {
if let Some((name2, recv2, args2, _span2)) = method_call(recv) {
if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
if let ("cloned", []) = (name2, args2) {
iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
}
@ -3604,13 +3605,13 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
(name @ ("map" | "map_err"), [m_arg]) => {
if name == "map" {
map_clone::check(cx, expr, recv, m_arg, self.msrv);
if let Some((map_name @ ("iter" | "into_iter"), recv2, _, _)) = method_call(recv) {
if let Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) = method_call(recv) {
iter_kv_map::check(cx, map_name, expr, recv2, m_arg);
}
} else {
map_err_ignore::check(cx, expr, m_arg);
}
if let Some((name, recv2, args, span2)) = method_call(recv) {
if let Some((name, recv2, args, span2,_)) = method_call(recv) {
match (name, args) {
("as_mut", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, true, self.msrv),
("as_ref", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, false, self.msrv),
@ -3630,7 +3631,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
manual_ok_or::check(cx, expr, recv, def, map);
},
("next", []) => {
if let Some((name2, recv2, args2, _)) = method_call(recv) {
if let Some((name2, recv2, args2, _, _)) = method_call(recv) {
match (name2, args2) {
("cloned", []) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
("filter", [arg]) => filter_next::check(cx, expr, recv2, arg),
@ -3643,10 +3644,10 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
}
},
("nth", [n_arg]) => match method_call(recv) {
Some(("bytes", recv2, [], _)) => bytes_nth::check(cx, expr, recv2, n_arg),
Some(("cloned", recv2, [], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
Some(("iter", recv2, [], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false),
Some(("iter_mut", recv2, [], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true),
Some(("bytes", recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg),
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
Some(("iter", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false),
Some(("iter_mut", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true),
_ => iter_nth_zero::check(cx, expr, recv, n_arg),
},
("ok_or_else", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"),
@ -3711,7 +3712,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
},
("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
("take", [_arg]) => {
if let Some((name2, recv2, args2, _span2)) = method_call(recv) {
if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
if let ("cloned", []) = (name2, args2) {
iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
}
@ -3734,13 +3735,13 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
},
("unwrap", []) => {
match method_call(recv) {
Some(("get", recv, [get_arg], _)) => {
Some(("get", recv, [get_arg], _, _)) => {
get_unwrap::check(cx, expr, recv, get_arg, false);
},
Some(("get_mut", recv, [get_arg], _)) => {
Some(("get_mut", recv, [get_arg], _, _)) => {
get_unwrap::check(cx, expr, recv, get_arg, true);
},
Some(("or", recv, [or_arg], or_span)) => {
Some(("or", recv, [or_arg], or_span, _)) => {
or_then_unwrap::check(cx, expr, recv, or_arg, or_span);
},
_ => {},
@ -3749,19 +3750,19 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
},
("unwrap_err", []) => unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests),
("unwrap_or", [u_arg]) => match method_call(recv) {
Some((arith @ ("checked_add" | "checked_sub" | "checked_mul"), lhs, [rhs], _)) => {
Some((arith @ ("checked_add" | "checked_sub" | "checked_mul"), lhs, [rhs], _, _)) => {
manual_saturating_arithmetic::check(cx, expr, lhs, rhs, u_arg, &arith["checked_".len()..]);
},
Some(("map", m_recv, [m_arg], span)) => {
Some(("map", m_recv, [m_arg], span, _)) => {
option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span);
},
Some(("then_some", t_recv, [t_arg], _)) => {
Some(("then_some", t_recv, [t_arg], _, _)) => {
obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg);
},
_ => {},
},
("unwrap_or_else", [u_arg]) => match method_call(recv) {
Some(("map", recv, [map_arg], _))
Some(("map", recv, [map_arg], _, _))
if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, self.msrv) => {},
_ => {
unwrap_or_else_default::check(cx, expr, recv, u_arg);
@ -3782,7 +3783,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
}
fn check_is_some_is_none(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, is_some: bool) {
if let Some((name @ ("find" | "position" | "rposition"), f_recv, [arg], span)) = method_call(recv) {
if let Some((name @ ("find" | "position" | "rposition"), f_recv, [arg], span, _)) = method_call(recv) {
search_is_some::check(cx, expr, name, is_some, f_recv, arg, recv, span);
}
}

223
clippy_lints/src/methods/needless_collect.rs
View File

@ -3,94 +3,99 @@
use clippy_utils::higher;
use clippy_utils::source::{snippet, snippet_with_applicability};
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{can_move_expr_to_closure, is_trait_method, path_to_local, path_to_local_id, CaptureKind};
use if_chain::if_chain;
use clippy_utils::ty::{is_type_diagnostic_item, make_normalized_projection, make_projection};
use clippy_utils::{
can_move_expr_to_closure, get_enclosing_block, get_parent_node, is_trait_method, path_to_local, path_to_local_id,
CaptureKind,
};
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{Applicability, MultiSpan};
use rustc_hir::intravisit::{walk_block, walk_expr, Visitor};
use rustc_hir::{Block, Expr, ExprKind, HirId, HirIdSet, Local, Mutability, Node, PatKind, Stmt, StmtKind};
use rustc_hir::{
BindingAnnotation, Block, Expr, ExprKind, HirId, HirIdSet, Local, Mutability, Node, PatKind, Stmt, StmtKind,
};
use rustc_lint::LateContext;
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_middle::ty::{self, Ty};
use rustc_span::sym;
use rustc_span::Span;
use rustc_middle::ty::{self, AssocKind, EarlyBinder, GenericArg, GenericArgKind, Ty};
use rustc_span::symbol::Ident;
use rustc_span::{sym, Span, Symbol};
const NEEDLESS_COLLECT_MSG: &str = "avoid using `collect()` when not needed";
pub(super) fn check<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
check_needless_collect_direct_usage(expr, cx);
check_needless_collect_indirect_usage(expr, cx);
}
fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
if_chain! {
if let ExprKind::MethodCall(method, receiver, args, _) = expr.kind;
if let ExprKind::MethodCall(chain_method, ..) = receiver.kind;
if chain_method.ident.name == sym!(collect) && is_trait_method(cx, receiver, sym::Iterator);
then {
let ty = cx.typeck_results().expr_ty(receiver);
let mut applicability = Applicability::MaybeIncorrect;
let is_empty_sugg = "next().is_none()".to_string();
let method_name = method.ident.name.as_str();
let sugg = if is_type_diagnostic_item(cx, ty, sym::Vec) ||
is_type_diagnostic_item(cx, ty, sym::VecDeque) ||
is_type_diagnostic_item(cx, ty, sym::LinkedList) ||
is_type_diagnostic_item(cx, ty, sym::BinaryHeap) {
match method_name {
"len" => "count()".to_string(),
"is_empty" => is_empty_sugg,
"contains" => {
let contains_arg = snippet_with_applicability(cx, args[0].span, "??", &mut applicability);
let (arg, pred) = contains_arg
.strip_prefix('&')
.map_or(("&x", &*contains_arg), |s| ("x", s));
format!("any(|{arg}| x == {pred})")
}
_ => return,
}
}
else if is_type_diagnostic_item(cx, ty, sym::BTreeMap) ||
is_type_diagnostic_item(cx, ty, sym::HashMap) {
match method_name {
"is_empty" => is_empty_sugg,
_ => return,
}
}
else {
return;
};
span_lint_and_sugg(
cx,
NEEDLESS_COLLECT,
chain_method.ident.span.with_hi(expr.span.hi()),
NEEDLESS_COLLECT_MSG,
"replace with",
sugg,
applicability,
);
}
}
}
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
name_span: Span,
collect_expr: &'tcx Expr<'_>,
iter_expr: &'tcx Expr<'tcx>,
call_span: Span,
) {
if let Some(parent) = get_parent_node(cx.tcx, collect_expr.hir_id) {
match parent {
Node::Expr(parent) => {
if let ExprKind::MethodCall(name, _, args @ ([] | [_]), _) = parent.kind {
let mut app = Applicability::MachineApplicable;
let name = name.ident.as_str();
let collect_ty = cx.typeck_results().expr_ty(collect_expr);
fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
if let ExprKind::Block(block, _) = expr.kind {
for stmt in block.stmts {
if_chain! {
if let StmtKind::Local(local) = stmt.kind;
if let PatKind::Binding(_, id, ..) = local.pat.kind;
if let Some(init_expr) = local.init;
if let ExprKind::MethodCall(method_name, iter_source, [], ..) = init_expr.kind;
if method_name.ident.name == sym!(collect) && is_trait_method(cx, init_expr, sym::Iterator);
let ty = cx.typeck_results().expr_ty(init_expr);
if is_type_diagnostic_item(cx, ty, sym::Vec) ||
is_type_diagnostic_item(cx, ty, sym::VecDeque) ||
is_type_diagnostic_item(cx, ty, sym::BinaryHeap) ||
is_type_diagnostic_item(cx, ty, sym::LinkedList);
let iter_ty = cx.typeck_results().expr_ty(iter_source);
if let Some(iter_calls) = detect_iter_and_into_iters(block, id, cx, get_captured_ids(cx, iter_ty));
if let [iter_call] = &*iter_calls;
then {
let sugg: String = match name {
"len" => {
if let Some(adt) = collect_ty.ty_adt_def()
&& matches!(
cx.tcx.get_diagnostic_name(adt.did()),
Some(sym::Vec | sym::VecDeque | sym::LinkedList | sym::BinaryHeap)
)
{
"count()".into()
} else {
return;
}
},
"is_empty"
if is_is_empty_sig(cx, parent.hir_id)
&& iterates_same_ty(cx, cx.typeck_results().expr_ty(iter_expr), collect_ty) =>
{
"next().is_none()".into()
},
"contains" => {
if is_contains_sig(cx, parent.hir_id, iter_expr)
&& let Some(arg) = args.first()
{
let (span, prefix) = if let ExprKind::AddrOf(_, _, arg) = arg.kind {
(arg.span, "")
} else {
(arg.span, "*")
};
let snip = snippet_with_applicability(cx, span, "??", &mut app);
format!("any(|x| x == {prefix}{snip})")
} else {
return;
}
},
_ => return,
};
span_lint_and_sugg(
cx,
NEEDLESS_COLLECT,
call_span.with_hi(parent.span.hi()),
NEEDLESS_COLLECT_MSG,
"replace with",
sugg,
app,
);
}
},
Node::Local(l) => {
if let PatKind::Binding(BindingAnnotation::NONE | BindingAnnotation::MUT, id, _, None)
= l.pat.kind
&& let ty = cx.typeck_results().expr_ty(collect_expr)
&& [sym::Vec, sym::VecDeque, sym::BinaryHeap, sym::LinkedList].into_iter()
.any(|item| is_type_diagnostic_item(cx, ty, item))
&& let iter_ty = cx.typeck_results().expr_ty(iter_expr)
&& let Some(block) = get_enclosing_block(cx, l.hir_id)
&& let Some(iter_calls) = detect_iter_and_into_iters(block, id, cx, get_captured_ids(cx, iter_ty))
&& let [iter_call] = &*iter_calls
{
let mut used_count_visitor = UsedCountVisitor {
cx,
id,
@ -102,20 +107,20 @@ fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCo
}
// Suggest replacing iter_call with iter_replacement, and removing stmt
let mut span = MultiSpan::from_span(method_name.ident.span);
let mut span = MultiSpan::from_span(name_span);
span.push_span_label(iter_call.span, "the iterator could be used here instead");
span_lint_hir_and_then(
cx,
super::NEEDLESS_COLLECT,
init_expr.hir_id,
collect_expr.hir_id,
span,
NEEDLESS_COLLECT_MSG,
|diag| {
let iter_replacement = format!("{}{}", Sugg::hir(cx, iter_source, ".."), iter_call.get_iter_method(cx));
let iter_replacement = format!("{}{}", Sugg::hir(cx, iter_expr, ".."), iter_call.get_iter_method(cx));
diag.multipart_suggestion(
iter_call.get_suggestion_text(),
vec![
(stmt.span, String::new()),
(l.span, String::new()),
(iter_call.span, iter_replacement)
],
Applicability::MaybeIncorrect,
@ -123,11 +128,61 @@ fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCo
},
);
}
}
},
_ => (),
}
}
}
/// Checks if the given method call matches the expected signature of `([&[mut]] self) -> bool`
fn is_is_empty_sig(cx: &LateContext<'_>, call_id: HirId) -> bool {
cx.typeck_results().type_dependent_def_id(call_id).map_or(false, |id| {
let sig = cx.tcx.fn_sig(id).skip_binder();
sig.inputs().len() == 1 && sig.output().is_bool()
})
}
/// Checks if `<iter_ty as Iterator>::Item` is the same as `<collect_ty as IntoIter>::Item`
fn iterates_same_ty<'tcx>(cx: &LateContext<'tcx>, iter_ty: Ty<'tcx>, collect_ty: Ty<'tcx>) -> bool {
let item = Symbol::intern("Item");
if let Some(iter_trait) = cx.tcx.get_diagnostic_item(sym::Iterator)
&& let Some(into_iter_trait) = cx.tcx.get_diagnostic_item(sym::IntoIterator)
&& let Some(iter_item_ty) = make_normalized_projection(cx.tcx, cx.param_env, iter_trait, item, [iter_ty])
&& let Some(into_iter_item_proj) = make_projection(cx.tcx, into_iter_trait, item, [collect_ty])
&& let Ok(into_iter_item_ty) = cx.tcx.try_normalize_erasing_regions(
cx.param_env,
cx.tcx.mk_projection(into_iter_item_proj.item_def_id, into_iter_item_proj.substs)
)
{
iter_item_ty == into_iter_item_ty
} else {
false
}
}
/// Checks if the given method call matches the expected signature of
/// `([&[mut]] self, &<iter_ty as Iterator>::Item) -> bool`
fn is_contains_sig(cx: &LateContext<'_>, call_id: HirId, iter_expr: &Expr<'_>) -> bool {
let typeck = cx.typeck_results();
if let Some(id) = typeck.type_dependent_def_id(call_id)
&& let sig = cx.tcx.fn_sig(id)
&& sig.skip_binder().output().is_bool()
&& let [_, search_ty] = *sig.skip_binder().inputs()
&& let ty::Ref(_, search_ty, Mutability::Not) = *cx.tcx.erase_late_bound_regions(sig.rebind(search_ty)).kind()
&& let Some(iter_trait) = cx.tcx.get_diagnostic_item(sym::Iterator)
&& let Some(iter_item) = cx.tcx
.associated_items(iter_trait)
.find_by_name_and_kind(cx.tcx, Ident::with_dummy_span(Symbol::intern("Item")), AssocKind::Type, iter_trait)
&& let substs = cx.tcx.mk_substs([GenericArg::from(typeck.expr_ty_adjusted(iter_expr))].into_iter())
&& let proj_ty = cx.tcx.mk_projection(iter_item.def_id, substs)
&& let Ok(item_ty) = cx.tcx.try_normalize_erasing_regions(cx.param_env, proj_ty)
{
item_ty == EarlyBinder(search_ty).subst(cx.tcx, cx.typeck_results().node_substs(call_id))
} else {
false
}
}
struct IterFunction {
func: IterFunctionKind,
span: Span,

134
clippy_utils/src/ty.rs
View File

@ -13,8 +13,9 @@
use rustc_lint::LateContext;
use rustc_middle::mir::interpret::{ConstValue, Scalar};
use rustc_middle::ty::{
self, AdtDef, Binder, BoundRegion, DefIdTree, FnSig, IntTy, ParamEnv, Predicate, PredicateKind, ProjectionTy,
Region, RegionKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor, UintTy, VariantDef, VariantDiscr,
self, AdtDef, AssocKind, Binder, BoundRegion, DefIdTree, FnSig, GenericParamDefKind, IntTy, ParamEnv, Predicate,
PredicateKind, ProjectionTy, Region, RegionKind, SubstsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable,
TypeVisitor, UintTy, VariantDef, VariantDiscr,
};
use rustc_middle::ty::{GenericArg, GenericArgKind};
use rustc_span::symbol::Ident;
@ -938,3 +939,132 @@ pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {
(Err(_), _) => 0,
}
}
/// Makes the projection type for the named associated type in the given impl or trait impl.
///
/// This function is for associated types which are "known" to exist, and as such, will only return
/// `None` when debug assertions are disabled in order to prevent ICE's. With debug assertions
/// enabled this will check that the named associated type exists, the correct number of
/// substitutions are given, and that the correct kinds of substitutions are given (lifetime,
/// constant or type). This will not check if type normalization would succeed.
pub fn make_projection<'tcx>(
tcx: TyCtxt<'tcx>,
container_id: DefId,
assoc_ty: Symbol,
substs: impl IntoIterator<Item = impl Into<GenericArg<'tcx>>>,
) -> Option<ProjectionTy<'tcx>> {
fn helper<'tcx>(
tcx: TyCtxt<'tcx>,
container_id: DefId,
assoc_ty: Symbol,
substs: SubstsRef<'tcx>,
) -> Option<ProjectionTy<'tcx>> {
let Some(assoc_item) = tcx
.associated_items(container_id)
.find_by_name_and_kind(tcx, Ident::with_dummy_span(assoc_ty), AssocKind::Type, container_id)
else {
debug_assert!(false, "type `{assoc_ty}` not found in `{container_id:?}`");
return None;
};
#[cfg(debug_assertions)]
{
let generics = tcx.generics_of(assoc_item.def_id);
let generic_count = generics.parent_count + generics.params.len();
let params = generics
.parent
.map_or([].as_slice(), |id| &*tcx.generics_of(id).params)
.iter()
.chain(&generics.params)
.map(|x| &x.kind);
debug_assert!(
generic_count == substs.len(),
"wrong number of substs for `{:?}`: found `{}` expected `{}`.\n\
note: the expected parameters are: {:#?}\n\
the given arguments are: `{:#?}`",
assoc_item.def_id,
substs.len(),
generic_count,
params.map(GenericParamDefKind::descr).collect::<Vec<_>>(),
substs,
);
if let Some((idx, (param, arg))) = params
.clone()
.zip(substs.iter().map(GenericArg::unpack))
.enumerate()
.find(|(_, (param, arg))| {
!matches!(
(param, arg),
(GenericParamDefKind::Lifetime, GenericArgKind::Lifetime(_))
| (GenericParamDefKind::Type { .. }, GenericArgKind::Type(_))
| (GenericParamDefKind::Const { .. }, GenericArgKind::Const(_))
)
})
{
debug_assert!(
false,
"mismatched subst type at index {}: expected a {}, found `{:?}`\n\
note: the expected parameters are {:#?}\n\
the given arguments are {:#?}",
idx,
param.descr(),
arg,
params.map(GenericParamDefKind::descr).collect::<Vec<_>>(),
substs,
);
}
}
Some(ProjectionTy {
substs,
item_def_id: assoc_item.def_id,
})
}
helper(
tcx,
container_id,
assoc_ty,
tcx.mk_substs(substs.into_iter().map(Into::into)),
)
}
/// Normalizes the named associated type in the given impl or trait impl.
///
/// This function is for associated types which are "known" to be valid with the given
/// substitutions, and as such, will only return `None` when debug assertions are disabled in order
/// to prevent ICE's. With debug assertions enabled this will check that that type normalization
/// succeeds as well as everything checked by `make_projection`.
pub fn make_normalized_projection<'tcx>(
tcx: TyCtxt<'tcx>,
param_env: ParamEnv<'tcx>,
container_id: DefId,
assoc_ty: Symbol,
substs: impl IntoIterator<Item = impl Into<GenericArg<'tcx>>>,
) -> Option<Ty<'tcx>> {
fn helper<'tcx>(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, ty: ProjectionTy<'tcx>) -> Option<Ty<'tcx>> {
#[cfg(debug_assertions)]
if let Some((i, subst)) = ty
.substs
.iter()
.enumerate()
.find(|(_, subst)| subst.has_late_bound_regions())
{
debug_assert!(
false,
"substs contain late-bound region at index `{i}` which can't be normalized.\n\
use `TyCtxt::erase_late_bound_regions`\n\
note: subst is `{subst:#?}`",
);
return None;
}
match tcx.try_normalize_erasing_regions(param_env, tcx.mk_projection(ty.item_def_id, ty.substs)) {
Ok(ty) => Some(ty),
Err(e) => {
debug_assert!(false, "failed to normalize type `{ty}`: {e:#?}");
None
},
}
}
helper(tcx, param_env, make_projection(tcx, container_id, assoc_ty, substs)?)
}

29
tests/ui/needless_collect.fixed
View File

@ -33,4 +33,33 @@ fn main() {
// `BinaryHeap` doesn't have `contains` method
sample.iter().count();
sample.iter().next().is_none();
// Don't lint string from str
let _ = ["", ""].into_iter().collect::<String>().is_empty();
let _ = sample.iter().next().is_none();
let _ = sample.iter().any(|x| x == &0);
struct VecWrapper<T>(Vec<T>);
impl<T> core::ops::Deref for VecWrapper<T> {
type Target = Vec<T>;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl<T> IntoIterator for VecWrapper<T> {
type IntoIter = <Vec<T> as IntoIterator>::IntoIter;
type Item = <Vec<T> as IntoIterator>::Item;
fn into_iter(self) -> Self::IntoIter {
self.0.into_iter()
}
}
impl<T> FromIterator<T> for VecWrapper<T> {
fn from_iter<I: IntoIterator<Item = T>>(iter: I) -> Self {
Self(Vec::from_iter(iter))
}
}
let _ = sample.iter().next().is_none();
let _ = sample.iter().any(|x| x == &0);
}

29
tests/ui/needless_collect.rs
View File

@ -33,4 +33,33 @@ fn main() {
// `BinaryHeap` doesn't have `contains` method
sample.iter().collect::<BinaryHeap<_>>().len();
sample.iter().collect::<BinaryHeap<_>>().is_empty();
// Don't lint string from str
let _ = ["", ""].into_iter().collect::<String>().is_empty();
let _ = sample.iter().collect::<HashSet<_>>().is_empty();
let _ = sample.iter().collect::<HashSet<_>>().contains(&&0);
struct VecWrapper<T>(Vec<T>);
impl<T> core::ops::Deref for VecWrapper<T> {
type Target = Vec<T>;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl<T> IntoIterator for VecWrapper<T> {
type IntoIter = <Vec<T> as IntoIterator>::IntoIter;
type Item = <Vec<T> as IntoIterator>::Item;
fn into_iter(self) -> Self::IntoIter {
self.0.into_iter()
}
}
impl<T> FromIterator<T> for VecWrapper<T> {
fn from_iter<I: IntoIterator<Item = T>>(iter: I) -> Self {
Self(Vec::from_iter(iter))
}
}
let _ = sample.iter().collect::<VecWrapper<_>>().is_empty();
let _ = sample.iter().collect::<VecWrapper<_>>().contains(&&0);
}

26
tests/ui/needless_collect.stderr
View File

@ -66,5 +66,29 @@ error: avoid using `collect()` when not needed
LL | sample.iter().collect::<BinaryHeap<_>>().is_empty();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`
error: aborting due to 11 previous errors
error: avoid using `collect()` when not needed
--> $DIR/needless_collect.rs:40:27
|
LL | let _ = sample.iter().collect::<HashSet<_>>().is_empty();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`
error: avoid using `collect()` when not needed
--> $DIR/needless_collect.rs:41:27
|
LL | let _ = sample.iter().collect::<HashSet<_>>().contains(&&0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == &0)`
error: avoid using `collect()` when not needed
--> $DIR/needless_collect.rs:63:27
|
LL | let _ = sample.iter().collect::<VecWrapper<_>>().is_empty();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`
error: avoid using `collect()` when not needed
--> $DIR/needless_collect.rs:64:27
|
LL | let _ = sample.iter().collect::<VecWrapper<_>>().contains(&&0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == &0)`
error: aborting due to 15 previous errors