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

merge explicit_into_iter_fn_arg into useless_conversion

Browse source
This commit is contained in:
y21 2023-05-30 22:32:26 +02:00
parent 6a76101d3a
commit de1f410018
11 changed files with 234 additions and 258 deletions
Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -4642,7 +4642,6 @@ Released 2018-09-13
[`explicit_auto_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_auto_deref
[`explicit_counter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_counter_loop
[`explicit_deref_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_methods
[`explicit_into_iter_fn_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_fn_arg
[`explicit_into_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_loop
[`explicit_iter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_iter_loop
[`explicit_write`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_write

1
clippy_lints/src/declared_lints.rs
View file

@ -157,7 +157,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::exhaustive_items::EXHAUSTIVE_ENUMS_INFO,
crate::exhaustive_items::EXHAUSTIVE_STRUCTS_INFO,
crate::exit::EXIT_INFO,
crate::explicit_into_iter_fn_arg::EXPLICIT_INTO_ITER_FN_ARG_INFO,
crate::explicit_write::EXPLICIT_WRITE_INFO,
crate::extra_unused_type_parameters::EXTRA_UNUSED_TYPE_PARAMETERS_INFO,
crate::fallible_impl_from::FALLIBLE_IMPL_FROM_INFO,

142
clippy_lints/src/explicit_into_iter_fn_arg.rs
View file

@ -1,142 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_then;
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;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
use rustc_span::Span;
declare_clippy_lint! {
/// ### What it does
/// Checks for calls to [`IntoIterator::into_iter`](https://doc.rust-lang.org/stable/std/iter/trait.IntoIterator.html#tymethod.into_iter)
/// in a call argument that accepts `IntoIterator`.
///
/// ### Why is this bad?
/// 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 = i32>>(iter: I) -> i32 {
/// iter.into_iter().filter(|&x| x % 2 == 0).sum()
/// }
///
/// let _ = even_sum([1, 2, 3].into_iter());
/// // ^^^^^^^^^^^^ redundant. `[i32; 3]` implements `IntoIterator`
/// ```
/// Use instead:
/// ```rust
/// fn even_sum<I: IntoIterator<Item = i32>>(iter: I) -> i32 {
/// iter.into_iter().filter(|&x| x % 2 == 0).sum()
/// }
///
/// let _ = even_sum([1, 2, 3]);
/// ```
#[clippy::version = "1.71.0"]
pub EXPLICIT_INTO_ITER_FN_ARG,
pedantic,
"explicit call to `.into_iter()` in function argument accepting `IntoIterator`"
}
declare_lint_pass!(ExplicitIntoIterFnArg => [EXPLICIT_INTO_ITER_FN_ARG]);
enum MethodOrFunction {
Method,
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, 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
}
}
impl<'tcx> LateLintPass<'tcx> for ExplicitIntoIterFnArg {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if expr.span.from_expansion() {
return;
}
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 = 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.hir_id)
.map(|did| (did, args, MethodOrFunction::Method))
}
_ => None,
};
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(kind.param_pos(arg_pos))
&& let ty::Param(param) = into_iter_param.kind()
&& let Some(span) = into_iter_bound(cx, parent_fn_did, into_iter_did, param.index)
{
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,
);
diag.span_note(span, "this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`");
});
}
}
}
}

2
clippy_lints/src/lib.rs
View file

@ -123,7 +123,6 @@ mod eta_reduction;
mod excessive_bools;
mod exhaustive_items;
mod exit;
mod explicit_into_iter_fn_arg;
mod explicit_write;
mod extra_unused_type_parameters;
mod fallible_impl_from;
@ -995,7 +994,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| Box::new(ref_patterns::RefPatterns));
store.register_late_pass(|_| Box::new(default_constructed_unit_structs::DefaultConstructedUnitStructs));
store.register_early_pass(|| Box::new(needless_else::NeedlessElse));
store.register_late_pass(|_| Box::new(explicit_into_iter_fn_arg::ExplicitIntoIterFnArg));
// add lints here, do not remove this comment, it's used in `new_lint`
}

121
clippy_lints/src/useless_conversion.rs
View file

@ -1,16 +1,17 @@
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg};
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
use clippy_utils::is_ty_alias;
use clippy_utils::source::{snippet, snippet_with_context};
use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_context};
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{is_copy, is_type_diagnostic_item, same_type_and_consts};
use clippy_utils::{get_parent_expr, is_trait_method, match_def_path, path_to_local, paths};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::def_id::DefId;
use rustc_hir::{BindingAnnotation, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::sym;
use rustc_span::{sym, Span};
declare_clippy_lint! {
/// ### What it does
@ -43,6 +44,65 @@ pub struct UselessConversion {
impl_lint_pass!(UselessConversion => [USELESS_CONVERSION]);
enum MethodOrFunction {
Method,
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, 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
}
})
}
/// Extracts the receiver of a `.into_iter()` method call.
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
}
}
/// Same as [`into_iter_call`], but tries to look for the innermost `.into_iter()` call, e.g.:
/// `foo.into_iter().into_iter()`
/// ^^^ we want this expression
fn into_iter_deep_call<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>) -> Option<&'hir Expr<'hir>> {
loop {
if let Some(recv) = into_iter_call(cx, expr) {
expr = recv;
} else {
return Some(expr);
}
}
}
#[expect(clippy::too_many_lines)]
impl<'tcx> LateLintPass<'tcx> for UselessConversion {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
@ -82,9 +142,58 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
);
}
}
if is_trait_method(cx, e, sym::IntoIterator) && name.ident.name == sym::into_iter {
if get_parent_expr(cx, e).is_some() &&
let Some(id) = path_to_local(recv) &&
if let Some(into_iter_recv) = into_iter_call(cx, e)
// Make sure that there is no parent expression, or if there is, make sure it's not a `.into_iter()` call.
// The reason for that is that we only want to lint once (the outermost call)
// in cases like `foo.into_iter().into_iter()`
&& get_parent_expr(cx, e)
.and_then(|parent| into_iter_call(cx, parent))
.is_none()
{
if let Some(parent) = get_parent_expr(cx, e) {
let parent_fn = 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.hir_id)
.map(|did| (did, args, MethodOrFunction::Method))
}
_ => None,
};
if let Some((parent_fn_did, args, kind)) = parent_fn
&& let Some(into_iter_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator)
&& let sig = cx.tcx.fn_sig(parent_fn_did).skip_binder().skip_binder()
&& let Some(arg_pos) = args.iter().position(|x| x.hir_id == e.hir_id)
&& 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, into_iter_did, param.index)
// Get the "innermost" `.into_iter()` call, e.g. given this expression:
// `foo.into_iter().into_iter()`
// ^^^
// We want this span
&& let Some(into_iter_recv) = into_iter_deep_call(cx, into_iter_recv)
{
let mut applicability = Applicability::MachineApplicable;
let sugg = snippet_with_applicability(cx, into_iter_recv.span.source_callsite(), "<expr>", &mut applicability).into_owned();
span_lint_and_then(cx, USELESS_CONVERSION, e.span, "explicit call to `.into_iter()` in function argument accepting `IntoIterator`", |diag| {
diag.span_suggestion(
e.span,
"consider removing `.into_iter()`",
sugg,
applicability,
);
diag.span_note(span, "this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`");
});
// Early return to avoid linting again with contradicting suggestions
return;
}
}
if let Some(id) = path_to_local(recv) &&
let Node::Pat(pat) = cx.tcx.hir().get(id) &&
let PatKind::Binding(ann, ..) = pat.kind &&
ann != BindingAnnotation::MUT

33
tests/ui/explicit_into_iter_fn_arg.fixed
View file

@ -1,33 +0,0 @@
//@run-rustfix
#![allow(unused, clippy::useless_conversion)]
#![warn(clippy::explicit_into_iter_fn_arg)]
fn a<T>(_: T) {}
fn b<T: IntoIterator<Item = i32>>(_: T) {}
fn c(_: impl IntoIterator<Item = i32>) {}
fn d<T>(_: T)
where
T: IntoIterator<Item = i32>,
{
}
fn f(_: std::vec::IntoIter<i32>) {}
fn main() {
a(vec![1, 2].into_iter());
b(vec![1, 2]);
c(vec![1, 2]);
d(vec![1, 2]);
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!());
}

33
tests/ui/explicit_into_iter_fn_arg.rs
View file

@ -1,33 +0,0 @@
//@run-rustfix
#![allow(unused, clippy::useless_conversion)]
#![warn(clippy::explicit_into_iter_fn_arg)]
fn a<T>(_: T) {}
fn b<T: IntoIterator<Item = i32>>(_: T) {}
fn c(_: impl IntoIterator<Item = i32>) {}
fn d<T>(_: T)
where
T: IntoIterator<Item = i32>,
{
}
fn f(_: std::vec::IntoIter<i32>) {}
fn main() {
a(vec![1, 2].into_iter());
b(vec![1, 2].into_iter());
c(vec![1, 2].into_iter());
d(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!());
}

39
tests/ui/explicit_into_iter_fn_arg.stderr
View file

@ -1,39 +0,0 @@
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
--> $DIR/explicit_into_iter_fn_arg.rs:18:7
|
LL | b(vec![1, 2].into_iter());
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
--> $DIR/explicit_into_iter_fn_arg.rs:7:9
|
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:19:7
|
LL | c(vec![1, 2].into_iter());
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
--> $DIR/explicit_into_iter_fn_arg.rs:8:14
|
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:20:7
|
LL | d(vec![1, 2].into_iter());
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
--> $DIR/explicit_into_iter_fn_arg.rs:11:8
|
LL | T: IntoIterator<Item = i32>,
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 3 previous errors

29
tests/ui/useless_conversion.fixed
View file

@ -155,6 +155,35 @@ fn main() {
let _ = vec![s4, s4, s4].into_iter();
}
#[allow(dead_code)]
fn explicit_into_iter_fn_arg() {
fn a<T>(_: T) {}
fn b<T: IntoIterator<Item = i32>>(_: T) {}
fn c(_: impl IntoIterator<Item = i32>) {}
fn d<T>(_: T)
where
T: IntoIterator<Item = i32>,
{
}
fn f(_: std::vec::IntoIter<i32>) {}
a(vec![1, 2].into_iter());
b(vec![1, 2]);
c(vec![1, 2]);
d(vec![1, 2]);
b([&1, &2, &3].into_iter().cloned());
b(vec![1, 2]);
b(vec![1, 2]);
macro_rules! macro_generated {
() => {
vec![1, 2].into_iter()
};
}
b(macro_generated!());
}
#[derive(Copy, Clone)]
struct Foo<const C: char>;

29
tests/ui/useless_conversion.rs
View file

@ -155,6 +155,35 @@ fn main() {
let _ = vec![s4, s4, s4].into_iter().into_iter();
}
#[allow(dead_code)]
fn explicit_into_iter_fn_arg() {
fn a<T>(_: T) {}
fn b<T: IntoIterator<Item = i32>>(_: T) {}
fn c(_: impl IntoIterator<Item = i32>) {}
fn d<T>(_: T)
where
T: IntoIterator<Item = i32>,
{
}
fn f(_: std::vec::IntoIter<i32>) {}
a(vec![1, 2].into_iter());
b(vec![1, 2].into_iter());
c(vec![1, 2].into_iter());
d(vec![1, 2].into_iter());
b([&1, &2, &3].into_iter().cloned());
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!());
}
#[derive(Copy, Clone)]
struct Foo<const C: char>;

62
tests/ui/useless_conversion.stderr
View file

@ -118,5 +118,65 @@ error: useless conversion to the same type: `std::vec::IntoIter<Foo<'a'>>`
LL | let _ = vec![s4, s4, s4].into_iter().into_iter();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![s4, s4, s4].into_iter()`
error: aborting due to 19 previous errors
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
--> $DIR/useless_conversion.rs:171:7
|
LL | b(vec![1, 2].into_iter());
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
--> $DIR/useless_conversion.rs:161:13
|
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
--> $DIR/useless_conversion.rs:172:7
|
LL | c(vec![1, 2].into_iter());
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
--> $DIR/useless_conversion.rs:162:18
|
LL | fn c(_: impl IntoIterator<Item = i32>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
--> $DIR/useless_conversion.rs:173:7
|
LL | d(vec![1, 2].into_iter());
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
--> $DIR/useless_conversion.rs:165:12
|
LL | T: IntoIterator<Item = i32>,
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
--> $DIR/useless_conversion.rs:176:7
|
LL | b(vec![1, 2].into_iter().into_iter());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
--> $DIR/useless_conversion.rs:161:13
|
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: explicit call to `.into_iter()` in function argument accepting `IntoIterator`
--> $DIR/useless_conversion.rs:177:7
|
LL | b(vec![1, 2].into_iter().into_iter().into_iter());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2]`
|
note: this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`
--> $DIR/useless_conversion.rs:161:13
|
LL | fn b<T: IntoIterator<Item = i32>>(_: T) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 24 previous errors