From 2cc38a23227e743aa24b7656ccbcee768ebef5ac Mon Sep 17 00:00:00 2001
From: Jason Newcomb <jsnewcomb@pm.me>
Date: Wed, 5 Jan 2022 13:46:35 -0500
Subject: [PATCH] Lint `iter_not_returning_iterator` on the trait definition
 rather than the implementation

---
 .../src/iter_not_returning_iterator.rs        | 61 +++++++++++++------
 tests/ui/iter_not_returning_iterator.rs       | 12 ++++
 tests/ui/iter_not_returning_iterator.stderr   |  8 ++-
 3 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/clippy_lints/src/iter_not_returning_iterator.rs b/clippy_lints/src/iter_not_returning_iterator.rs
index 0af6b3b7d46..91d3c00039a 100644
--- a/clippy_lints/src/iter_not_returning_iterator.rs
+++ b/clippy_lints/src/iter_not_returning_iterator.rs
@@ -1,8 +1,7 @@
-use clippy_utils::{diagnostics::span_lint, return_ty, ty::implements_trait};
-use rustc_hir::{ImplItem, ImplItemKind};
+use clippy_utils::{diagnostics::span_lint, get_parent_node, ty::implements_trait};
+use rustc_hir::{def_id::LocalDefId, FnSig, ImplItem, ImplItemKind, Item, ItemKind, Node, TraitItem, TraitItemKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::symbol::kw;
 use rustc_span::symbol::sym;
 
 declare_clippy_lint! {
@@ -41,25 +40,47 @@ declare_clippy_lint! {
 declare_lint_pass!(IterNotReturningIterator => [ITER_NOT_RETURNING_ITERATOR]);
 
 impl LateLintPass<'_> for IterNotReturningIterator {
-    fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'tcx>) {
-        let name = impl_item.ident.name.as_str();
-        if_chain! {
-            if let ImplItemKind::Fn(fn_sig, _) = &impl_item.kind;
-            let ret_ty = return_ty(cx, impl_item.hir_id());
-            if matches!(name, "iter" | "iter_mut");
-            if let [param] = cx.tcx.fn_arg_names(impl_item.def_id);
-            if param.name == kw::SelfLower;
-            if let Some(iter_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
-            if !implements_trait(cx, ret_ty, iter_trait_id, &[]);
+    fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) {
+        let name = item.ident.name.as_str();
+        if matches!(name, "iter" | "iter_mut") {
+            if let TraitItemKind::Fn(fn_sig, _) = &item.kind {
+                check_sig(cx, name, fn_sig, item.def_id);
+            }
+        }
+    }
 
-            then {
-                span_lint(
-                    cx,
-                    ITER_NOT_RETURNING_ITERATOR,
-                    fn_sig.span,
-                    &format!("this method is named `{}` but its return type does not implement `Iterator`", name),
-                );
+    fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'tcx>) {
+        let name = item.ident.name.as_str();
+        if matches!(name, "iter" | "iter_mut")
+            && !matches!(
+                get_parent_node(cx.tcx, item.hir_id()),
+                Some(Node::Item(Item { kind: ItemKind::Impl(i), .. })) if i.of_trait.is_some()
+            )
+        {
+            if let ImplItemKind::Fn(fn_sig, _) = &item.kind {
+                check_sig(cx, name, fn_sig, item.def_id);
             }
         }
     }
 }
+
+fn check_sig(cx: &LateContext<'_>, name: &str, sig: &FnSig<'_>, fn_id: LocalDefId) {
+    if sig.decl.implicit_self.has_implicit_self() {
+        let ret_ty = cx.tcx.fn_sig(fn_id).skip_binder().output();
+        if cx
+            .tcx
+            .get_diagnostic_item(sym::Iterator)
+            .map_or(false, |iter_id| !implements_trait(cx, ret_ty, iter_id, &[]))
+        {
+            span_lint(
+                cx,
+                ITER_NOT_RETURNING_ITERATOR,
+                sig.span,
+                &format!(
+                    "this method is named `{}` but its return type does not implement `Iterator`",
+                    name
+                ),
+            );
+        }
+    }
+}
diff --git a/tests/ui/iter_not_returning_iterator.rs b/tests/ui/iter_not_returning_iterator.rs
index 377f760b3c4..f5ee044576c 100644
--- a/tests/ui/iter_not_returning_iterator.rs
+++ b/tests/ui/iter_not_returning_iterator.rs
@@ -44,4 +44,16 @@ impl Iterator for Counter {
     }
 }
 
+trait Iter {
+    type I;
+    fn iter(&self) -> Self::I;
+}
+
+impl Iter for () {
+    type I = core::slice::Iter<'static, ()>;
+    fn iter(&self) -> Self::I {
+        [].iter()
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/iter_not_returning_iterator.stderr b/tests/ui/iter_not_returning_iterator.stderr
index 2273cd0be66..ddb2b88d5f9 100644
--- a/tests/ui/iter_not_returning_iterator.stderr
+++ b/tests/ui/iter_not_returning_iterator.stderr
@@ -12,5 +12,11 @@ error: this method is named `iter_mut` but its return type does not implement `I
 LL |     fn iter_mut(&self) -> Counter2 {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 2 previous errors
+error: this method is named `iter` but its return type does not implement `Iterator`
+  --> $DIR/iter_not_returning_iterator.rs:49:5
+   |
+LL |     fn iter(&self) -> Self::I;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors