Auto merge of #7610 - Labelray:master, r=camsteffen

Add new lint `iter_not_returning_iterator`

Add new lint [`iter_not_returning_iterator`] to detect method `iter()` or `iter_mut()` returning a type not implementing `Iterator`
changelog: Add new lint [`iter_not_returning_iterator`]

5 files changed, 132 insertions, 0 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f5ac2f7c9f8..71383e8116b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2722,6 +2722,7 @@ Released 2018-09-13
 [`iter_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_count
 [`iter_next_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_loop
 [`iter_next_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_slice
+[`iter_not_returning_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_not_returning_iterator
 [`iter_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth
 [`iter_nth_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth_zero
 [`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next
diff --git a/clippy_lints/src/iter_not_returning_iterator.rs b/clippy_lints/src/iter_not_returning_iterator.rs
new file mode 100644
index 00000000000..6c779348ca2
--- /dev/null
+++ b/clippy_lints/src/iter_not_returning_iterator.rs
@@ -0,0 +1,64 @@
+use clippy_utils::{diagnostics::span_lint, return_ty, ty::implements_trait};
+use rustc_hir::{ImplItem, ImplItemKind};
+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! {
+    /// ### What it does
+    /// Detects methods named `iter` or `iter_mut` that do not have a return type that implements `Iterator`.
+    ///
+    /// ### Why is this bad?
+    /// Methods named `iter` or `iter_mut` conventionally return an `Iterator`.
+    ///
+    /// ### Example
+    /// ```rust
+    /// // `String` does not implement `Iterator`
+    /// struct Data {}
+    /// impl Data {
+    ///     fn iter(&self) -> String {
+    ///         todo!()
+    ///     }
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// use std::str::Chars;
+    /// struct Data {}
+    /// impl Data {
+    ///    fn iter(&self) -> Chars<'static> {
+    ///        todo!()
+    ///    }
+    /// }
+    /// ```
+    pub ITER_NOT_RETURNING_ITERATOR,
+    pedantic,
+    "methods named `iter` or `iter_mut` that do not return an `Iterator`"
+}
+
+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: &str = &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, &[]);
+
+            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),
+                );
+            }
+        }
+    }
+}
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 9544309ae4f..e3dd43d6982 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -241,6 +241,7 @@ mod int_plus_one;
 mod integer_division;
 mod invalid_upcast_comparisons;
 mod items_after_statements;
+mod iter_not_returning_iterator;
 mod large_const_arrays;
 mod large_enum_variant;
 mod large_stack_arrays;
@@ -674,6 +675,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         integer_division::INTEGER_DIVISION,
         invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS,
         items_after_statements::ITEMS_AFTER_STATEMENTS,
+        iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR,
         large_const_arrays::LARGE_CONST_ARRAYS,
         large_enum_variant::LARGE_ENUM_VARIANT,
         large_stack_arrays::LARGE_STACK_ARRAYS,
@@ -1104,6 +1106,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(infinite_iter::MAYBE_INFINITE_ITER),
         LintId::of(invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS),
         LintId::of(items_after_statements::ITEMS_AFTER_STATEMENTS),
+        LintId::of(iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR),
         LintId::of(large_stack_arrays::LARGE_STACK_ARRAYS),
         LintId::of(let_underscore::LET_UNDERSCORE_DROP),
         LintId::of(literal_representation::LARGE_DIGIT_GROUPS),
@@ -2131,6 +2134,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(strlen_on_c_strings::StrlenOnCStrings));
     store.register_late_pass(move || Box::new(self_named_constructors::SelfNamedConstructors));
     store.register_late_pass(move || Box::new(feature_name::FeatureName));
+    store.register_late_pass(move || Box::new(iter_not_returning_iterator::IterNotReturningIterator));
 }
 
 #[rustfmt::skip]
diff --git a/tests/ui/iter_not_returning_iterator.rs b/tests/ui/iter_not_returning_iterator.rs
new file mode 100644
index 00000000000..377f760b3c4
--- /dev/null
+++ b/tests/ui/iter_not_returning_iterator.rs
@@ -0,0 +1,47 @@
+#![warn(clippy::iter_not_returning_iterator)]
+
+struct Data {
+    begin: u32,
+}
+
+struct Counter {
+    count: u32,
+}
+
+impl Data {
+    fn iter(&self) -> Counter {
+        todo!()
+    }
+
+    fn iter_mut(&self) -> Counter {
+        todo!()
+    }
+}
+
+struct Data2 {
+    begin: u32,
+}
+
+struct Counter2 {
+    count: u32,
+}
+
+impl Data2 {
+    fn iter(&self) -> Counter2 {
+        todo!()
+    }
+
+    fn iter_mut(&self) -> Counter2 {
+        todo!()
+    }
+}
+
+impl Iterator for Counter {
+    type Item = u32;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        todo!()
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/iter_not_returning_iterator.stderr b/tests/ui/iter_not_returning_iterator.stderr
new file mode 100644
index 00000000000..2273cd0be66
--- /dev/null
+++ b/tests/ui/iter_not_returning_iterator.stderr
@@ -0,0 +1,16 @@
+error: this method is named `iter` but its return type does not implement `Iterator`
+  --> $DIR/iter_not_returning_iterator.rs:30:5
+   |
+LL |     fn iter(&self) -> Counter2 {
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::iter-not-returning-iterator` implied by `-D warnings`
+
+error: this method is named `iter_mut` but its return type does not implement `Iterator`
+  --> $DIR/iter_not_returning_iterator.rs:34:5
+   |
+LL |     fn iter_mut(&self) -> Counter2 {
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+