Auto merge of #9134 - Jarcho:while_let_iter_closure, r=dswij

Improve `while_let_on_iterator` suggestion inside an `FnOnce` closure

changelog: Improve `while_let_on_iterator` suggestion inside an `FnOnce` closure

2 files changed, 91 insertions, 17 deletions

diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index fd0c6869929..b4b2edacb3d 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -93,7 +93,9 @@ use rustc_middle::ty::fast_reject::SimplifiedTypeGen::{
     ArraySimplifiedType, BoolSimplifiedType, CharSimplifiedType, FloatSimplifiedType, IntSimplifiedType,
     PtrSimplifiedType, SliceSimplifiedType, StrSimplifiedType, UintSimplifiedType,
 };
-use rustc_middle::ty::{layout::IntegerExt, BorrowKind, DefIdTree, Ty, TyCtxt, TypeAndMut, TypeFoldable, UpvarCapture};
+use rustc_middle::ty::{
+    layout::IntegerExt, BorrowKind, ClosureKind, DefIdTree, Ty, TyCtxt, TypeAndMut, TypeFoldable, UpvarCapture,
+};
 use rustc_middle::ty::{FloatTy, IntTy, UintTy};
 use rustc_semver::RustcVersion;
 use rustc_session::Session;
@@ -105,7 +107,7 @@ use rustc_span::{Span, DUMMY_SP};
 use rustc_target::abi::Integer;
 
 use crate::consts::{constant, Constant};
-use crate::ty::{can_partially_move_ty, is_copy, is_recursively_primitive_type};
+use crate::ty::{can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type, ty_is_fn_once_param};
 use crate::visitors::expr_visitor_no_bodies;
 
 pub fn parse_msrv(msrv: &str, sess: Option<&Session>, span: Option<Span>) -> Option<RustcVersion> {
@@ -1197,16 +1199,54 @@ pub fn get_enclosing_block<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Optio
 }
 
 /// Gets the loop or closure enclosing the given expression, if any.
-pub fn get_enclosing_loop_or_closure<'tcx>(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
-    for (_, node) in tcx.hir().parent_iter(expr.hir_id) {
+pub fn get_enclosing_loop_or_multi_call_closure<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &Expr<'_>,
+) -> Option<&'tcx Expr<'tcx>> {
+    for (_, node) in cx.tcx.hir().parent_iter(expr.hir_id) {
         match node {
-            Node::Expr(
-                e @ Expr {
-                    kind: ExprKind::Loop(..) | ExprKind::Closure { .. },
-                    ..
+            Node::Expr(e) => match e.kind {
+                ExprKind::Closure { .. } => {
+                    if let rustc_ty::Closure(_, subs) = cx.typeck_results().expr_ty(e).kind()
+                        && subs.as_closure().kind() == ClosureKind::FnOnce
+                    {
+                        continue;
+                    }
+                    let is_once = walk_to_expr_usage(cx, e, |node, id| {
+                        let Node::Expr(e) = node else {
+                            return None;
+                        };
+                        match e.kind {
+                            ExprKind::Call(f, _) if f.hir_id == id => Some(()),
+                            ExprKind::Call(f, args) => {
+                                let i = args.iter().position(|arg| arg.hir_id == id)?;
+                                let sig = expr_sig(cx, f)?;
+                                let predicates = sig
+                                    .predicates_id()
+                                    .map_or(cx.param_env, |id| cx.tcx.param_env(id))
+                                    .caller_bounds();
+                                sig.input(i).and_then(|ty| {
+                                    ty_is_fn_once_param(cx.tcx, ty.skip_binder(), predicates).then_some(())
+                                })
+                            },
+                            ExprKind::MethodCall(_, args, _) => {
+                                let i = args.iter().position(|arg| arg.hir_id == id)?;
+                                let id = cx.typeck_results().type_dependent_def_id(e.hir_id)?;
+                                let ty = cx.tcx.fn_sig(id).skip_binder().inputs()[i];
+                                ty_is_fn_once_param(cx.tcx, ty, cx.tcx.param_env(id).caller_bounds()).then_some(())
+                            },
+                            _ => None,
+                        }
+                    })
+                    .is_some();
+                    if !is_once {
+                        return Some(e);
+                    }
                 },
-            ) => return Some(e),
-            Node::Expr(_) | Node::Stmt(_) | Node::Block(_) | Node::Local(_) | Node::Arm(_) => (),
+                ExprKind::Loop(..) => return Some(e),
+                _ => (),
+            },
+            Node::Stmt(_) | Node::Block(_) | Node::Local(_) | Node::Arm(_) => (),
             _ => break,
         }
     }
diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs
index cb3d91c0919..2bfe550c895 100644
--- a/clippy_utils/src/ty.rs
+++ b/clippy_utils/src/ty.rs
@@ -501,7 +501,7 @@ pub fn all_predicates_of(tcx: TyCtxt<'_>, id: DefId) -> impl Iterator<Item = &(P
 /// A signature for a function like type.
 #[derive(Clone, Copy)]
 pub enum ExprFnSig<'tcx> {
-    Sig(Binder<'tcx, FnSig<'tcx>>),
+    Sig(Binder<'tcx, FnSig<'tcx>>, Option<DefId>),
     Closure(Option<&'tcx FnDecl<'tcx>>, Binder<'tcx, FnSig<'tcx>>),
     Trait(Binder<'tcx, Ty<'tcx>>, Option<Binder<'tcx, Ty<'tcx>>>),
 }
@@ -510,7 +510,7 @@ impl<'tcx> ExprFnSig<'tcx> {
     /// bounds only for variadic functions, otherwise this will panic.
     pub fn input(self, i: usize) -> Option<Binder<'tcx, Ty<'tcx>>> {
         match self {
-            Self::Sig(sig) => {
+            Self::Sig(sig, _) => {
                 if sig.c_variadic() {
                     sig.inputs().map_bound(|inputs| inputs.get(i).copied()).transpose()
                 } else {
@@ -527,7 +527,7 @@ impl<'tcx> ExprFnSig<'tcx> {
     /// functions, otherwise this will panic.
     pub fn input_with_hir(self, i: usize) -> Option<(Option<&'tcx hir::Ty<'tcx>>, Binder<'tcx, Ty<'tcx>>)> {
         match self {
-            Self::Sig(sig) => {
+            Self::Sig(sig, _) => {
                 if sig.c_variadic() {
                     sig.inputs()
                         .map_bound(|inputs| inputs.get(i).copied())
@@ -549,16 +549,20 @@ impl<'tcx> ExprFnSig<'tcx> {
     /// specified.
     pub fn output(self) -> Option<Binder<'tcx, Ty<'tcx>>> {
         match self {
-            Self::Sig(sig) | Self::Closure(_, sig) => Some(sig.output()),
+            Self::Sig(sig, _) | Self::Closure(_, sig) => Some(sig.output()),
             Self::Trait(_, output) => output,
         }
     }
+
+    pub fn predicates_id(&self) -> Option<DefId> {
+        if let ExprFnSig::Sig(_, id) = *self { id } else { None }
+    }
 }
 
 /// If the expression is function like, get the signature for it.
 pub fn expr_sig<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option<ExprFnSig<'tcx>> {
     if let Res::Def(DefKind::Fn | DefKind::Ctor(_, CtorKind::Fn) | DefKind::AssocFn, id) = path_res(cx, expr) {
-        Some(ExprFnSig::Sig(cx.tcx.fn_sig(id)))
+        Some(ExprFnSig::Sig(cx.tcx.fn_sig(id), Some(id)))
     } else {
         ty_sig(cx, cx.typeck_results().expr_ty_adjusted(expr).peel_refs())
     }
@@ -575,9 +579,9 @@ fn ty_sig<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<ExprFnSig<'tcx>>
                 .and_then(|id| cx.tcx.hir().fn_decl_by_hir_id(cx.tcx.hir().local_def_id_to_hir_id(id)));
             Some(ExprFnSig::Closure(decl, subs.as_closure().sig()))
         },
-        ty::FnDef(id, subs) => Some(ExprFnSig::Sig(cx.tcx.bound_fn_sig(id).subst(cx.tcx, subs))),
+        ty::FnDef(id, subs) => Some(ExprFnSig::Sig(cx.tcx.bound_fn_sig(id).subst(cx.tcx, subs), Some(id))),
         ty::Opaque(id, _) => ty_sig(cx, cx.tcx.type_of(id)),
-        ty::FnPtr(sig) => Some(ExprFnSig::Sig(sig)),
+        ty::FnPtr(sig) => Some(ExprFnSig::Sig(sig, None)),
         ty::Dynamic(bounds, _) => {
             let lang_items = cx.tcx.lang_items();
             match bounds.principal() {
@@ -793,3 +797,33 @@ pub fn variant_of_res<'tcx>(cx: &LateContext<'tcx>, res: Res) -> Option<&'tcx Va
         _ => None,
     }
 }
+
+/// Checks if the type is a type parameter implementing `FnOnce`, but not `FnMut`.
+pub fn ty_is_fn_once_param<'tcx>(tcx: TyCtxt<'_>, ty: Ty<'tcx>, predicates: &'tcx [Predicate<'_>]) -> bool {
+    let ty::Param(ty) = *ty.kind() else {
+        return false;
+    };
+    let lang = tcx.lang_items();
+    let (Some(fn_once_id), Some(fn_mut_id), Some(fn_id))
+        = (lang.fn_once_trait(), lang.fn_mut_trait(), lang.fn_trait())
+    else {
+        return false;
+    };
+    predicates
+        .iter()
+        .try_fold(false, |found, p| {
+            if let PredicateKind::Trait(p) = p.kind().skip_binder()
+            && let ty::Param(self_ty) = p.trait_ref.self_ty().kind()
+            && ty.index == self_ty.index
+        {
+            // This should use `super_traits_of`, but that's a private function.
+            if p.trait_ref.def_id == fn_once_id {
+                return Some(true);
+            } else if p.trait_ref.def_id == fn_mut_id || p.trait_ref.def_id == fn_id {
+                return None;
+            }
+        }
+            Some(found)
+        })
+        .unwrap_or(false)
+}