about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-10-07 15:55:34 +0000
committerbors <bors@rust-lang.org>2023-10-07 15:55:34 +0000
commit76240459976735c14bddbfa2ec4ac5188f901f0a (patch)
tree6fe6da18e704ab46dc7b61bcc086756dc898861b
parent3662bd8f587c9305e4d231a9456e63b92b8a7729 (diff)
parent1c6fa2989d056de1183e5df1e1d778e925a6a6fe (diff)
downloadrust-76240459976735c14bddbfa2ec4ac5188f901f0a.tar.gz
rust-76240459976735c14bddbfa2ec4ac5188f901f0a.zip
Auto merge of #11639 - y21:issue11635, r=llogiq
[`into_iter_without_iter`]: walk up deref impl chain to find `iter` methods

Fixes #11635

changelog: [`into_iter_without_iter`]: walk up deref impl chain to find `iter` methods
-rw-r--r--clippy_lints/src/iter_without_into_iter.rs25
-rw-r--r--tests/ui/into_iter_without_iter.rs34
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 43e1b92c9b9..ee3d1735043 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<Item = Ty<'tcx>> + '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 6be3bb8abdd..e6ff821a8ad 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<u8>);
+    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()
+        }
+    }
+}