Update iterator_step_by_zero

Move `iterator_step_by_zero` into `methods` since it applies to all
iterators and not just ranges. Simplify the code while doing so.

3 files changed, 42 insertions, 47 deletions

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index bae4eebf850..7fb499ebf85 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -606,6 +606,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         &methods::GET_UNWRAP,
         &methods::INEFFICIENT_TO_STRING,
         &methods::INTO_ITER_ON_REF,
+        &methods::ITERATOR_STEP_BY_ZERO,
         &methods::ITER_CLONED_COLLECT,
         &methods::ITER_NTH,
         &methods::ITER_SKIP_NEXT,
@@ -699,7 +700,6 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         &ptr::PTR_ARG,
         &ptr_offset_with_cast::PTR_OFFSET_WITH_CAST,
         &question_mark::QUESTION_MARK,
-        &ranges::ITERATOR_STEP_BY_ZERO,
         &ranges::RANGE_MINUS_ONE,
         &ranges::RANGE_PLUS_ONE,
         &ranges::RANGE_ZIP_WITH_LEN,
@@ -1179,6 +1179,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         LintId::of(&methods::FLAT_MAP_IDENTITY),
         LintId::of(&methods::INEFFICIENT_TO_STRING),
         LintId::of(&methods::INTO_ITER_ON_REF),
+        LintId::of(&methods::ITERATOR_STEP_BY_ZERO),
         LintId::of(&methods::ITER_CLONED_COLLECT),
         LintId::of(&methods::ITER_NTH),
         LintId::of(&methods::ITER_SKIP_NEXT),
@@ -1244,7 +1245,6 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         LintId::of(&ptr::PTR_ARG),
         LintId::of(&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST),
         LintId::of(&question_mark::QUESTION_MARK),
-        LintId::of(&ranges::ITERATOR_STEP_BY_ZERO),
         LintId::of(&ranges::RANGE_MINUS_ONE),
         LintId::of(&ranges::RANGE_PLUS_ONE),
         LintId::of(&ranges::RANGE_ZIP_WITH_LEN),
@@ -1521,6 +1521,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
         LintId::of(&mem_replace::MEM_REPLACE_WITH_UNINIT),
         LintId::of(&methods::CLONE_DOUBLE_REF),
+        LintId::of(&methods::ITERATOR_STEP_BY_ZERO),
         LintId::of(&methods::TEMPORARY_CSTRING_AS_PTR),
         LintId::of(&methods::UNINIT_ASSUMED_INIT),
         LintId::of(&methods::ZST_OFFSET),
@@ -1533,7 +1534,6 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         LintId::of(&non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
         LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS),
         LintId::of(&ptr::MUT_FROM_REF),
-        LintId::of(&ranges::ITERATOR_STEP_BY_ZERO),
         LintId::of(&regex::INVALID_REGEX),
         LintId::of(&serde_api::SERDE_API_MISUSE),
         LintId::of(&suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index ca62e7ea9d2..8e94cc0f002 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -738,6 +738,26 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
+    /// **What it does:** Checks for calling `.step_by(0)` on iterators,
+    /// which never terminates.
+    ///
+    /// **Why is this bad?** This very much looks like an oversight, since with
+    /// `loop { .. }` there is an obvious better way to endlessly loop.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```ignore
+    /// for x in (5..5).step_by(0) {
+    ///     ..
+    /// }
+    /// ```
+    pub ITERATOR_STEP_BY_ZERO,
+    correctness,
+    "using `Iterator::step_by(0)`, which produces an infinite iterator"
+}
+
+declare_clippy_lint! {
     /// **What it does:** Checks for use of `.iter().nth()` (and the related
     /// `.iter_mut().nth()`) on standard library types with O(1) element access.
     ///
@@ -1115,6 +1135,7 @@ declare_lint_pass!(Methods => [
     FLAT_MAP_IDENTITY,
     FIND_MAP,
     MAP_FLATTEN,
+    ITERATOR_STEP_BY_ZERO,
     ITER_NTH,
     ITER_SKIP_NEXT,
     GET_UNWRAP,
@@ -1173,6 +1194,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
             },
             ["nth", "iter"] => lint_iter_nth(cx, expr, arg_lists[1], false),
             ["nth", "iter_mut"] => lint_iter_nth(cx, expr, arg_lists[1], true),
+            ["step_by", ..] => lint_step_by(cx, expr, arg_lists[0]),
             ["next", "skip"] => lint_iter_skip_next(cx, expr),
             ["collect", "cloned"] => lint_iter_cloned_collect(cx, expr, arg_lists[1]),
             ["as_ref"] => lint_asref(cx, expr, "as_ref", arg_lists[0]),
@@ -1950,6 +1972,20 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args:
     }
 }
 
+fn lint_step_by<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Expr, args: &'tcx [hir::Expr]) {
+    if match_trait_method(cx, expr, &paths::ITERATOR) {
+        use crate::consts::{constant, Constant};
+        if let Some((Constant::Int(0), _)) = constant(cx, cx.tables, &args[1]) {
+            span_lint(
+                cx,
+                ITERATOR_STEP_BY_ZERO,
+                expr.span,
+                "Iterator::step_by(0) will panic at runtime",
+            );
+        }
+    }
+}
+
 fn lint_iter_nth<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Expr, iter_args: &'tcx [hir::Expr], is_mut: bool) {
     let mut_str = if is_mut { "_mut" } else { "" };
     let caller_type = if derefs_to_slice(cx, &iter_args[0], cx.tables.expr_ty(&iter_args[0])).is_some() {
diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs
index b2db5f9365d..6a16adf71dc 100644
--- a/clippy_lints/src/ranges.rs
+++ b/clippy_lints/src/ranges.rs
@@ -8,28 +8,8 @@ use syntax::ast::RangeLimits;
 use syntax::source_map::Spanned;
 
 use crate::utils::sugg::Sugg;
-use crate::utils::{get_trait_def_id, higher, implements_trait, SpanlessEq};
-use crate::utils::{is_integer_const, paths, snippet, snippet_opt, span_lint, span_lint_and_then};
-
-declare_clippy_lint! {
-    /// **What it does:** Checks for calling `.step_by(0)` on iterators,
-    /// which never terminates.
-    ///
-    /// **Why is this bad?** This very much looks like an oversight, since with
-    /// `loop { .. }` there is an obvious better way to endlessly loop.
-    ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
-    /// ```ignore
-    /// for x in (5..5).step_by(0) {
-    ///     ..
-    /// }
-    /// ```
-    pub ITERATOR_STEP_BY_ZERO,
-    correctness,
-    "using `Iterator::step_by(0)`, which produces an infinite iterator"
-}
+use crate::utils::{higher, SpanlessEq};
+use crate::utils::{is_integer_const, snippet, snippet_opt, span_lint, span_lint_and_then};
 
 declare_clippy_lint! {
     /// **What it does:** Checks for zipping a collection with the range of
@@ -102,7 +82,6 @@ declare_clippy_lint! {
 }
 
 declare_lint_pass!(Ranges => [
-    ITERATOR_STEP_BY_ZERO,
     RANGE_ZIP_WITH_LEN,
     RANGE_PLUS_ONE,
     RANGE_MINUS_ONE
@@ -112,19 +91,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Ranges {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
         if let ExprKind::MethodCall(ref path, _, ref args) = expr.kind {
             let name = path.ident.as_str();
-
-            // Range with step_by(0).
-            if name == "step_by" && args.len() == 2 && has_step_by(cx, &args[0]) {
-                use crate::consts::{constant, Constant};
-                if let Some((Constant::Int(0), _)) = constant(cx, cx.tables, &args[1]) {
-                    span_lint(
-                        cx,
-                        ITERATOR_STEP_BY_ZERO,
-                        expr.span,
-                        "Iterator::step_by(0) will panic at runtime",
-                    );
-                }
-            } else if name == "zip" && args.len() == 2 {
+            if name == "zip" && args.len() == 2 {
                 let iter = &args[0].kind;
                 let zip_arg = &args[1];
                 if_chain! {
@@ -232,14 +199,6 @@ fn check_inclusive_range_minus_one(cx: &LateContext<'_, '_>, expr: &Expr) {
     }
 }
 
-fn has_step_by(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
-    // No need for `walk_ptrs_ty` here because `step_by` moves `self`, so it
-    // can't be called on a borrowed range.
-    let ty = cx.tables.expr_ty_adjusted(expr);
-
-    get_trait_def_id(cx, &paths::ITERATOR).map_or(false, |iterator_trait| implements_trait(cx, ty, iterator_trait, &[]))
-}
-
 fn y_plus_one<'t>(cx: &LateContext<'_, '_>, expr: &'t Expr) -> Option<&'t Expr> {
     match expr.kind {
         ExprKind::Binary(