user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

update docs, simplify arg->param map, dont lint on chain

Browse source
This commit is contained in:
y21 2023-05-24 15:56:28 +02:00
parent 0181d772a4
commit 6a76101d3a
4 changed files with 90 additions and 74 deletions
Download patch file Download diff file Expand all files Collapse all files

124
clippy_lints/src/explicit_into_iter_fn_arg.rs
View file

@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{get_parent_expr, is_trait_method};
use rustc_errors::Applicability;
use rustc_hir::def_id::DefId;
@ -16,50 +16,26 @@ declare_clippy_lint! {
/// in a call argument that accepts `IntoIterator`.
///
/// ### Why is this bad?
/// If a function has a generic parameter with an `IntoIterator` trait bound, it means that the function
/// will *have* to call `.into_iter()` to get an iterator out of it, thereby making the call to `.into_iter()`
/// at call site redundant.
///
/// Consider this example:
/// ```rs,ignore
/// fn foo<T: IntoIterator<Item = u32>>(iter: T) {
/// let it = iter.into_iter();
/// ^^^^^^^^^^^^ the function has to call `.into_iter()` to get the iterator
/// }
///
/// foo(vec![1, 2, 3].into_iter());
/// ^^^^^^^^^^^^ ... making this `.into_iter()` call redundant.
/// ```
///
/// The reason for why calling `.into_iter()` twice (once at call site and again inside of the function) works in the first place
/// is because there is a blanket implementation of `IntoIterator` for all types that implement `Iterator` in the standard library,
/// in which it simply returns itself, effectively making the second call to `.into_iter()` a "no-op":
/// ```rust,ignore
/// impl<I: Iterator> IntoIterator for I {
/// type Item = I::Item;
/// type IntoIter = I;
///
/// fn into_iter(self) -> I {
/// self
/// }
/// }
/// ```
/// If a generic parameter has an `IntoIterator` bound, there is no need to call `.into_iter()` at call site.
/// Calling `IntoIterator::into_iter()` on a value implies that its type already implements `IntoIterator`,
/// so you can just pass the value as is.
///
/// ### Example
/// ```rust
/// fn even_sum<I: IntoIterator<Item = u32>>(iter: I) -> u32 {
/// fn even_sum<I: IntoIterator<Item = i32>>(iter: I) -> i32 {
/// iter.into_iter().filter(|&x| x % 2 == 0).sum()
/// }
///
/// let _ = even_sum(vec![1, 2, 3].into_iter());
/// let _ = even_sum([1, 2, 3].into_iter());
/// // ^^^^^^^^^^^^ redundant. `[i32; 3]` implements `IntoIterator`
/// ```
/// Use instead:
/// ```rust
/// fn even_sum<I: IntoIterator<Item = u32>>(iter: I) -> u32 {
/// fn even_sum<I: IntoIterator<Item = i32>>(iter: I) -> i32 {
/// iter.into_iter().filter(|&x| x % 2 == 0).sum()
/// }
///
/// let _ = even_sum(vec![1, 2, 3]);
/// let _ = even_sum([1, 2, 3]);
/// ```
#[clippy::version = "1.71.0"]
pub EXPLICIT_INTO_ITER_FN_ARG,
@ -73,23 +49,41 @@ enum MethodOrFunction {
Function,
}
impl MethodOrFunction {
/// Maps the argument position in `pos` to the parameter position.
/// For methods, `self` is skipped.
fn param_pos(self, pos: usize) -> usize {
match self {
MethodOrFunction::Method => pos + 1,
MethodOrFunction::Function => pos,
}
}
}
/// Returns the span of the `IntoIterator` trait bound in the function pointed to by `fn_did`
fn into_iter_bound(cx: &LateContext<'_>, fn_did: DefId, param_index: u32) -> Option<Span> {
if let Some(into_iter_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator) {
cx.tcx
.predicates_of(fn_did)
.predicates
.iter()
.find_map(|&(ref pred, span)| {
if let ty::PredicateKind::Clause(ty::Clause::Trait(tr)) = pred.kind().skip_binder()
&& tr.def_id() == into_iter_did
&& tr.self_ty().is_param(param_index)
{
Some(span)
} else {
None
}
})
fn into_iter_bound(cx: &LateContext<'_>, fn_did: DefId, into_iter_did: DefId, param_index: u32) -> Option<Span> {
cx.tcx
.predicates_of(fn_did)
.predicates
.iter()
.find_map(|&(ref pred, span)| {
if let ty::PredicateKind::Clause(ty::Clause::Trait(tr)) = pred.kind().skip_binder()
&& tr.def_id() == into_iter_did
&& tr.self_ty().is_param(param_index)
{
Some(span)
} else {
None
}
})
}
fn into_iter_call<'hir>(cx: &LateContext<'_>, expr: &'hir Expr<'hir>) -> Option<&'hir Expr<'hir>> {
if let ExprKind::MethodCall(name, recv, _, _) = expr.kind
&& is_trait_method(cx, expr, sym::IntoIterator)
&& name.ident.name == sym::into_iter
{
Some(recv)
} else {
None
}
@ -101,40 +95,44 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitIntoIterFnArg {
return;
}
if let ExprKind::MethodCall(name, recv, ..) = expr.kind
&& is_trait_method(cx, expr, sym::IntoIterator)
&& name.ident.name == sym::into_iter
&& let Some(parent_expr) = get_parent_expr(cx, expr)
if let Some(recv) = into_iter_call(cx, expr)
&& let Some(parent) = get_parent_expr(cx, expr)
// Make sure that this is not a chained into_iter call (i.e. `x.into_iter().into_iter()`)
// That case is already covered by `useless_conversion` and we don't want to lint twice
// with two contradicting suggestions.
&& into_iter_call(cx, parent).is_none()
&& into_iter_call(cx, recv).is_none()
&& let Some(into_iter_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator)
{
let parent_expr = match parent_expr.kind {
let parent = match parent.kind {
ExprKind::Call(recv, args) if let ExprKind::Path(ref qpath) = recv.kind => {
cx.qpath_res(qpath, recv.hir_id).opt_def_id()
.map(|did| (did, args, MethodOrFunction::Function))
}
ExprKind::MethodCall(.., args, _) => {
cx.typeck_results().type_dependent_def_id(parent_expr.hir_id)
cx.typeck_results().type_dependent_def_id(parent.hir_id)
.map(|did| (did, args, MethodOrFunction::Method))
}
_ => None,
};
if let Some((parent_fn_did, args, kind)) = parent_expr
if let Some((parent_fn_did, args, kind)) = parent
&& let sig = cx.tcx.fn_sig(parent_fn_did).skip_binder().skip_binder()
&& let Some(arg_pos) = args.iter().position(|x| x.hir_id == expr.hir_id)
&& let Some(&into_iter_param) = sig.inputs().get(match kind {
MethodOrFunction::Function => arg_pos,
MethodOrFunction::Method => arg_pos + 1, // skip self arg
})
&& let Some(&into_iter_param) = sig.inputs().get(kind.param_pos(arg_pos))
&& let ty::Param(param) = into_iter_param.kind()
&& let Some(span) = into_iter_bound(cx, parent_fn_did, param.index)
&& let Some(span) = into_iter_bound(cx, parent_fn_did, into_iter_did, param.index)
{
let sugg = snippet(cx, recv.span.source_callsite(), "<expr>").into_owned();
let mut applicability = Applicability::MachineApplicable;
let sugg = snippet_with_applicability(cx, recv.span.source_callsite(), "<expr>", &mut applicability).into_owned();
span_lint_and_then(cx, EXPLICIT_INTO_ITER_FN_ARG, expr.span, "explicit call to `.into_iter()` in function argument accepting `IntoIterator`", |diag| {
diag.span_suggestion(
expr.span,
"consider removing `.into_iter()`",
sugg,
Applicability::MachineApplicable,
applicability,
);
diag.span_note(span, "this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`");
});

17
tests/ui/explicit_into_iter_fn_arg.fixed
View file

@ -1,6 +1,6 @@
//@run-rustfix
#![allow(unused)]
#![allow(unused, clippy::useless_conversion)]
#![warn(clippy::explicit_into_iter_fn_arg)]
fn a<T>(_: T) {}
@ -11,7 +11,6 @@ where
T: IntoIterator<Item = i32>,
{
}
fn e<T: IntoIterator<Item = i32>>(_: T) {}
fn f(_: std::vec::IntoIter<i32>) {}
fn main() {
@ -19,6 +18,16 @@ fn main() {
b(vec![1, 2]);
c(vec![1, 2]);
d(vec![1, 2]);
e([&1, &2, &3].into_iter().cloned());
f(vec![1, 2].into_iter());
b([&1, &2, &3].into_iter().cloned());
// Don't lint chained `.into_iter().into_iter()` calls. Covered by useless_conversion.
b(vec![1, 2].into_iter().into_iter());
b(vec![1, 2].into_iter().into_iter().into_iter());
macro_rules! macro_generated {
() => {
vec![1, 2].into_iter()
};
}
b(macro_generated!());
}

17
tests/ui/explicit_into_iter_fn_arg.rs
View file

@ -1,6 +1,6 @@
//@run-rustfix
#![allow(unused)]
#![allow(unused, clippy::useless_conversion)]
#![warn(clippy::explicit_into_iter_fn_arg)]
fn a<T>(_: T) {}
@ -11,7 +11,6 @@ where
T: IntoIterator<Item = i32>,
{
}
fn e<T: IntoIterator<Item = i32>>(_: T) {}
fn f(_: std::vec::IntoIter<i32>) {}
fn main() {
@ -19,6 +18,16 @@ fn main() {
b(vec![1, 2].into_iter());
c(vec![1, 2].into_iter());
d(vec![1, 2].into_iter());
e([&1, &2, &3].into_iter().cloned());
f(vec![1, 2].into_iter());
b([&1, &2, &3].into_iter().cloned());
// Don't lint chained `.into_iter().into_iter()` calls. Covered by useless_conversion.
b(vec![1, 2].into_iter().into_iter());
b(vec![1, 2].into_iter().into_iter().into_iter());
macro_rules! macro_generated {
() => {
vec![1, 2].into_iter()
};
}
b(macro_generated!());
}

6
tests/ui/explicit_into_iter_fn_arg.stderr
View file

@ -1,5 +1,5 @@
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
--> $DIR/explicit_into_iter_fn_arg.rs:19:7
--> $DIR/explicit_into_iter_fn_arg.rs:18:7
|
LL | b(vec![1, 2].into_iter());
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
@ -12,7 +12,7 @@ LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
= note: `-D clippy::explicit-into-iter-fn-arg` implied by `-D warnings`
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
--> $DIR/explicit_into_iter_fn_arg.rs:20:7
--> $DIR/explicit_into_iter_fn_arg.rs:19:7
|
LL | c(vec![1, 2].into_iter());
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
@ -24,7 +24,7 @@ LL | fn c(_: impl IntoIterator<Item = i32>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
--> $DIR/explicit_into_iter_fn_arg.rs:21:7
--> $DIR/explicit_into_iter_fn_arg.rs:20:7
|
LL | d(vec![1, 2].into_iter());
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`