Add iterator reduction coverage to never_loop

Author: cuongleqq

clippy_lints/src/loops/mod.rs

@@ -26,7 +26,7 @@ mod while_let_on_iterator;
 
 use clippy_config::Conf;
 use clippy_utils::msrvs::Msrv;
-use clippy_utils::{higher, sym};
+use clippy_utils::{higher, sym, ty};
 use rustc_ast::Label;
 use rustc_hir::{Expr, ExprKind, LoopSource, Pat};
 use rustc_lint::{LateContext, LateLintPass};
@@ -889,6 +889,22 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
         {
             unused_enumerate_index::check_method(cx, expr, recv, arg);
         }
+
+        if let ExprKind::MethodCall(path, recv, args, _) = expr.kind
+            && matches!(
+                path.ident.name,
+                sym::for_each
+                    | sym::try_for_each
+                    | sym::fold
+                    | sym::try_fold
+                    | sym::reduce
+                    | sym::all
+                    | sym::any
+            )
+            && ty::get_iterator_item_ty(cx, cx.typeck_results().expr_ty(recv)).is_some()
+        {
+            never_loop::check_iterator_reduction(cx, expr, recv, args);
+        }
     }
 }
 

clippy_lints/src/loops/never_loop.rs

@@ -4,13 +4,14 @@ 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::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,
 };
 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 +73,55 @@ pub(super) fn check<'tcx>(
     }
 }
 
+pub(super) fn check_iterator_reduction<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'tcx>,
+    recv: &'tcx Expr<'tcx>,
+    args: &'tcx [Expr<'tcx>],
+) {
+    // identify which argument is the closure based on the method kind
+    let Some(method_name) = (match expr.kind {
+        ExprKind::MethodCall(path, ..) => Some(path.ident.name),
+        _ => None,
+    }) else {
+        return;
+    };
+
+    let closure_arg = match method_name {
+        sym::for_each | sym::try_for_each | sym::reduce | sym::all | sym::any => args.get(0),
+        sym::fold | sym::try_fold => args.get(1),
+        _ => None,
+    };
+
+    let Some(closure_arg) = closure_arg else {
+        return;
+    };
+
+    // extract the closure body
+    let closure_body = match closure_arg.kind {
+        ExprKind::Closure(closure) => cx.tcx.hir_body(closure.body).value,
+        _ => return,
+    };
+
+    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::Unspecified;
+                let recv_snip = make_iterator_snippet(cx, recv, &mut app);
+                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);
+            },
+        );
+        return;
+    }
+}
+
 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_utils/src/sym.rs

@@ -66,6 +66,7 @@ generate! {
     Regex,
     RegexBuilder,
     RegexSet,
+    reduce,
     Start,
     Symbol,
     SyntaxContext,
@@ -364,7 +365,9 @@ generate! {
     trim_start,
     trim_start_matches,
     truncate,
+    try_fold,
     try_for_each,
+    try_reduce,
     unreachable_pub,
     unsafe_removed_from_name,
     unused,

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
+