Auto merge of rust-lang#6824 - Y-Nak:refactor_loops_module, r=flip1995

Refactor: organize loops file into loops module (Delegated)

`@flip1995` `@nahuakang`

closes rust-lang#6693
r? `@flip1995`

 As we talked about in the PM of Zulip,  this PR is a delegated PR from `@nahuakang.`

Changes from the last commit of rust-lang#6693:
1. Unify the name of the main entries of all modules to check, that was pointed out [here](rust-lang/rust-clippy#6693 (comment))
2. Simplify ` check_for_loop_arg`, that was pointed out [here](rust-lang/rust-clippy#6693 (comment)) and [here](rust-lang/rust-clippy#6693 (comment))
3. Resolve conflicts

changelog: Refactor `loops.rs` file into `loops` module.

clippy_lints/src/loops/empty_loop.rs

@@ -0,0 +1,17 @@
+use super::EMPTY_LOOP;
+use crate::utils::{is_in_panic_handler, is_no_std_crate, span_lint_and_help};
+
+use rustc_hir::{Block, Expr};
+use rustc_lint::LateContext;
+
+pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_block: &'tcx Block<'_>) {
+    if loop_block.stmts.is_empty() && loop_block.expr.is_none() && !is_in_panic_handler(cx, expr) {
+        let msg = "empty `loop {}` wastes CPU cycles";
+        let help = if is_no_std_crate(cx.tcx.hir().krate()) {
+            "you should either use `panic!()` or add a call pausing or sleeping the thread to the loop body"
+        } else {
+            "you should either use `panic!()` or add `std::thread::sleep(..);` to the loop body"
+        };
+        span_lint_and_help(cx, EMPTY_LOOP, expr.span, msg, None, help);
+    }
+}

clippy_lints/src/loops/explicit_counter_loop.rs

@@ -0,0 +1,58 @@
+use super::{
+    get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor, EXPLICIT_COUNTER_LOOP,
+};
+use crate::utils::{get_enclosing_block, is_integer_const, snippet_with_applicability, span_lint_and_sugg};
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::intravisit::{walk_block, walk_expr};
+use rustc_hir::{Expr, Pat};
+use rustc_lint::LateContext;
+
+// To trigger the EXPLICIT_COUNTER_LOOP lint, a variable must be
+// incremented exactly once in the loop body, and initialized to zero
+// at the start of the loop.
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    pat: &'tcx Pat<'_>,
+    arg: &'tcx Expr<'_>,
+    body: &'tcx Expr<'_>,
+    expr: &'tcx Expr<'_>,
+) {
+    // Look for variables that are incremented once per loop iteration.
+    let mut increment_visitor = IncrementVisitor::new(cx);
+    walk_expr(&mut increment_visitor, body);
+
+    // For each candidate, check the parent block to see if
+    // it's initialized to zero at the start of the loop.
+    if let Some(block) = get_enclosing_block(&cx, expr.hir_id) {
+        for id in increment_visitor.into_results() {
+            let mut initialize_visitor = InitializeVisitor::new(cx, expr, id);
+            walk_block(&mut initialize_visitor, block);
+
+            if_chain! {
+                if let Some((name, initializer)) = initialize_visitor.get_result();
+                if is_integer_const(cx, initializer, 0);
+                then {
+                    let mut applicability = Applicability::MachineApplicable;
+
+                    let for_span = get_span_of_entire_for_loop(expr);
+
+                    span_lint_and_sugg(
+                        cx,
+                        EXPLICIT_COUNTER_LOOP,
+                        for_span.with_hi(arg.span.hi()),
+                        &format!("the variable `{}` is used as a loop counter", name),
+                        "consider using",
+                        format!(
+                            "for ({}, {}) in {}.enumerate()",
+                            name,
+                            snippet_with_applicability(cx, pat.span, "item", &mut applicability),
+                            make_iterator_snippet(cx, arg, &mut applicability),
+                        ),
+                        applicability,
+                    );
+                }
+            }
+        }
+    }
+}

clippy_lints/src/loops/explicit_into_iter_loop.rs

@@ -0,0 +1,27 @@
+use super::EXPLICIT_INTO_ITER_LOOP;
+use crate::utils::{snippet_with_applicability, span_lint_and_sugg};
+use rustc_errors::Applicability;
+use rustc_hir::Expr;
+use rustc_lint::LateContext;
+use rustc_middle::ty::TyS;
+
+pub(super) fn check(cx: &LateContext<'_>, args: &'hir [Expr<'hir>], arg: &Expr<'_>) {
+    let receiver_ty = cx.typeck_results().expr_ty(&args[0]);
+    let receiver_ty_adjusted = cx.typeck_results().expr_ty_adjusted(&args[0]);
+    if !TyS::same_type(receiver_ty, receiver_ty_adjusted) {
+        return;
+    }
+
+    let mut applicability = Applicability::MachineApplicable;
+    let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability);
+    span_lint_and_sugg(
+        cx,
+        EXPLICIT_INTO_ITER_LOOP,
+        arg.span,
+        "it is more concise to loop over containers instead of using explicit \
+            iteration methods",
+        "to write this more concisely, try",
+        object.to_string(),
+        applicability,
+    );
+}

clippy_lints/src/loops/explicit_iter_loop.rs

@@ -0,0 +1,74 @@
+use super::EXPLICIT_ITER_LOOP;
+use crate::utils::{match_trait_method, snippet_with_applicability, span_lint_and_sugg};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, Mutability};
+use rustc_lint::LateContext;
+use rustc_middle::ty::{self, Ty, TyS};
+use rustc_span::symbol::sym;
+
+use crate::utils::{is_type_diagnostic_item, match_type, paths};
+
+pub(super) fn check(cx: &LateContext<'_>, args: &[Expr<'_>], arg: &Expr<'_>, method_name: &str) {
+    let should_lint = match method_name {
+        "iter" | "iter_mut" => is_ref_iterable_type(cx, &args[0]),
+        "into_iter" if match_trait_method(cx, arg, &paths::INTO_ITERATOR) => {
+            let receiver_ty = cx.typeck_results().expr_ty(&args[0]);
+            let receiver_ty_adjusted = cx.typeck_results().expr_ty_adjusted(&args[0]);
+            let ref_receiver_ty = cx.tcx.mk_ref(
+                cx.tcx.lifetimes.re_erased,
+                ty::TypeAndMut {
+                    ty: receiver_ty,
+                    mutbl: Mutability::Not,
+                },
+            );
+            TyS::same_type(receiver_ty_adjusted, ref_receiver_ty)
+        },
+        _ => false,
+    };
+
+    if !should_lint {
+        return;
+    }
+
+    let mut applicability = Applicability::MachineApplicable;
+    let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability);
+    let muta = if method_name == "iter_mut" { "mut " } else { "" };
+    span_lint_and_sugg(
+        cx,
+        EXPLICIT_ITER_LOOP,
+        arg.span,
+        "it is more concise to loop over references to containers instead of using explicit \
+         iteration methods",
+        "to write this more concisely, try",
+        format!("&{}{}", muta, object),
+        applicability,
+    )
+}
+
+/// Returns `true` if the type of expr is one that provides `IntoIterator` impls
+/// for `&T` and `&mut T`, such as `Vec`.
+#[rustfmt::skip]
+fn is_ref_iterable_type(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
+    // no walk_ptrs_ty: calling iter() on a reference can make sense because it
+    // will allow further borrows afterwards
+    let ty = cx.typeck_results().expr_ty(e);
+    is_iterable_array(ty, cx) ||
+    is_type_diagnostic_item(cx, ty, sym::vec_type) ||
+    match_type(cx, ty, &paths::LINKED_LIST) ||
+    is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) ||
+    is_type_diagnostic_item(cx, ty, sym!(hashset_type)) ||
+    is_type_diagnostic_item(cx, ty, sym!(vecdeque_type)) ||
+    match_type(cx, ty, &paths::BINARY_HEAP) ||
+    match_type(cx, ty, &paths::BTREEMAP) ||
+    match_type(cx, ty, &paths::BTREESET)
+}
+
+fn is_iterable_array<'tcx>(ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool {
+    // IntoIterator is currently only implemented for array sizes <= 32 in rustc
+    match ty.kind() {
+        ty::Array(_, n) => n
+            .try_eval_usize(cx.tcx, cx.param_env)
+            .map_or(false, |val| (0..=32).contains(&val)),
+        _ => false,
+    }
+}

clippy_lints/src/loops/for_kv_map.rs

@@ -0,0 +1,70 @@
+use super::FOR_KV_MAP;
+use crate::utils::visitors::LocalUsedVisitor;
+use crate::utils::{is_type_diagnostic_item, match_type, multispan_sugg, paths, snippet, span_lint_and_then, sugg};
+use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Pat, PatKind};
+use rustc_lint::LateContext;
+use rustc_middle::ty;
+
+/// Checks for the `FOR_KV_MAP` lint.
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    pat: &'tcx Pat<'_>,
+    arg: &'tcx Expr<'_>,
+    body: &'tcx Expr<'_>,
+    expr: &'tcx Expr<'_>,
+) {
+    let pat_span = pat.span;
+
+    if let PatKind::Tuple(ref pat, _) = pat.kind {
+        if pat.len() == 2 {
+            let arg_span = arg.span;
+            let (new_pat_span, kind, ty, mutbl) = match *cx.typeck_results().expr_ty(arg).kind() {
+                ty::Ref(_, ty, mutbl) => match (&pat[0].kind, &pat[1].kind) {
+                    (key, _) if pat_is_wild(cx, key, body) => (pat[1].span, "value", ty, mutbl),
+                    (_, value) if pat_is_wild(cx, value, body) => (pat[0].span, "key", ty, Mutability::Not),
+                    _ => return,
+                },
+                _ => return,
+            };
+            let mutbl = match mutbl {
+                Mutability::Not => "",
+                Mutability::Mut => "_mut",
+            };
+            let arg = match arg.kind {
+                ExprKind::AddrOf(BorrowKind::Ref, _, ref expr) => &**expr,
+                _ => arg,
+            };
+
+            if is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) || match_type(cx, ty, &paths::BTREEMAP) {
+                span_lint_and_then(
+                    cx,
+                    FOR_KV_MAP,
+                    expr.span,
+                    &format!("you seem to want to iterate on a map's {}s", kind),
+                    |diag| {
+                        let map = sugg::Sugg::hir(cx, arg, "map");
+                        multispan_sugg(
+                            diag,
+                            "use the corresponding method",
+                            vec![
+                                (pat_span, snippet(cx, new_pat_span, kind).into_owned()),
+                                (arg_span, format!("{}.{}s{}()", map.maybe_par(), kind, mutbl)),
+                            ],
+                        );
+                    },
+                );
+            }
+        }
+    }
+}
+
+/// Returns `true` if the pattern is a `PatWild` or an ident prefixed with `_`.
+fn pat_is_wild<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool {
+    match *pat {
+        PatKind::Wild => true,
+        PatKind::Binding(_, id, ident, None) if ident.as_str().starts_with('_') => {
+            !LocalUsedVisitor::new(cx, id).check_expr(body)
+        },
+        _ => false,
+    }
+}

clippy_lints/src/loops/for_loops_over_fallibles.rs

@@ -0,0 +1,45 @@
+use super::FOR_LOOPS_OVER_FALLIBLES;
+use crate::utils::{is_type_diagnostic_item, snippet, span_lint_and_help};
+use rustc_hir::{Expr, Pat};
+use rustc_lint::LateContext;
+use rustc_span::symbol::sym;
+
+/// Checks for `for` loops over `Option`s and `Result`s.
+pub(super) fn check(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) {
+    let ty = cx.typeck_results().expr_ty(arg);
+    if is_type_diagnostic_item(cx, ty, sym::option_type) {
+        span_lint_and_help(
+            cx,
+            FOR_LOOPS_OVER_FALLIBLES,
+            arg.span,
+            &format!(
+                "for loop over `{0}`, which is an `Option`. This is more readably written as an \
+                `if let` statement",
+                snippet(cx, arg.span, "_")
+            ),
+            None,
+            &format!(
+                "consider replacing `for {0} in {1}` with `if let Some({0}) = {1}`",
+                snippet(cx, pat.span, "_"),
+                snippet(cx, arg.span, "_")
+            ),
+        );
+    } else if is_type_diagnostic_item(cx, ty, sym::result_type) {
+        span_lint_and_help(
+            cx,
+            FOR_LOOPS_OVER_FALLIBLES,
+            arg.span,
+            &format!(
+                "for loop over `{0}`, which is a `Result`. This is more readably written as an \
+                `if let` statement",
+                snippet(cx, arg.span, "_")
+            ),
+            None,
+            &format!(
+                "consider replacing `for {0} in {1}` with `if let Ok({0}) = {1}`",
+                snippet(cx, pat.span, "_"),
+                snippet(cx, arg.span, "_")
+            ),
+        );
+    }
+}

clippy_lints/src/loops/iter_next_loop.rs

@@ -0,0 +1,19 @@
+use super::ITER_NEXT_LOOP;
+use crate::utils::{match_trait_method, paths, span_lint};
+use rustc_hir::Expr;
+use rustc_lint::LateContext;
+
+pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>, expr: &Expr<'_>) -> bool {
+    if match_trait_method(cx, arg, &paths::ITERATOR) {
+        span_lint(
+            cx,
+            ITER_NEXT_LOOP,
+            expr.span,
+            "you are iterating over `Iterator::next()` which is an Option; this will compile but is \
+            probably not what you want",
+        );
+        true
+    } else {
+        false
+    }
+}

clippy_lints/src/loops/manual_flatten.rs

@@ -0,0 +1,79 @@
+use super::utils::make_iterator_snippet;
+use super::MANUAL_FLATTEN;
+use crate::utils::{is_ok_ctor, is_some_ctor, path_to_local_id, span_lint_and_then};
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, MatchSource, Pat, PatKind, QPath, StmtKind};
+use rustc_lint::LateContext;
+use rustc_span::source_map::Span;
+
+/// Check for unnecessary `if let` usage in a for loop where only the `Some` or `Ok` variant of the
+/// iterator element is used.
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    pat: &'tcx Pat<'_>,
+    arg: &'tcx Expr<'_>,
+    body: &'tcx Expr<'_>,
+    span: Span,
+) {
+    if let ExprKind::Block(ref block, _) = body.kind {
+        // Ensure the `if let` statement is the only expression or statement in the for-loop
+        let inner_expr = if block.stmts.len() == 1 && block.expr.is_none() {
+            let match_stmt = &block.stmts[0];
+            if let StmtKind::Semi(inner_expr) = match_stmt.kind {
+                Some(inner_expr)
+            } else {
+                None
+            }
+        } else if block.stmts.is_empty() {
+            block.expr
+        } else {
+            None
+        };
+
+        if_chain! {
+            if let Some(inner_expr) = inner_expr;
+            if let ExprKind::Match(
+                ref match_expr, ref match_arms, MatchSource::IfLetDesugar{ contains_else_clause: false }
+            ) = inner_expr.kind;
+            // Ensure match_expr in `if let` statement is the same as the pat from the for-loop
+            if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind;
+            if path_to_local_id(match_expr, pat_hir_id);
+            // Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
+            if let PatKind::TupleStruct(QPath::Resolved(None, path), _, _) = match_arms[0].pat.kind;
+            let some_ctor = is_some_ctor(cx, path.res);
+            let ok_ctor = is_ok_ctor(cx, path.res);
+            if some_ctor || ok_ctor;
+            let if_let_type = if some_ctor { "Some" } else { "Ok" };
+
+            then {
+                // Prepare the error message
+                let msg = format!("unnecessary `if let` since only the `{}` variant of the iterator element is used", if_let_type);
+
+                // Prepare the help message
+                let mut applicability = Applicability::MaybeIncorrect;
+                let arg_snippet = make_iterator_snippet(cx, arg, &mut applicability);
+
+                span_lint_and_then(
+                    cx,
+                    MANUAL_FLATTEN,
+                    span,
+                    &msg,
+                    |diag| {
+                        let sugg = format!("{}.flatten()", arg_snippet);
+                        diag.span_suggestion(
+                            arg.span,
+                            "try",
+                            sugg,
+                            Applicability::MaybeIncorrect,
+                        );
+                        diag.span_help(
+                            inner_expr.span,
+                            "...and remove the `if let` statement in the for loop",
+                        );
+                    }
+                );
+            }
+        }
+    }
+}