From 1c6fa2989d056de1183e5df1e1d778e925a6a6fe Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Sat, 7 Oct 2023 16:33:06 +0200 Subject: [PATCH] [`into_iter_without_iter`]: look for `iter` method in deref chains --- clippy_lints/src/iter_without_into_iter.rs | 25 ++++++++++++++-- tests/ui/into_iter_without_iter.rs | 34 ++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/iter_without_into_iter.rs b/clippy_lints/src/iter_without_into_iter.rs index 43e1b92c9b9a7..ee3d1735043d5 100644 --- a/clippy_lints/src/iter_without_into_iter.rs +++ b/clippy_lints/src/iter_without_into_iter.rs @@ -9,6 +9,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{sym, Symbol}; +use std::iter; declare_clippy_lint! { /// ### What it does @@ -52,7 +53,8 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does /// This is the opposite of the `iter_without_into_iter` lint. - /// It looks for `IntoIterator for (&|&mut) Type` implementations without an inherent `iter` or `iter_mut` method. + /// It looks for `IntoIterator for (&|&mut) Type` implementations without an inherent `iter` or `iter_mut` method + /// on the type or on any of the types in its `Deref` chain. /// /// ### Why is this bad? /// It's not bad, but having them is idiomatic and allows the type to be used in iterator chains @@ -102,7 +104,20 @@ fn is_nameable_in_impl_trait(ty: &rustc_hir::Ty<'_>) -> bool { !matches!(ty.kind, TyKind::OpaqueDef(..)) } -fn type_has_inherent_method(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> bool { +/// Returns the deref chain of a type, starting with the type itself. +fn deref_chain<'cx, 'tcx>(cx: &'cx LateContext<'tcx>, ty: Ty<'tcx>) -> impl Iterator> + 'cx { + iter::successors(Some(ty), |&ty| { + if let Some(deref_did) = cx.tcx.lang_items().deref_trait() + && implements_trait(cx, ty, deref_did, &[]) + { + make_normalized_projection(cx.tcx, cx.param_env, deref_did, sym::Target, [ty]) + } else { + None + } + }) +} + +fn adt_has_inherent_method(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> bool { if let Some(ty_did) = ty.ty_adt_def().map(ty::AdtDef::did) { cx.tcx.inherent_impls(ty_did).iter().any(|&did| { cx.tcx @@ -127,7 +142,11 @@ impl LateLintPass<'_> for IterWithoutIntoIter { Mutability::Mut => sym::iter_mut, Mutability::Not => sym::iter, } - && !type_has_inherent_method(cx, ty, expected_method_name) + && !deref_chain(cx, ty) + .any(|ty| { + // We can't check inherent impls for slices, but we know that they have an `iter(_mut)` method + ty.peel_refs().is_slice() || adt_has_inherent_method(cx, ty, expected_method_name) + }) && let Some(iter_assoc_span) = imp.items.iter().find_map(|item| { if item.ident.name == sym!(IntoIter) { Some(cx.tcx.hir().impl_item(item.id).expect_type().span) diff --git a/tests/ui/into_iter_without_iter.rs b/tests/ui/into_iter_without_iter.rs index 6be3bb8abdddc..e6ff821a8ad04 100644 --- a/tests/ui/into_iter_without_iter.rs +++ b/tests/ui/into_iter_without_iter.rs @@ -122,3 +122,37 @@ fn main() { } } } + +fn issue11635() { + // A little more involved than the original repro in the issue, but this tests that it correctly + // works for more than one deref step + + use std::ops::Deref; + + pub struct Thing(Vec); + pub struct Thing2(Thing); + + impl Deref for Thing { + type Target = [u8]; + + fn deref(&self) -> &Self::Target { + &self.0 + } + } + + impl Deref for Thing2 { + type Target = Thing; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + + impl<'a> IntoIterator for &'a Thing2 { + type Item = &'a u8; + type IntoIter = <&'a [u8] as IntoIterator>::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } + } +}