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] 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 +