Add iterator reduction coverage to never_loop (#16222)

Fixes #16061.

Extend `never_loop` to also lint iterator reduction methods (e.g.
`for_each`, `try_for_each`, `fold`, `try_fold`, `reduce`, `all`, `any`)
when the closure’s body diverges (return type `!`). Add a UI test
`never_loop_iterator_reduction` to cover these cases.

Testing:
- TESTNAME=never_loop_iterator_reduction cargo uitest

changelog: [`never_loop`]: lint diverging iterator reduction closures
like `for_each` and `fold`

Author: Jarcho

clippy_lints/src/loops/mod.rs

@@ -26,6 +26,7 @@ mod while_let_on_iterator;
 
 use clippy_config::Conf;
 use clippy_utils::msrvs::Msrv;
+use clippy_utils::res::{MaybeDef, MaybeTypeckRes};
 use clippy_utils::{higher, sym};
 use rustc_ast::Label;
 use rustc_hir::{Expr, ExprKind, LoopSource, Pat};
@@ -881,13 +882,44 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
             manual_while_let_some::check(cx, condition, body, span);
         }
 
-        if let ExprKind::MethodCall(path, recv, [arg], _) = expr.kind
-            && matches!(
-                path.ident.name,
-                sym::all | sym::any | sym::filter_map | sym::find_map | sym::flat_map | sym::for_each | sym::map
-            )
-        {
-            unused_enumerate_index::check_method(cx, expr, recv, arg);
+        if let ExprKind::MethodCall(path, recv, args, _) = expr.kind {
+            let name = path.ident.name;
+
+            let is_iterator_method = || {
+                cx.ty_based_def(expr)
+                    .assoc_fn_parent(cx)
+                    .is_diag_item(cx, sym::Iterator)
+            };
+
+            // is_iterator_method is a bit expensive, so we call it last in each match arm
+            match (name, args) {
+                (sym::for_each | sym::all | sym::any, [arg]) => {
+                    if let ExprKind::Closure(closure) = arg.kind
+                        && is_iterator_method()
+                    {
+                        unused_enumerate_index::check_method(cx, recv, arg, closure);
+                        never_loop::check_iterator_reduction(cx, expr, recv, closure);
+                    }
+                },
+
+                (sym::filter_map | sym::find_map | sym::flat_map | sym::map, [arg]) => {
+                    if let ExprKind::Closure(closure) = arg.kind
+                        && is_iterator_method()
+                    {
+                        unused_enumerate_index::check_method(cx, recv, arg, closure);
+                    }
+                },
+
+                (sym::try_for_each | sym::reduce, [arg]) | (sym::fold | sym::try_fold, [_, arg]) => {
+                    if let ExprKind::Closure(closure) = arg.kind
+                        && is_iterator_method()
+                    {
+                        never_loop::check_iterator_reduction(cx, expr, recv, closure);
+                    }
+                },
+
+                _ => {},
+            }
         }
     }
 }

clippy_lints/src/loops/never_loop.rs

@@ -3,14 +3,16 @@ use super::utils::make_iterator_snippet;
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::higher::ForLoop;
 use clippy_utils::macros::root_macro_call_first_node;
-use clippy_utils::source::snippet;
+use clippy_utils::source::{snippet, snippet_with_context};
+use clippy_utils::sym;
 use clippy_utils::visitors::{Descend, for_each_expr_without_closures};
 use rustc_errors::Applicability;
 use rustc_hir::{
-    Block, Destination, Expr, ExprKind, HirId, InlineAsm, InlineAsmOperand, Node, Pat, Stmt, StmtKind, StructTailExpr,
+    Block, Closure, Destination, Expr, ExprKind, HirId, InlineAsm, InlineAsmOperand, Node, Pat, Stmt, StmtKind,
+    StructTailExpr,
 };
 use rustc_lint::LateContext;
-use rustc_span::{BytePos, Span, sym};
+use rustc_span::{BytePos, Span};
 use std::iter::once;
 use std::ops::ControlFlow;
 
@@ -72,6 +74,31 @@ pub(super) fn check<'tcx>(
     }
 }
 
+pub(super) fn check_iterator_reduction<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'tcx>,
+    recv: &'tcx Expr<'tcx>,
+    closure: &'tcx Closure<'tcx>,
+) {
+    let closure_body = cx.tcx.hir_body(closure.body).value;
+    let body_ty = cx.typeck_results().expr_ty(closure_body);
+    if body_ty.is_never() {
+        span_lint_and_then(
+            cx,
+            NEVER_LOOP,
+            expr.span,
+            "this iterator reduction never loops (closure always diverges)",
+            |diag| {
+                let mut app = Applicability::HasPlaceholders;
+                let recv_snip = snippet_with_context(cx, recv.span, expr.span.ctxt(), "<iter>", &mut app).0;
+                diag.note("if you only need one element, `if let Some(x) = iter.next()` is clearer");
+                let sugg = format!("if let Some(x) = {recv_snip}.next() {{ ... }}");
+                diag.span_suggestion_verbose(expr.span, "consider this pattern", sugg, app);
+            },
+        );
+    }
+}
+
 fn contains_any_break_or_continue(block: &Block<'_>) -> bool {
     for_each_expr_without_closures(block, |e| match e.kind {
         ExprKind::Break(..) | ExprKind::Continue(..) => ControlFlow::Break(()),

clippy_lints/src/loops/unused_enumerate_index.rs

@@ -1,10 +1,10 @@
 use super::UNUSED_ENUMERATE_INDEX;
 use clippy_utils::diagnostics::span_lint_hir_and_then;
-use clippy_utils::res::{MaybeDef, MaybeTypeckRes};
+use clippy_utils::res::MaybeDef;
 use clippy_utils::source::{SpanRangeExt, walk_span_to_context};
 use clippy_utils::{expr_or_init, pat_is_wild};
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind, Pat, PatKind, TyKind};
+use rustc_hir::{Closure, Expr, ExprKind, Pat, PatKind, TyKind};
 use rustc_lint::LateContext;
 use rustc_span::{Span, SyntaxContext, sym};
 
@@ -60,14 +60,12 @@ pub(super) fn check<'tcx>(
 
 pub(super) fn check_method<'tcx>(
     cx: &LateContext<'tcx>,
-    e: &'tcx Expr<'tcx>,
     recv: &'tcx Expr<'tcx>,
     arg: &'tcx Expr<'tcx>,
+    closure: &'tcx Closure<'tcx>,
 ) {
-    if let ExprKind::Closure(closure) = arg.kind
-        && let body = cx.tcx.hir_body(closure.body)
-        && let [param] = body.params
-        && cx.ty_based_def(e).opt_parent(cx).is_diag_item(cx, sym::Iterator)
+    let body = cx.tcx.hir_body(closure.body);
+    if let [param] = body.params
         && let [input] = closure.fn_decl.inputs
         && !arg.span.from_expansion()
         && !input.span.from_expansion()

clippy_utils/src/sym.rs

@@ -275,6 +275,7 @@ generate! {
     read_to_string,
     read_unaligned,
     read_volatile,
+    reduce,
     redundant_imports,
     redundant_pub_crate,
     regex,
@@ -364,6 +365,7 @@ generate! {
     trim_start,
     trim_start_matches,
     truncate,
+    try_fold,
     try_for_each,
     unreachable_pub,
     unsafe_removed_from_name,

tests/ui/never_loop_iterator_reduction.rs

@@ -0,0 +1,17 @@
+//@no-rustfix
+#![warn(clippy::never_loop)]
+
+fn main() {
+    // diverging closure: should trigger
+    [0, 1].into_iter().for_each(|x| {
+        //~^ never_loop
+
+        let _ = x;
+        panic!("boom");
+    });
+
+    // benign closure: should NOT trigger
+    [0, 1].into_iter().for_each(|x| {
+        let _ = x + 1;
+    });
+}

tests/ui/never_loop_iterator_reduction.stderr

@@ -0,0 +1,27 @@
+error: this iterator reduction never loops (closure always diverges)
+  --> tests/ui/never_loop_iterator_reduction.rs:6:5
+   |
+LL | /     [0, 1].into_iter().for_each(|x| {
+LL | |
+LL | |
+LL | |         let _ = x;
+LL | |         panic!("boom");
+LL | |     });
+   | |______^
+   |
+   = note: if you only need one element, `if let Some(x) = iter.next()` is clearer
+   = note: `-D clippy::never-loop` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::never_loop)]`
+help: consider this pattern
+   |
+LL -     [0, 1].into_iter().for_each(|x| {
+LL -
+LL - 
+LL -         let _ = x;
+LL -         panic!("boom");
+LL -     });
+LL +     if let Some(x) = [0, 1].into_iter().next() { ... };
+   |
+
+error: aborting due to 1 previous error
+