From 516f4f6aef224137a84f4033935f5e9cb359b273 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Mon, 22 May 2023 23:52:37 +0200 Subject: [PATCH 1/5] new lint: `explicit_into_iter_fn_arg` --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/explicit_into_iter_fn_arg.rs | 144 ++++++++++++++++++ clippy_lints/src/lib.rs | 2 + tests/ui/explicit_into_iter_fn_arg.fixed | 24 +++ tests/ui/explicit_into_iter_fn_arg.rs | 24 +++ tests/ui/explicit_into_iter_fn_arg.stderr | 39 +++++ 7 files changed, 235 insertions(+) create mode 100644 clippy_lints/src/explicit_into_iter_fn_arg.rs create mode 100644 tests/ui/explicit_into_iter_fn_arg.fixed create mode 100644 tests/ui/explicit_into_iter_fn_arg.rs create mode 100644 tests/ui/explicit_into_iter_fn_arg.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index a2648d9faa6..7a127c08197 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4642,6 +4642,7 @@ 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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 4ade25e1257..fceb6e4b2ae 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -157,6 +157,7 @@ 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, diff --git a/clippy_lints/src/explicit_into_iter_fn_arg.rs b/clippy_lints/src/explicit_into_iter_fn_arg.rs new file mode 100644 index 00000000000..007a3f43449 --- /dev/null +++ b/clippy_lints/src/explicit_into_iter_fn_arg.rs @@ -0,0 +1,144 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::snippet; +use clippy_utils::{get_parent_expr, is_trait_method}; +use rustc_errors::Applicability; +use rustc_hir::def_id::DefId; +use rustc_hir::*; +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 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>(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 IntoIterator for I { + /// type Item = I::Item; + /// type IntoIter = I; + /// + /// fn into_iter(self) -> I { + /// self + /// } + /// } + /// ``` + /// + /// ### Example + /// ```rust + /// fn even_sum>(iter: I) -> u32 { + /// iter.into_iter().filter(|&x| x % 2 == 0).sum() + /// } + /// + /// let _ = even_sum(vec![1, 2, 3].into_iter()); + /// ``` + /// Use instead: + /// ```rust + /// fn even_sum>(iter: I) -> u32 { + /// iter.into_iter().filter(|&x| x % 2 == 0).sum() + /// } + /// + /// let _ = even_sum(vec![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, +} + +/// 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 { + 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 + } + }) + } 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 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) + { + let parent_expr = match parent_expr.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) + .map(|did| (did, args, MethodOrFunction::Method)) + } + _ => None, + }; + + if let Some((parent_fn_did, args, kind)) = parent_expr + && 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 ty::Param(param) = into_iter_param.kind() + && let Some(span) = into_iter_bound(cx, parent_fn_did, param.index) + { + let sugg = snippet(cx, recv.span.source_callsite(), "").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, + ); + diag.span_note(span, "this parameter accepts any `IntoIterator`, so you don't need to call `.into_iter()`"); + }); + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 10404d51ba5..bfe52e182b2 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -123,6 +123,7 @@ 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; @@ -994,6 +995,7 @@ 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` } diff --git a/tests/ui/explicit_into_iter_fn_arg.fixed b/tests/ui/explicit_into_iter_fn_arg.fixed new file mode 100644 index 00000000000..219b60bd051 --- /dev/null +++ b/tests/ui/explicit_into_iter_fn_arg.fixed @@ -0,0 +1,24 @@ +//@run-rustfix + +#![allow(unused)] +#![warn(clippy::explicit_into_iter_fn_arg)] + +fn a(_: T) {} +fn b>(_: T) {} +fn c(_: impl IntoIterator) {} +fn d(_: T) +where + T: IntoIterator, +{ +} +fn e>(_: T) {} +fn f(_: std::vec::IntoIter) {} + +fn main() { + a(vec![1, 2].into_iter()); + 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()); +} diff --git a/tests/ui/explicit_into_iter_fn_arg.rs b/tests/ui/explicit_into_iter_fn_arg.rs new file mode 100644 index 00000000000..358f4bc661d --- /dev/null +++ b/tests/ui/explicit_into_iter_fn_arg.rs @@ -0,0 +1,24 @@ +//@run-rustfix + +#![allow(unused)] +#![warn(clippy::explicit_into_iter_fn_arg)] + +fn a(_: T) {} +fn b>(_: T) {} +fn c(_: impl IntoIterator) {} +fn d(_: T) +where + T: IntoIterator, +{ +} +fn e>(_: T) {} +fn f(_: std::vec::IntoIter) {} + +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()); + e([&1, &2, &3].into_iter().cloned()); + f(vec![1, 2].into_iter()); +} diff --git a/tests/ui/explicit_into_iter_fn_arg.stderr b/tests/ui/explicit_into_iter_fn_arg.stderr new file mode 100644 index 00000000000..a0bd81c21ea --- /dev/null +++ b/tests/ui/explicit_into_iter_fn_arg.stderr @@ -0,0 +1,39 @@ +error: explicit call to `.into_iter()` in function argument accepting `IntoIterator` + --> $DIR/explicit_into_iter_fn_arg.rs:19: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) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ + = 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 + | +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) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: explicit call to `.into_iter()` in function argument accepting `IntoIterator` + --> $DIR/explicit_into_iter_fn_arg.rs:21: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, + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + From 0181d772a4a9702c3ef2fbe5b1b0bf0d94b8fcd7 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 23 May 2023 16:56:13 +0200 Subject: [PATCH 2/5] dogfood --- clippy_lints/src/booleans.rs | 2 +- clippy_lints/src/explicit_into_iter_fn_arg.rs | 2 +- clippy_lints/src/loops/manual_memcpy.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/booleans.rs b/clippy_lints/src/booleans.rs index 455f0df7cd0..a08113bf9e6 100644 --- a/clippy_lints/src/booleans.rs +++ b/clippy_lints/src/booleans.rs @@ -441,7 +441,7 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> { diag.span_suggestions( e.span, "try", - suggestions.into_iter(), + suggestions, // nonminimal_bool can produce minimal but // not human readable expressions (#3141) Applicability::Unspecified, diff --git a/clippy_lints/src/explicit_into_iter_fn_arg.rs b/clippy_lints/src/explicit_into_iter_fn_arg.rs index 007a3f43449..a56d72f0dea 100644 --- a/clippy_lints/src/explicit_into_iter_fn_arg.rs +++ b/clippy_lints/src/explicit_into_iter_fn_arg.rs @@ -3,7 +3,7 @@ use clippy_utils::source::snippet; use clippy_utils::{get_parent_expr, is_trait_method}; use rustc_errors::Applicability; use rustc_hir::def_id::DefId; -use rustc_hir::*; +use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; diff --git a/clippy_lints/src/loops/manual_memcpy.rs b/clippy_lints/src/loops/manual_memcpy.rs index d4c3f76b864..7d1f8ef29c8 100644 --- a/clippy_lints/src/loops/manual_memcpy.rs +++ b/clippy_lints/src/loops/manual_memcpy.rs @@ -51,7 +51,7 @@ pub(super) fn check<'tcx>( iter_b = Some(get_assignment(body)); } - let assignments = iter_a.into_iter().flatten().chain(iter_b.into_iter()); + let assignments = iter_a.into_iter().flatten().chain(iter_b); let big_sugg = assignments // The only statements in the for loops can be indexed assignments from @@ -402,7 +402,7 @@ fn get_assignments<'a, 'tcx>( StmtKind::Local(..) | StmtKind::Item(..) => None, StmtKind::Expr(e) | StmtKind::Semi(e) => Some(e), }) - .chain((*expr).into_iter()) + .chain(*expr) .filter(move |e| { if let ExprKind::AssignOp(_, place, _) = e.kind { path_to_local(place).map_or(false, |id| { From 6a76101d3a46fb1dc21b2ee3044dfac30e3d9a1d Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Wed, 24 May 2023 15:56:28 +0200 Subject: [PATCH 3/5] update docs, simplify arg->param map, dont lint on chain --- clippy_lints/src/explicit_into_iter_fn_arg.rs | 124 +++++++++--------- tests/ui/explicit_into_iter_fn_arg.fixed | 17 ++- tests/ui/explicit_into_iter_fn_arg.rs | 17 ++- tests/ui/explicit_into_iter_fn_arg.stderr | 6 +- 4 files changed, 90 insertions(+), 74 deletions(-) diff --git a/clippy_lints/src/explicit_into_iter_fn_arg.rs b/clippy_lints/src/explicit_into_iter_fn_arg.rs index a56d72f0dea..ff4144df9b9 100644 --- a/clippy_lints/src/explicit_into_iter_fn_arg.rs +++ b/clippy_lints/src/explicit_into_iter_fn_arg.rs @@ -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>(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 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>(iter: I) -> u32 { + /// fn even_sum>(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>(iter: I) -> u32 { + /// fn even_sum>(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 { - 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 { + 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(), "").into_owned(); + let mut applicability = Applicability::MachineApplicable; + let sugg = snippet_with_applicability(cx, recv.span.source_callsite(), "", &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()`"); }); diff --git a/tests/ui/explicit_into_iter_fn_arg.fixed b/tests/ui/explicit_into_iter_fn_arg.fixed index 219b60bd051..9d778faf8e0 100644 --- a/tests/ui/explicit_into_iter_fn_arg.fixed +++ b/tests/ui/explicit_into_iter_fn_arg.fixed @@ -1,6 +1,6 @@ //@run-rustfix -#![allow(unused)] +#![allow(unused, clippy::useless_conversion)] #![warn(clippy::explicit_into_iter_fn_arg)] fn a(_: T) {} @@ -11,7 +11,6 @@ where T: IntoIterator, { } -fn e>(_: T) {} fn f(_: std::vec::IntoIter) {} 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!()); } diff --git a/tests/ui/explicit_into_iter_fn_arg.rs b/tests/ui/explicit_into_iter_fn_arg.rs index 358f4bc661d..1e051fd06ec 100644 --- a/tests/ui/explicit_into_iter_fn_arg.rs +++ b/tests/ui/explicit_into_iter_fn_arg.rs @@ -1,6 +1,6 @@ //@run-rustfix -#![allow(unused)] +#![allow(unused, clippy::useless_conversion)] #![warn(clippy::explicit_into_iter_fn_arg)] fn a(_: T) {} @@ -11,7 +11,6 @@ where T: IntoIterator, { } -fn e>(_: T) {} fn f(_: std::vec::IntoIter) {} 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!()); } diff --git a/tests/ui/explicit_into_iter_fn_arg.stderr b/tests/ui/explicit_into_iter_fn_arg.stderr index a0bd81c21ea..14b95fb097a 100644 --- a/tests/ui/explicit_into_iter_fn_arg.stderr +++ b/tests/ui/explicit_into_iter_fn_arg.stderr @@ -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) {} = 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) {} | ^^^^^^^^^^^^^^^^^^^^^^^^ 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]` From de1f410018b6ec176a5584896ce017ab3f3edf91 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 30 May 2023 22:32:26 +0200 Subject: [PATCH 4/5] merge `explicit_into_iter_fn_arg` into `useless_conversion` --- CHANGELOG.md | 1 - clippy_lints/src/declared_lints.rs | 1 - clippy_lints/src/explicit_into_iter_fn_arg.rs | 142 ------------------ clippy_lints/src/lib.rs | 2 - clippy_lints/src/useless_conversion.rs | 121 ++++++++++++++- tests/ui/explicit_into_iter_fn_arg.fixed | 33 ---- tests/ui/explicit_into_iter_fn_arg.rs | 33 ---- tests/ui/explicit_into_iter_fn_arg.stderr | 39 ----- tests/ui/useless_conversion.fixed | 29 ++++ tests/ui/useless_conversion.rs | 29 ++++ tests/ui/useless_conversion.stderr | 62 +++++++- 11 files changed, 234 insertions(+), 258 deletions(-) delete mode 100644 clippy_lints/src/explicit_into_iter_fn_arg.rs delete mode 100644 tests/ui/explicit_into_iter_fn_arg.fixed delete mode 100644 tests/ui/explicit_into_iter_fn_arg.rs delete mode 100644 tests/ui/explicit_into_iter_fn_arg.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a127c08197..a2648d9faa6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index fceb6e4b2ae..4ade25e1257 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -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, diff --git a/clippy_lints/src/explicit_into_iter_fn_arg.rs b/clippy_lints/src/explicit_into_iter_fn_arg.rs deleted file mode 100644 index ff4144df9b9..00000000000 --- a/clippy_lints/src/explicit_into_iter_fn_arg.rs +++ /dev/null @@ -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>(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>(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 { - 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(), "", &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()`"); - }); - } - } - } -} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index bfe52e182b2..10404d51ba5 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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` } diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs index 28c3fc859e3..6f1e9d5f9de 100644 --- a/clippy_lints/src/useless_conversion.rs +++ b/clippy_lints/src/useless_conversion.rs @@ -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 { + 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(), "", &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 diff --git a/tests/ui/explicit_into_iter_fn_arg.fixed b/tests/ui/explicit_into_iter_fn_arg.fixed deleted file mode 100644 index 9d778faf8e0..00000000000 --- a/tests/ui/explicit_into_iter_fn_arg.fixed +++ /dev/null @@ -1,33 +0,0 @@ -//@run-rustfix - -#![allow(unused, clippy::useless_conversion)] -#![warn(clippy::explicit_into_iter_fn_arg)] - -fn a(_: T) {} -fn b>(_: T) {} -fn c(_: impl IntoIterator) {} -fn d(_: T) -where - T: IntoIterator, -{ -} -fn f(_: std::vec::IntoIter) {} - -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!()); -} diff --git a/tests/ui/explicit_into_iter_fn_arg.rs b/tests/ui/explicit_into_iter_fn_arg.rs deleted file mode 100644 index 1e051fd06ec..00000000000 --- a/tests/ui/explicit_into_iter_fn_arg.rs +++ /dev/null @@ -1,33 +0,0 @@ -//@run-rustfix - -#![allow(unused, clippy::useless_conversion)] -#![warn(clippy::explicit_into_iter_fn_arg)] - -fn a(_: T) {} -fn b>(_: T) {} -fn c(_: impl IntoIterator) {} -fn d(_: T) -where - T: IntoIterator, -{ -} -fn f(_: std::vec::IntoIter) {} - -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!()); -} diff --git a/tests/ui/explicit_into_iter_fn_arg.stderr b/tests/ui/explicit_into_iter_fn_arg.stderr deleted file mode 100644 index 14b95fb097a..00000000000 --- a/tests/ui/explicit_into_iter_fn_arg.stderr +++ /dev/null @@ -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) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^ - = 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) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^ - -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, - | ^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 3 previous errors - diff --git a/tests/ui/useless_conversion.fixed b/tests/ui/useless_conversion.fixed index c16caa38fe9..e9563b8a60a 100644 --- a/tests/ui/useless_conversion.fixed +++ b/tests/ui/useless_conversion.fixed @@ -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) {} + fn b>(_: T) {} + fn c(_: impl IntoIterator) {} + fn d(_: T) + where + T: IntoIterator, + { + } + fn f(_: std::vec::IntoIter) {} + + 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; diff --git a/tests/ui/useless_conversion.rs b/tests/ui/useless_conversion.rs index c75a2bce4ca..c2f4e3117d2 100644 --- a/tests/ui/useless_conversion.rs +++ b/tests/ui/useless_conversion.rs @@ -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) {} + fn b>(_: T) {} + fn c(_: impl IntoIterator) {} + fn d(_: T) + where + T: IntoIterator, + { + } + fn f(_: std::vec::IntoIter) {} + + 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; diff --git a/tests/ui/useless_conversion.stderr b/tests/ui/useless_conversion.stderr index 4dca3aac533..68f38faab59 100644 --- a/tests/ui/useless_conversion.stderr +++ b/tests/ui/useless_conversion.stderr @@ -118,5 +118,65 @@ error: useless conversion to the same type: `std::vec::IntoIter>` 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) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +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) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +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, + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +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) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +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) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 24 previous errors From d816eba09d4ca01ae6a3576de6db3ee415214d03 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Tue, 30 May 2023 22:32:47 +0200 Subject: [PATCH 5/5] fix new failing proc macro tests --- tests/ui/auxiliary/proc_macro_derive.rs | 110 +++++++++++------------- 1 file changed, 49 insertions(+), 61 deletions(-) diff --git a/tests/ui/auxiliary/proc_macro_derive.rs b/tests/ui/auxiliary/proc_macro_derive.rs index 5a924ca1830..5ea86a9ff37 100644 --- a/tests/ui/auxiliary/proc_macro_derive.rs +++ b/tests/ui/auxiliary/proc_macro_derive.rs @@ -90,70 +90,58 @@ pub fn extra_lifetime(_input: TokenStream) -> TokenStream { #[allow(unused)] #[proc_macro_derive(ArithmeticDerive)] pub fn arithmetic_derive(_: TokenStream) -> TokenStream { - >::from_iter( - [ - Ident::new("fn", Span::call_site()).into(), - Ident::new("_foo", Span::call_site()).into(), - Group::new(Delimiter::Parenthesis, TokenStream::new()).into(), - Group::new( - Delimiter::Brace, - >::from_iter( - [ - Ident::new("let", Span::call_site()).into(), - Ident::new("mut", Span::call_site()).into(), - Ident::new("_n", Span::call_site()).into(), - Punct::new('=', Spacing::Alone).into(), - Literal::i32_unsuffixed(9).into(), - Punct::new(';', Spacing::Alone).into(), - Ident::new("_n", Span::call_site()).into(), - Punct::new('=', Spacing::Alone).into(), - Literal::i32_unsuffixed(9).into(), - Punct::new('/', Spacing::Alone).into(), - Literal::i32_unsuffixed(2).into(), - Punct::new(';', Spacing::Alone).into(), - Ident::new("_n", Span::call_site()).into(), - Punct::new('=', Spacing::Alone).into(), - Punct::new('-', Spacing::Alone).into(), - Ident::new("_n", Span::call_site()).into(), - Punct::new(';', Spacing::Alone).into(), - ] - .into_iter(), - ), - ) - .into(), - ] - .into_iter(), - ) + >::from_iter([ + Ident::new("fn", Span::call_site()).into(), + Ident::new("_foo", Span::call_site()).into(), + Group::new(Delimiter::Parenthesis, TokenStream::new()).into(), + Group::new( + Delimiter::Brace, + >::from_iter([ + Ident::new("let", Span::call_site()).into(), + Ident::new("mut", Span::call_site()).into(), + Ident::new("_n", Span::call_site()).into(), + Punct::new('=', Spacing::Alone).into(), + Literal::i32_unsuffixed(9).into(), + Punct::new(';', Spacing::Alone).into(), + Ident::new("_n", Span::call_site()).into(), + Punct::new('=', Spacing::Alone).into(), + Literal::i32_unsuffixed(9).into(), + Punct::new('/', Spacing::Alone).into(), + Literal::i32_unsuffixed(2).into(), + Punct::new(';', Spacing::Alone).into(), + Ident::new("_n", Span::call_site()).into(), + Punct::new('=', Spacing::Alone).into(), + Punct::new('-', Spacing::Alone).into(), + Ident::new("_n", Span::call_site()).into(), + Punct::new(';', Spacing::Alone).into(), + ]), + ) + .into(), + ]) } #[allow(unused)] #[proc_macro_derive(ShadowDerive)] pub fn shadow_derive(_: TokenStream) -> TokenStream { - >::from_iter( - [ - Ident::new("fn", Span::call_site()).into(), - Ident::new("_foo", Span::call_site()).into(), - Group::new(Delimiter::Parenthesis, TokenStream::new()).into(), - Group::new( - Delimiter::Brace, - >::from_iter( - [ - Ident::new("let", Span::call_site()).into(), - Ident::new("_x", Span::call_site()).into(), - Punct::new('=', Spacing::Alone).into(), - Literal::i32_unsuffixed(2).into(), - Punct::new(';', Spacing::Alone).into(), - Ident::new("let", Span::call_site()).into(), - Ident::new("_x", Span::call_site()).into(), - Punct::new('=', Spacing::Alone).into(), - Ident::new("_x", Span::call_site()).into(), - Punct::new(';', Spacing::Alone).into(), - ] - .into_iter(), - ), - ) - .into(), - ] - .into_iter(), - ) + >::from_iter([ + Ident::new("fn", Span::call_site()).into(), + Ident::new("_foo", Span::call_site()).into(), + Group::new(Delimiter::Parenthesis, TokenStream::new()).into(), + Group::new( + Delimiter::Brace, + >::from_iter([ + Ident::new("let", Span::call_site()).into(), + Ident::new("_x", Span::call_site()).into(), + Punct::new('=', Spacing::Alone).into(), + Literal::i32_unsuffixed(2).into(), + Punct::new(';', Spacing::Alone).into(), + Ident::new("let", Span::call_site()).into(), + Ident::new("_x", Span::call_site()).into(), + Punct::new('=', Spacing::Alone).into(), + Ident::new("_x", Span::call_site()).into(), + Punct::new(';', Spacing::Alone).into(), + ]), + ) + .into(), + ]) }