Move iter_once and iter_empty to methods as a late pass

This enables more thorough checking of types to avoid triggering on
custom Some and None enum variants

10 files changed, 245 insertions, 170 deletions

diff --git a/clippy_lints/src/iter_once_empty.rs b/clippy_lints/src/iter_once_empty.rs
deleted file mode 100644
index 09896001112..00000000000
--- a/clippy_lints/src/iter_once_empty.rs
+++ /dev/null
@@ -1,164 +0,0 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::source::snippet;
-use rustc_ast::ast::{Expr, ExprKind};
-use rustc_errors::Applicability;
-use rustc_lint::{EarlyContext, EarlyLintPass};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
-
-declare_clippy_lint! {
-    /// ### What it does
-    ///
-    /// Checks for usage of:
-    ///
-    /// - `[foo].iter()`
-    /// - `[foo].iter_mut()`
-    /// - `[foo].into_iter()`
-    /// - `Some(foo).iter()`
-    /// - `Some(foo).iter_mut()`
-    /// - `Some(foo).into_iter()`
-    ///
-    /// ### Why is this bad?
-    ///
-    /// It is simpler to use the once function from the standard library:
-    ///
-    /// ### Example
-    ///
-    /// ```rust
-    /// let a = [123].iter();
-    /// let b = Some(123).into_iter();
-    /// ```
-    /// Use instead:
-    /// ```rust
-    /// use std::iter;
-    /// let a = iter::once(&123);
-    /// let b = iter::once(123);
-    /// ```
-    ///
-    /// ### Known problems
-    ///
-    /// The type of the resulting iterator might become incompatible with its usage
-    #[clippy::version = "1.64.0"]
-    pub ITER_ONCE,
-    nursery,
-    "Iterator for array of length 1"
-}
-
-declare_clippy_lint! {
-    /// ### What it does
-    ///
-    /// Checks for usage of:
-    ///
-    /// - `[].iter()`
-    /// - `[].iter_mut()`
-    /// - `[].into_iter()`
-    /// - `None.iter()`
-    /// - `None.iter_mut()`
-    /// - `None.into_iter()`
-    ///
-    /// ### Why is this bad?
-    ///
-    /// It is simpler to use the empty function from the standard library:
-    ///
-    /// ### Example
-    ///
-    /// ```rust
-    /// use std::{slice, option};
-    /// let a: slice::Iter<i32> = [].iter();
-    /// let f: option::IntoIter<i32> = None.into_iter();
-    /// ```
-    /// Use instead:
-    /// ```rust
-    /// use std::iter;
-    /// let a: iter::Empty<i32> = iter::empty();
-    /// let b: iter::Empty<i32> = iter::empty();
-    /// ```
-    ///
-    /// ### Known problems
-    ///
-    /// The type of the resulting iterator might become incompatible with its usage
-    #[clippy::version = "1.64.0"]
-    pub ITER_EMPTY,
-    nursery,
-    "Iterator for empty array"
-}
-
-declare_lint_pass!(IterOnceEmpty => [ITER_ONCE, ITER_EMPTY]);
-
-impl EarlyLintPass for IterOnceEmpty {
-    fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
-        if expr.span.from_expansion() {
-            // Don't lint match expressions present in
-            // macro_rules! block
-            return;
-        }
-
-        let (method_name, args) = if let ExprKind::MethodCall(seg, args, _) = &expr.kind {
-            (seg.ident.as_str(), args)
-        } else {
-            return;
-        };
-        let arg = if args.len() == 1 {
-            &args[0]
-        } else {
-            return;
-        };
-
-        let item = match &arg.kind {
-            ExprKind::Array(v) if v.len() <= 1 => v.first(),
-            ExprKind::Path(None, p) => {
-                if p.segments.len() == 1 && p.segments[0].ident.name == rustc_span::sym::None {
-                    None
-                } else {
-                    return;
-                }
-            },
-            ExprKind::Call(f, some_args) if some_args.len() == 1 => {
-                if let ExprKind::Path(None, p) = &f.kind {
-                    if p.segments.len() == 1 && p.segments[0].ident.name == rustc_span::sym::Some {
-                        Some(&some_args[0])
-                    } else {
-                        return;
-                    }
-                } else {
-                    return;
-                }
-            },
-            _ => return,
-        };
-
-        if let Some(i) = item {
-            let (sugg, msg) = match method_name {
-                "iter" => (
-                    format!("std::iter::once(&{})", snippet(cx, i.span, "...")),
-                    "this `iter` call can be replaced with std::iter::once",
-                ),
-                "iter_mut" => (
-                    format!("std::iter::once(&mut {})", snippet(cx, i.span, "...")),
-                    "this `iter_mut` call can be replaced with std::iter::once",
-                ),
-                "into_iter" => (
-                    format!("std::iter::once({})", snippet(cx, i.span, "...")),
-                    "this `into_iter` call can be replaced with std::iter::once",
-                ),
-                _ => return,
-            };
-            span_lint_and_sugg(cx, ITER_ONCE, expr.span, msg, "try", sugg, Applicability::Unspecified);
-        } else {
-            let msg = match method_name {
-                "iter" => "this `iter call` can be replaced with std::iter::empty",
-                "iter_mut" => "this `iter_mut` call can be replaced with std::iter::empty",
-                "into_iter" => "this `into_iter` call can be replaced with std::iter::empty",
-                _ => return,
-            };
-            span_lint_and_sugg(
-                cx,
-                ITER_EMPTY,
-                expr.span,
-                msg,
-                "try",
-                "std::iter::empty()".to_string(),
-                Applicability::Unspecified,
-            );
-        }
-    }
-}
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index 49fc3a89527..e49321d83fa 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -200,8 +200,6 @@ store.register_lints(&[
     invalid_utf8_in_unchecked::INVALID_UTF8_IN_UNCHECKED,
     items_after_statements::ITEMS_AFTER_STATEMENTS,
     iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR,
-    iter_once_empty::ITER_EMPTY,
-    iter_once_empty::ITER_ONCE,
     large_const_arrays::LARGE_CONST_ARRAYS,
     large_enum_variant::LARGE_ENUM_VARIANT,
     large_include_file::LARGE_INCLUDE_FILE,
@@ -314,9 +312,11 @@ store.register_lints(&[
     methods::ITERATOR_STEP_BY_ZERO,
     methods::ITER_CLONED_COLLECT,
     methods::ITER_COUNT,
+    methods::ITER_EMPTY,
     methods::ITER_NEXT_SLICE,
     methods::ITER_NTH,
     methods::ITER_NTH_ZERO,
+    methods::ITER_ONCE,
     methods::ITER_OVEREAGER_CLONED,
     methods::ITER_SKIP_NEXT,
     methods::ITER_WITH_DRAIN,
diff --git a/clippy_lints/src/lib.register_pedantic.rs b/clippy_lints/src/lib.register_pedantic.rs
index 6063fb00a7b..4250ee055e6 100644
--- a/clippy_lints/src/lib.register_pedantic.rs
+++ b/clippy_lints/src/lib.register_pedantic.rs
@@ -41,8 +41,6 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
     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(iter_once_empty::ITER_EMPTY),
-    LintId::of(iter_once_empty::ITER_ONCE),
     LintId::of(large_stack_arrays::LARGE_STACK_ARRAYS),
     LintId::of(let_underscore::LET_UNDERSCORE_DROP),
     LintId::of(literal_representation::LARGE_DIGIT_GROUPS),
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 78279db9ad5..88c1a727f8d 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -258,7 +258,6 @@ mod invalid_upcast_comparisons;
 mod invalid_utf8_in_unchecked;
 mod items_after_statements;
 mod iter_not_returning_iterator;
-mod iter_once_empty;
 mod large_const_arrays;
 mod large_enum_variant;
 mod large_include_file;
@@ -932,7 +931,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(invalid_utf8_in_unchecked::InvalidUtf8InUnchecked));
     store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default()));
     store.register_late_pass(|| Box::new(manual_instant_elapsed::ManualInstantElapsed));
-    store.register_early_pass(|| Box::new(iter_once_empty::IterOnceEmpty));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 
diff --git a/clippy_lints/src/methods/iter_once_empty.rs b/clippy_lints/src/methods/iter_once_empty.rs
new file mode 100644
index 00000000000..d45dfc67880
--- /dev/null
+++ b/clippy_lints/src/methods/iter_once_empty.rs
@@ -0,0 +1,70 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::is_lang_ctor;
+use clippy_utils::source::snippet;
+
+use rustc_errors::Applicability;
+use rustc_hir::LangItem::{OptionNone, OptionSome};
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+
+use super::{ITER_EMPTY, ITER_ONCE};
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, method_name: &str, recv: &Expr<'_>) {
+    let item = match &recv.kind {
+        ExprKind::Array(v) if v.len() <= 1 => v.first(),
+        ExprKind::Path(p) => {
+            if is_lang_ctor(cx, p, OptionNone) {
+                None
+            } else {
+                return;
+            }
+        },
+        ExprKind::Call(f, some_args) if some_args.len() == 1 => {
+            if let ExprKind::Path(p) = &f.kind {
+                if is_lang_ctor(cx, p, OptionSome) {
+                    Some(&some_args[0])
+                } else {
+                    return;
+                }
+            } else {
+                return;
+            }
+        },
+        _ => return,
+    };
+
+    if let Some(i) = item {
+        let (sugg, msg) = match method_name {
+            "iter" => (
+                format!("std::iter::once(&{})", snippet(cx, i.span, "...")),
+                "this `iter` call can be replaced with std::iter::once",
+            ),
+            "iter_mut" => (
+                format!("std::iter::once(&mut {})", snippet(cx, i.span, "...")),
+                "this `iter_mut` call can be replaced with std::iter::once",
+            ),
+            "into_iter" => (
+                format!("std::iter::once({})", snippet(cx, i.span, "...")),
+                "this `into_iter` call can be replaced with std::iter::once",
+            ),
+            _ => return,
+        };
+        span_lint_and_sugg(cx, ITER_ONCE, expr.span, msg, "try", sugg, Applicability::Unspecified);
+    } else {
+        let msg = match method_name {
+            "iter" => "this `iter call` can be replaced with std::iter::empty",
+            "iter_mut" => "this `iter_mut` call can be replaced with std::iter::empty",
+            "into_iter" => "this `into_iter` call can be replaced with std::iter::empty",
+            _ => return,
+        };
+        span_lint_and_sugg(
+            cx,
+            ITER_EMPTY,
+            expr.span,
+            msg,
+            "try",
+            "std::iter::empty()".to_string(),
+            Applicability::Unspecified,
+        );
+    }
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 5ac6b09f0aa..e449ae0e424 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -33,6 +33,7 @@ mod iter_count;
 mod iter_next_slice;
 mod iter_nth;
 mod iter_nth_zero;
+mod iter_once_empty;
 mod iter_overeager_cloned;
 mod iter_skip_next;
 mod iter_with_drain;
@@ -2304,6 +2305,83 @@ declare_clippy_lint! {
     more clearly with `if .. else ..`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for usage of:
+    ///
+    /// - `[foo].iter()`
+    /// - `[foo].iter_mut()`
+    /// - `[foo].into_iter()`
+    /// - `Some(foo).iter()`
+    /// - `Some(foo).iter_mut()`
+    /// - `Some(foo).into_iter()`
+    ///
+    /// ### Why is this bad?
+    ///
+    /// It is simpler to use the once function from the standard library:
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// let a = [123].iter();
+    /// let b = Some(123).into_iter();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// use std::iter;
+    /// let a = iter::once(&123);
+    /// let b = iter::once(123);
+    /// ```
+    ///
+    /// ### Known problems
+    ///
+    /// The type of the resulting iterator might become incompatible with its usage
+    #[clippy::version = "1.64.0"]
+    pub ITER_ONCE,
+    nursery,
+    "Iterator for array of length 1"
+}
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for usage of:
+    ///
+    /// - `[].iter()`
+    /// - `[].iter_mut()`
+    /// - `[].into_iter()`
+    /// - `None.iter()`
+    /// - `None.iter_mut()`
+    /// - `None.into_iter()`
+    ///
+    /// ### Why is this bad?
+    ///
+    /// It is simpler to use the empty function from the standard library:
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// use std::{slice, option};
+    /// let a: slice::Iter<i32> = [].iter();
+    /// let f: option::IntoIter<i32> = None.into_iter();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// use std::iter;
+    /// let a: iter::Empty<i32> = iter::empty();
+    /// let b: iter::Empty<i32> = iter::empty();
+    /// ```
+    ///
+    /// ### Known problems
+    ///
+    /// The type of the resulting iterator might become incompatible with its usage
+    #[clippy::version = "1.64.0"]
+    pub ITER_EMPTY,
+    nursery,
+    "Iterator for empty array"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Option<RustcVersion>,
@@ -2406,6 +2484,8 @@ impl_lint_pass!(Methods => [
     NEEDLESS_OPTION_TAKE,
     NO_EFFECT_REPLACE,
     OBFUSCATED_IF_ELSE,
+    ITER_ONCE,
+    ITER_EMPTY
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -2708,6 +2788,7 @@ impl Methods {
                 ("is_digit", [radix]) => is_digit_ascii_radix::check(cx, expr, recv, radix, self.msrv),
                 ("is_none", []) => check_is_some_is_none(cx, expr, recv, false),
                 ("is_some", []) => check_is_some_is_none(cx, expr, recv, true),
+                ("iter" | "iter_mut" | "into_iter", []) => iter_once_empty::check(cx, expr, name, recv),
                 ("join", [join_arg]) => {
                     if let Some(("collect", _, span)) = method_call(recv) {
                         unnecessary_join::check(cx, expr, recv, join_arg, span);
diff --git a/tests/ui/iter_empty.fixed b/tests/ui/iter_empty.fixed
index fb7118f0d4a..690da5a8761 100644
--- a/tests/ui/iter_empty.fixed
+++ b/tests/ui/iter_empty.fixed
@@ -26,7 +26,30 @@ macro_rules! in_macros {
     };
 }
 
+// Don't trigger on a `None` that isn't std's option
+mod custom_option {
+    #[allow(unused)]
+    enum CustomOption {
+        Some(i32),
+        None,
+    }
+
+    impl CustomOption {
+        fn iter(&self) {}
+        fn iter_mut(&mut self) {}
+        fn into_iter(self) {}
+    }
+    use CustomOption::*;
+
+    pub fn custom_option() {
+        None.iter();
+        None.iter_mut();
+        None.into_iter();
+    }
+}
+
 fn main() {
     array();
+    custom_option::custom_option();
     in_macros!();
 }
diff --git a/tests/ui/iter_empty.rs b/tests/ui/iter_empty.rs
index bb192fe16d1..f8b56898f2d 100644
--- a/tests/ui/iter_empty.rs
+++ b/tests/ui/iter_empty.rs
@@ -26,7 +26,30 @@ macro_rules! in_macros {
     };
 }
 
+// Don't trigger on a `None` that isn't std's option
+mod custom_option {
+    #[allow(unused)]
+    enum CustomOption {
+        Some(i32),
+        None,
+    }
+
+    impl CustomOption {
+        fn iter(&self) {}
+        fn iter_mut(&mut self) {}
+        fn into_iter(self) {}
+    }
+    use CustomOption::*;
+
+    pub fn custom_option() {
+        None.iter();
+        None.iter_mut();
+        None.into_iter();
+    }
+}
+
 fn main() {
     array();
+    custom_option::custom_option();
     in_macros!();
 }
diff --git a/tests/ui/iter_once.fixed b/tests/ui/iter_once.fixed
index 7247e34d7e8..0c82ab20be1 100644
--- a/tests/ui/iter_once.fixed
+++ b/tests/ui/iter_once.fixed
@@ -26,7 +26,30 @@ macro_rules! in_macros {
     };
 }
 
+// Don't trigger on a `Some` that isn't std's option
+mod custom_option {
+    #[allow(unused)]
+    enum CustomOption {
+        Some(i32),
+        None,
+    }
+
+    impl CustomOption {
+        fn iter(&self) {}
+        fn iter_mut(&mut self) {}
+        fn into_iter(self) {}
+    }
+    use CustomOption::*;
+
+    pub fn custom_option() {
+        Some(3).iter();
+        Some(3).iter_mut();
+        Some(3).into_iter();
+    }
+}
+
 fn main() {
     array();
+    custom_option::custom_option();
     in_macros!();
 }
diff --git a/tests/ui/iter_once.rs b/tests/ui/iter_once.rs
index 3a2b9c95cc5..d561bf27c1d 100644
--- a/tests/ui/iter_once.rs
+++ b/tests/ui/iter_once.rs
@@ -26,7 +26,30 @@ macro_rules! in_macros {
     };
 }
 
+// Don't trigger on a `Some` that isn't std's option
+mod custom_option {
+    #[allow(unused)]
+    enum CustomOption {
+        Some(i32),
+        None,
+    }
+
+    impl CustomOption {
+        fn iter(&self) {}
+        fn iter_mut(&mut self) {}
+        fn into_iter(self) {}
+    }
+    use CustomOption::*;
+
+    pub fn custom_option() {
+        Some(3).iter();
+        Some(3).iter_mut();
+        Some(3).into_iter();
+    }
+}
+
 fn main() {
     array();
+    custom_option::custom_option();
     in_macros!();
 }