Skip to content

Commit 8bfc8bc

Browse files
JarchoJarcho
committed
Lint needless_collect on non-std collection types
1 parent b0dcb86 commit 8bfc8bc

File tree

9 files changed

+408
-151
lines changed

9 files changed

+408
-151
lines changed

‎clippy_lints/src/methods/collapsible_str_replace.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ pub(super) fn check<'tcx>(
2323
// If the parent node's `to` argument is the same as the `to` argument
2424
// of the last replace call in the current chain, don't lint as it was already linted
2525
if let Some(parent) = get_parent_expr(cx, expr)
26-
&& let Some(("replace", _, [current_from, current_to], _)) = method_call(parent)
26+
&& let Some(("replace", _, [current_from, current_to], _, _)) = method_call(parent)
2727
&& eq_expr_value(cx, to, current_to)
2828
&& from_kind == cx.typeck_results().expr_ty(current_from).peel_refs().kind()
2929
{
@@ -48,7 +48,7 @@ fn collect_replace_calls<'tcx>(
4848
let mut from_args = VecDeque::new();
4949

5050
let _: Option<()> = for_each_expr(expr, |e| {
51-
if let Some(("replace", _, [from, to], _)) = method_call(e) {
51+
if let Some(("replace", _, [from, to], _, _)) = method_call(e) {
5252
if eq_expr_value(cx, to_arg, to) && cx.typeck_results().expr_ty(from).peel_refs().is_char() {
5353
methods.push_front(e);
5454
from_args.push_front(from);
@@ -78,7 +78,7 @@ fn check_consecutive_replace_calls<'tcx>(
7878
.collect();
7979
let app = Applicability::MachineApplicable;
8080
let earliest_replace_call = replace_methods.methods.front().unwrap();
81-
if let Some((_, _, [..], span_lo)) = method_call(earliest_replace_call) {
81+
if let Some((_, _, [..], span_lo, _)) = method_call(earliest_replace_call) {
8282
span_lint_and_sugg(
8383
cx,
8484
COLLAPSIBLE_STR_REPLACE,

‎clippy_lints/src/methods/manual_str_repeat.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,8 @@ pub(super) fn check(
5959
if let ExprKind::Call(repeat_fn, [repeat_arg]) = take_self_arg.kind;
6060
if is_path_diagnostic_item(cx, repeat_fn, sym::iter_repeat);
6161
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(collect_expr), sym::String);
62-
if let Some(collect_id) = cx.typeck_results().type_dependent_def_id(collect_expr.hir_id);
6362
if let Some(take_id) = cx.typeck_results().type_dependent_def_id(take_expr.hir_id);
6463
if let Some(iter_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
65-
if cx.tcx.trait_of_item(collect_id) == Some(iter_trait_id);
6664
if cx.tcx.trait_of_item(take_id) == Some(iter_trait_id);
6765
if let Some(repeat_kind) = parse_repeat_arg(cx, repeat_arg);
6866
let ctxt = collect_expr.span.ctxt();

‎clippy_lints/src/methods/map_collect_result_unit.rs

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::is_trait_method;
32
use clippy_utils::source::snippet;
43
use clippy_utils::ty::is_type_diagnostic_item;
54
use if_chain::if_chain;
@@ -11,18 +10,10 @@ use rustc_span::symbol::sym;
1110

1211
use super::MAP_COLLECT_RESULT_UNIT;
1312

14-
pub(super) fn check(
15-
cx: &LateContext<'_>,
16-
expr: &hir::Expr<'_>,
17-
iter: &hir::Expr<'_>,
18-
map_fn: &hir::Expr<'_>,
19-
collect_recv: &hir::Expr<'_>,
20-
) {
13+
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, iter: &hir::Expr<'_>, map_fn: &hir::Expr<'_>) {
14+
// return of collect `Result<(),_>`
15+
let collect_ret_ty = cx.typeck_results().expr_ty(expr);
2116
if_chain! {
22-
// called on Iterator
23-
if is_trait_method(cx, collect_recv, sym::Iterator);
24-
// return of collect `Result<(),_>`
25-
let collect_ret_ty = cx.typeck_results().expr_ty(expr);
2617
if is_type_diagnostic_item(cx, collect_ret_ty, sym::Result);
2718
if let ty::Adt(_, substs) = collect_ret_ty.kind();
2819
if let Some(result_t) = substs.types().next();

‎clippy_lints/src/methods/mod.rs

Lines changed: 48 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -3296,11 +3296,11 @@ impl_lint_pass!(Methods => [
32963296
/// Extracts a method call name, args, and `Span` of the method name.
32973297
fn method_call<'tcx>(
32983298
recv: &'tcx hir::Expr<'tcx>,
3299-
) -> Option<(&'tcx str, &'tcx hir::Expr<'tcx>, &'tcx [hir::Expr<'tcx>], Span)> {
3300-
if let ExprKind::MethodCall(path, receiver, args, _) = recv.kind {
3299+
) -> Option<(&'tcx str, &'tcx hir::Expr<'tcx>, &'tcx [hir::Expr<'tcx>], Span, Span)> {
3300+
if let ExprKind::MethodCall(path, receiver, args, call_span) = recv.kind {
33013301
if !args.iter().any(|e| e.span.from_expansion()) && !receiver.span.from_expansion() {
33023302
let name = path.ident.name.as_str();
3303-
return Some((name, receiver, args, path.ident.span));
3303+
return Some((name, receiver, args, path.ident.span, call_span));
33043304
}
33053305
}
33063306
None
@@ -3341,8 +3341,6 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
33413341
},
33423342
_ => (),
33433343
}
3344-
3345-
needless_collect::check(expr, cx);
33463344
}
33473345

33483346
#[allow(clippy::too_many_lines)]
@@ -3488,7 +3486,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
34883486
impl Methods {
34893487
#[allow(clippy::too_many_lines)]
34903488
fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
3491-
if let Some((name, recv, args, span)) = method_call(expr) {
3489+
if let Some((name, recv, args, span, call_span)) = method_call(expr) {
34923490
match (name, args) {
34933491
("add" | "offset" | "sub" | "wrapping_offset" | "wrapping_add" | "wrapping_sub", [_arg]) => {
34943492
zst_offset::check(cx, expr, recv);
@@ -3507,28 +3505,31 @@ impl Methods {
35073505
("as_ref", []) => useless_asref::check(cx, expr, "as_ref", recv),
35083506
("assume_init", []) => uninit_assumed_init::check(cx, expr, recv),
35093507
("cloned", []) => cloned_instead_of_copied::check(cx, expr, recv, span, self.msrv),
3510-
("collect", []) => match method_call(recv) {
3511-
Some((name @ ("cloned" | "copied"), recv2, [], _)) => {
3512-
iter_cloned_collect::check(cx, name, expr, recv2);
3513-
},
3514-
Some(("map", m_recv, [m_arg], _)) => {
3515-
map_collect_result_unit::check(cx, expr, m_recv, m_arg, recv);
3516-
},
3517-
Some(("take", take_self_arg, [take_arg], _)) => {
3518-
if meets_msrv(self.msrv, msrvs::STR_REPEAT) {
3519-
manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg);
3520-
}
3521-
},
3522-
_ => {},
3508+
("collect", []) if is_trait_method(cx, expr, sym::Iterator) => {
3509+
needless_collect::check(cx, span, expr, recv, call_span);
3510+
match method_call(recv) {
3511+
Some((name @ ("cloned" | "copied"), recv2, [], _, _)) => {
3512+
iter_cloned_collect::check(cx, name, expr, recv2);
3513+
},
3514+
Some(("map", m_recv, [m_arg], _, _)) => {
3515+
map_collect_result_unit::check(cx, expr, m_recv, m_arg);
3516+
},
3517+
Some(("take", take_self_arg, [take_arg], _, _)) => {
3518+
if meets_msrv(self.msrv, msrvs::STR_REPEAT) {
3519+
manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg);
3520+
}
3521+
},
3522+
_ => {},
3523+
}
35233524
},
35243525
("count", []) if is_trait_method(cx, expr, sym::Iterator) => match method_call(recv) {
3525-
Some(("cloned", recv2, [], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, true, false),
3526-
Some((name2 @ ("into_iter" | "iter" | "iter_mut"), recv2, [], _)) => {
3526+
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, true, false),
3527+
Some((name2 @ ("into_iter" | "iter" | "iter_mut"), recv2, [], _, _)) => {
35273528
iter_count::check(cx, expr, recv2, name2);
35283529
},
3529-
Some(("map", _, [arg], _)) => suspicious_map::check(cx, expr, recv, arg),
3530-
Some(("filter", recv2, [arg], _)) => bytecount::check(cx, expr, recv2, arg),
3531-
Some(("bytes", recv2, [], _)) => bytes_count_to_len::check(cx, expr, recv, recv2),
3530+
Some(("map", _, [arg], _, _)) => suspicious_map::check(cx, expr, recv, arg),
3531+
Some(("filter", recv2, [arg], _, _)) => bytecount::check(cx, expr, recv2, arg),
3532+
Some(("bytes", recv2, [], _, _)) => bytes_count_to_len::check(cx, expr, recv, recv2),
35323533
_ => {},
35333534
},
35343535
("drain", [arg]) => {
@@ -3540,8 +3541,8 @@ impl Methods {
35403541
}
35413542
},
35423543
("expect", [_]) => match method_call(recv) {
3543-
Some(("ok", recv, [], _)) => ok_expect::check(cx, expr, recv),
3544-
Some(("err", recv, [], err_span)) => err_expect::check(cx, expr, recv, self.msrv, span, err_span),
3544+
Some(("ok", recv, [], _, _)) => ok_expect::check(cx, expr, recv),
3545+
Some(("err", recv, [], err_span, _)) => err_expect::check(cx, expr, recv, self.msrv, span, err_span),
35453546
_ => expect_used::check(cx, expr, recv, false, self.allow_expect_in_tests),
35463547
},
35473548
("expect_err", [_]) => expect_used::check(cx, expr, recv, true, self.allow_expect_in_tests),
@@ -3561,13 +3562,13 @@ impl Methods {
35613562
flat_map_option::check(cx, expr, arg, span);
35623563
},
35633564
("flatten", []) => match method_call(recv) {
3564-
Some(("map", recv, [map_arg], map_span)) => map_flatten::check(cx, expr, recv, map_arg, map_span),
3565-
Some(("cloned", recv2, [], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, true),
3565+
Some(("map", recv, [map_arg], map_span, _)) => map_flatten::check(cx, expr, recv, map_arg, map_span),
3566+
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, true),
35663567
_ => {},
35673568
},
35683569
("fold", [init, acc]) => unnecessary_fold::check(cx, expr, init, acc, span),
35693570
("for_each", [_]) => {
3570-
if let Some(("inspect", _, [_], span2)) = method_call(recv) {
3571+
if let Some(("inspect", _, [_], span2, _)) = method_call(recv) {
35713572
inspect_for_each::check(cx, expr, span2);
35723573
}
35733574
},
@@ -3587,12 +3588,12 @@ impl Methods {
35873588
iter_on_single_or_empty_collections::check(cx, expr, name, recv);
35883589
},
35893590
("join", [join_arg]) => {
3590-
if let Some(("collect", _, _, span)) = method_call(recv) {
3591+
if let Some(("collect", _, _, span, _)) = method_call(recv) {
35913592
unnecessary_join::check(cx, expr, recv, join_arg, span);
35923593
}
35933594
},
35943595
("last", []) | ("skip", [_]) => {
3595-
if let Some((name2, recv2, args2, _span2)) = method_call(recv) {
3596+
if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
35963597
if let ("cloned", []) = (name2, args2) {
35973598
iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
35983599
}
@@ -3604,13 +3605,13 @@ impl Methods {
36043605
(name @ ("map" | "map_err"), [m_arg]) => {
36053606
if name == "map" {
36063607
map_clone::check(cx, expr, recv, m_arg, self.msrv);
3607-
if let Some((map_name @ ("iter" | "into_iter"), recv2, _, _)) = method_call(recv) {
3608+
if let Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) = method_call(recv) {
36083609
iter_kv_map::check(cx, map_name, expr, recv2, m_arg);
36093610
}
36103611
} else {
36113612
map_err_ignore::check(cx, expr, m_arg);
36123613
}
3613-
if let Some((name, recv2, args, span2)) = method_call(recv) {
3614+
if let Some((name, recv2, args, span2,_)) = method_call(recv) {
36143615
match (name, args) {
36153616
("as_mut", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, true, self.msrv),
36163617
("as_ref", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, false, self.msrv),
@@ -3630,7 +3631,7 @@ impl Methods {
36303631
manual_ok_or::check(cx, expr, recv, def, map);
36313632
},
36323633
("next", []) => {
3633-
if let Some((name2, recv2, args2, _)) = method_call(recv) {
3634+
if let Some((name2, recv2, args2, _, _)) = method_call(recv) {
36343635
match (name2, args2) {
36353636
("cloned", []) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
36363637
("filter", [arg]) => filter_next::check(cx, expr, recv2, arg),
@@ -3643,10 +3644,10 @@ impl Methods {
36433644
}
36443645
},
36453646
("nth", [n_arg]) => match method_call(recv) {
3646-
Some(("bytes", recv2, [], _)) => bytes_nth::check(cx, expr, recv2, n_arg),
3647-
Some(("cloned", recv2, [], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
3648-
Some(("iter", recv2, [], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false),
3649-
Some(("iter_mut", recv2, [], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true),
3647+
Some(("bytes", recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg),
3648+
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
3649+
Some(("iter", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false),
3650+
Some(("iter_mut", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true),
36503651
_ => iter_nth_zero::check(cx, expr, recv, n_arg),
36513652
},
36523653
("ok_or_else", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"),
@@ -3711,7 +3712,7 @@ impl Methods {
37113712
},
37123713
("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
37133714
("take", [_arg]) => {
3714-
if let Some((name2, recv2, args2, _span2)) = method_call(recv) {
3715+
if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
37153716
if let ("cloned", []) = (name2, args2) {
37163717
iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
37173718
}
@@ -3734,13 +3735,13 @@ impl Methods {
37343735
},
37353736
("unwrap", []) => {
37363737
match method_call(recv) {
3737-
Some(("get", recv, [get_arg], _)) => {
3738+
Some(("get", recv, [get_arg], _, _)) => {
37383739
get_unwrap::check(cx, expr, recv, get_arg, false);
37393740
},
3740-
Some(("get_mut", recv, [get_arg], _)) => {
3741+
Some(("get_mut", recv, [get_arg], _, _)) => {
37413742
get_unwrap::check(cx, expr, recv, get_arg, true);
37423743
},
3743-
Some(("or", recv, [or_arg], or_span)) => {
3744+
Some(("or", recv, [or_arg], or_span, _)) => {
37443745
or_then_unwrap::check(cx, expr, recv, or_arg, or_span);
37453746
},
37463747
_ => {},
@@ -3749,19 +3750,19 @@ impl Methods {
37493750
},
37503751
("unwrap_err", []) => unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests),
37513752
("unwrap_or", [u_arg]) => match method_call(recv) {
3752-
Some((arith @ ("checked_add" | "checked_sub" | "checked_mul"), lhs, [rhs], _)) => {
3753+
Some((arith @ ("checked_add" | "checked_sub" | "checked_mul"), lhs, [rhs], _, _)) => {
37533754
manual_saturating_arithmetic::check(cx, expr, lhs, rhs, u_arg, &arith["checked_".len()..]);
37543755
},
3755-
Some(("map", m_recv, [m_arg], span)) => {
3756+
Some(("map", m_recv, [m_arg], span, _)) => {
37563757
option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span);
37573758
},
3758-
Some(("then_some", t_recv, [t_arg], _)) => {
3759+
Some(("then_some", t_recv, [t_arg], _, _)) => {
37593760
obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg);
37603761
},
37613762
_ => {},
37623763
},
37633764
("unwrap_or_else", [u_arg]) => match method_call(recv) {
3764-
Some(("map", recv, [map_arg], _))
3765+
Some(("map", recv, [map_arg], _, _))
37653766
if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, self.msrv) => {},
37663767
_ => {
37673768
unwrap_or_else_default::check(cx, expr, recv, u_arg);
@@ -3782,7 +3783,7 @@ impl Methods {
37823783
}
37833784

37843785
fn check_is_some_is_none(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, is_some: bool) {
3785-
if let Some((name @ ("find" | "position" | "rposition"), f_recv, [arg], span)) = method_call(recv) {
3786+
if let Some((name @ ("find" | "position" | "rposition"), f_recv, [arg], span, _)) = method_call(recv) {
37863787
search_is_some::check(cx, expr, name, is_some, f_recv, arg, recv, span);
37873788
}
37883789
}

0 commit comments

Comments
 (0)