Skip to content

Commit 516f4f6

Browse files
committed
new lint: explicit_into_iter_fn_arg
1 parent 5187e92 commit 516f4f6

File tree

7 files changed

+235
-0
lines changed

7 files changed

+235
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4642,6 +4642,7 @@ Released 2018-09-13
46424642
[`explicit_auto_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref
46434643
[`explicit_counter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_counter_loop
46444644
[`explicit_deref_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_methods
4645+
[`explicit_into_iter_fn_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_fn_arg
46454646
[`explicit_into_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_loop
46464647
[`explicit_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_iter_loop
46474648
[`explicit_write`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_write

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
157157
crate::exhaustive_items::EXHAUSTIVE_ENUMS_INFO,
158158
crate::exhaustive_items::EXHAUSTIVE_STRUCTS_INFO,
159159
crate::exit::EXIT_INFO,
160+
crate::explicit_into_iter_fn_arg::EXPLICIT_INTO_ITER_FN_ARG_INFO,
160161
crate::explicit_write::EXPLICIT_WRITE_INFO,
161162
crate::extra_unused_type_parameters::EXTRA_UNUSED_TYPE_PARAMETERS_INFO,
162163
crate::fallible_impl_from::FALLIBLE_IMPL_FROM_INFO,
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::source::snippet;
3+
use clippy_utils::{get_parent_expr, is_trait_method};
4+
use rustc_errors::Applicability;
5+
use rustc_hir::def_id::DefId;
6+
use rustc_hir::*;
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_middle::ty;
9+
use rustc_session::{declare_lint_pass, declare_tool_lint};
10+
use rustc_span::sym;
11+
use rustc_span::Span;
12+
13+
declare_clippy_lint! {
14+
/// ### What it does
15+
/// Checks for calls to [`IntoIterator::into_iter`](https://doc.rust-lang.org/stable/std/iter/trait.IntoIterator.html#tymethod.into_iter)
16+
/// in a call argument that accepts `IntoIterator`.
17+
///
18+
/// ### Why is this bad?
19+
/// If a function has a generic parameter with an `IntoIterator` trait bound, it means that the function
20+
/// will *have* to call `.into_iter()` to get an iterator out of it, thereby making the call to `.into_iter()`
21+
/// at call site redundant.
22+
///
23+
/// Consider this example:
24+
/// ```rs,ignore
25+
/// fn foo<T: IntoIterator<Item = u32>>(iter: T) {
26+
/// let it = iter.into_iter();
27+
/// ^^^^^^^^^^^^ the function has to call `.into_iter()` to get the iterator
28+
/// }
29+
///
30+
/// foo(vec![1, 2, 3].into_iter());
31+
/// ^^^^^^^^^^^^ ... making this `.into_iter()` call redundant.
32+
/// ```
33+
///
34+
/// The reason for why calling `.into_iter()` twice (once at call site and again inside of the function) works in the first place
35+
/// is because there is a blanket implementation of `IntoIterator` for all types that implement `Iterator` in the standard library,
36+
/// in which it simply returns itself, effectively making the second call to `.into_iter()` a "no-op":
37+
/// ```rust,ignore
38+
/// impl<I: Iterator> IntoIterator for I {
39+
/// type Item = I::Item;
40+
/// type IntoIter = I;
41+
///
42+
/// fn into_iter(self) -> I {
43+
/// self
44+
/// }
45+
/// }
46+
/// ```
47+
///
48+
/// ### Example
49+
/// ```rust
50+
/// fn even_sum<I: IntoIterator<Item = u32>>(iter: I) -> u32 {
51+
/// iter.into_iter().filter(|&x| x % 2 == 0).sum()
52+
/// }
53+
///
54+
/// let _ = even_sum(vec![1, 2, 3].into_iter());
55+
/// ```
56+
/// Use instead:
57+
/// ```rust
58+
/// fn even_sum<I: IntoIterator<Item = u32>>(iter: I) -> u32 {
59+
/// iter.into_iter().filter(|&x| x % 2 == 0).sum()
60+
/// }
61+
///
62+
/// let _ = even_sum(vec![1, 2, 3]);
63+
/// ```
64+
#[clippy::version = "1.71.0"]
65+
pub EXPLICIT_INTO_ITER_FN_ARG,
66+
pedantic,
67+
"explicit call to `.into_iter()` in function argument accepting `IntoIterator`"
68+
}
69+
declare_lint_pass!(ExplicitIntoIterFnArg => [EXPLICIT_INTO_ITER_FN_ARG]);
70+
71+
enum MethodOrFunction {
72+
Method,
73+
Function,
74+
}
75+
76+
/// Returns the span of the `IntoIterator` trait bound in the function pointed to by `fn_did`
77+
fn into_iter_bound(cx: &LateContext<'_>, fn_did: DefId, param_index: u32) -> Option<Span> {
78+
if let Some(into_iter_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator) {
79+
cx.tcx
80+
.predicates_of(fn_did)
81+
.predicates
82+
.iter()
83+
.find_map(|&(ref pred, span)| {
84+
if let ty::PredicateKind::Clause(ty::Clause::Trait(tr)) = pred.kind().skip_binder()
85+
&& tr.def_id() == into_iter_did
86+
&& tr.self_ty().is_param(param_index)
87+
{
88+
Some(span)
89+
} else {
90+
None
91+
}
92+
})
93+
} else {
94+
None
95+
}
96+
}
97+
98+
impl<'tcx> LateLintPass<'tcx> for ExplicitIntoIterFnArg {
99+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
100+
if expr.span.from_expansion() {
101+
return;
102+
}
103+
104+
if let ExprKind::MethodCall(name, recv, ..) = expr.kind
105+
&& is_trait_method(cx, expr, sym::IntoIterator)
106+
&& name.ident.name == sym::into_iter
107+
&& let Some(parent_expr) = get_parent_expr(cx, expr)
108+
{
109+
let parent_expr = match parent_expr.kind {
110+
ExprKind::Call(recv, args) if let ExprKind::Path(ref qpath) = recv.kind => {
111+
cx.qpath_res(qpath, recv.hir_id).opt_def_id()
112+
.map(|did| (did, args, MethodOrFunction::Function))
113+
}
114+
ExprKind::MethodCall(.., args, _) => {
115+
cx.typeck_results().type_dependent_def_id(parent_expr.hir_id)
116+
.map(|did| (did, args, MethodOrFunction::Method))
117+
}
118+
_ => None,
119+
};
120+
121+
if let Some((parent_fn_did, args, kind)) = parent_expr
122+
&& let sig = cx.tcx.fn_sig(parent_fn_did).skip_binder().skip_binder()
123+
&& let Some(arg_pos) = args.iter().position(|x| x.hir_id == expr.hir_id)
124+
&& let Some(&into_iter_param) = sig.inputs().get(match kind {
125+
MethodOrFunction::Function => arg_pos,
126+
MethodOrFunction::Method => arg_pos + 1, // skip self arg
127+
})
128+
&& let ty::Param(param) = into_iter_param.kind()
129+
&& let Some(span) = into_iter_bound(cx, parent_fn_did, param.index)
130+
{
131+
let sugg = snippet(cx, recv.span.source_callsite(), "<expr>").into_owned();
132+
span_lint_and_then(cx, EXPLICIT_INTO_ITER_FN_ARG, expr.span, "explicit call to `.into_iter()` in function argument accepting `IntoIterator`", |diag| {
133+
diag.span_suggestion(
134+
expr.span,
135+
"consider removing `.into_iter()`",
136+
sugg,
137+
Applicability::MachineApplicable,
138+
);
139+
diag.span_note(span, "this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`");
140+
});
141+
}
142+
}
143+
}
144+
}

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ mod eta_reduction;
123123
mod excessive_bools;
124124
mod exhaustive_items;
125125
mod exit;
126+
mod explicit_into_iter_fn_arg;
126127
mod explicit_write;
127128
mod extra_unused_type_parameters;
128129
mod fallible_impl_from;
@@ -994,6 +995,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
994995
store.register_early_pass(|| Box::new(ref_patterns::RefPatterns));
995996
store.register_late_pass(|_| Box::new(default_constructed_unit_structs::DefaultConstructedUnitStructs));
996997
store.register_early_pass(|| Box::new(needless_else::NeedlessElse));
998+
store.register_late_pass(|_| Box::new(explicit_into_iter_fn_arg::ExplicitIntoIterFnArg));
997999
// add lints here, do not remove this comment, it's used in `new_lint`
9981000
}
9991001

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//@run-rustfix
2+
3+
#![allow(unused)]
4+
#![warn(clippy::explicit_into_iter_fn_arg)]
5+
6+
fn a<T>(_: T) {}
7+
fn b<T: IntoIterator<Item = i32>>(_: T) {}
8+
fn c(_: impl IntoIterator<Item = i32>) {}
9+
fn d<T>(_: T)
10+
where
11+
T: IntoIterator<Item = i32>,
12+
{
13+
}
14+
fn e<T: IntoIterator<Item = i32>>(_: T) {}
15+
fn f(_: std::vec::IntoIter<i32>) {}
16+
17+
fn main() {
18+
a(vec![1, 2].into_iter());
19+
b(vec![1, 2]);
20+
c(vec![1, 2]);
21+
d(vec![1, 2]);
22+
e([&1, &2, &3].into_iter().cloned());
23+
f(vec![1, 2].into_iter());
24+
}

tests/ui/explicit_into_iter_fn_arg.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//@run-rustfix
2+
3+
#![allow(unused)]
4+
#![warn(clippy::explicit_into_iter_fn_arg)]
5+
6+
fn a<T>(_: T) {}
7+
fn b<T: IntoIterator<Item = i32>>(_: T) {}
8+
fn c(_: impl IntoIterator<Item = i32>) {}
9+
fn d<T>(_: T)
10+
where
11+
T: IntoIterator<Item = i32>,
12+
{
13+
}
14+
fn e<T: IntoIterator<Item = i32>>(_: T) {}
15+
fn f(_: std::vec::IntoIter<i32>) {}
16+
17+
fn main() {
18+
a(vec![1, 2].into_iter());
19+
b(vec![1, 2].into_iter());
20+
c(vec![1, 2].into_iter());
21+
d(vec![1, 2].into_iter());
22+
e([&1, &2, &3].into_iter().cloned());
23+
f(vec![1, 2].into_iter());
24+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
2+
--> $DIR/explicit_into_iter_fn_arg.rs:19:7
3+
|
4+
LL | b(vec![1, 2].into_iter());
5+
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
6+
|
7+
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
8+
--> $DIR/explicit_into_iter_fn_arg.rs:7:9
9+
|
10+
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^
12+
= note: `-D clippy::explicit-into-iter-fn-arg` implied by `-D warnings`
13+
14+
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
15+
--> $DIR/explicit_into_iter_fn_arg.rs:20:7
16+
|
17+
LL | c(vec![1, 2].into_iter());
18+
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
19+
|
20+
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
21+
--> $DIR/explicit_into_iter_fn_arg.rs:8:14
22+
|
23+
LL | fn c(_: impl IntoIterator<Item = i32>) {}
24+
| ^^^^^^^^^^^^^^^^^^^^^^^^
25+
26+
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
27+
--> $DIR/explicit_into_iter_fn_arg.rs:21:7
28+
|
29+
LL | d(vec![1, 2].into_iter());
30+
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
31+
|
32+
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
33+
--> $DIR/explicit_into_iter_fn_arg.rs:11:8
34+
|
35+
LL | T: IntoIterator<Item = i32>,
36+
| ^^^^^^^^^^^^^^^^^^^^^^^^
37+
38+
error: aborting due to 3 previous errors
39+

0 commit comments

Comments
 (0)