Rework redundant_closure

* Better track when a early-bound region appears when a late-bound region is required
* Don't lint when the closure gives explicit types.

Author: Jarcho

clippy_lints/src/derive.rs

@@ -470,12 +470,12 @@ fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_r
         if let Some(def_id) = trait_ref.trait_def_id();
         if cx.tcx.is_diagnostic_item(sym::PartialEq, def_id);
         let param_env = param_env_for_derived_eq(cx.tcx, adt.did(), eq_trait_def_id);
-        if !implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, []);
+        if !implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, &[]);
         // If all of our fields implement `Eq`, we can implement `Eq` too
         if adt
             .all_fields()
             .map(|f| f.ty(cx.tcx, substs))
-            .all(|ty| implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, []));
+            .all(|ty| implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, &[]));
         then {
             span_lint_and_sugg(
                 cx,

clippy_lints/src/eta_reduction.rs

@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then};
 use clippy_utils::ptr::get_spans;
 use clippy_utils::source::{snippet, snippet_opt};
 use clippy_utils::ty::{
-    implements_trait, implements_trait_with_env, is_copy, is_type_diagnostic_item, is_type_lang_item,
+    implements_trait, implements_trait_with_env_from_iter, is_copy, is_type_diagnostic_item, is_type_lang_item,
 };
 use clippy_utils::{get_trait_def_id, is_self, paths};
 use if_chain::if_chain;
@@ -190,7 +190,13 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue {
                 if !ty.is_mutable_ptr();
                 if !is_copy(cx, ty);
                 if ty.is_sized(cx.tcx, cx.param_env);
-                if !allowed_traits.iter().any(|&t| implements_trait_with_env(cx.tcx, cx.param_env, ty, t, [None]));
+                if !allowed_traits.iter().any(|&t| implements_trait_with_env_from_iter(
+                    cx.tcx,
+                    cx.param_env,
+                    ty,
+                    t,
+                    [Option::<ty::GenericArg<'tcx>>::None],
+                ));
                 if !implements_borrow_trait;
                 if !all_borrowable_trait;
 

clippy_lints/src/needless_pass_by_value.rs

@@ -1,5 +1,6 @@
 #![feature(array_chunks)]
 #![feature(box_patterns)]
+#![feature(is_some_and)]
 #![feature(let_chains)]
 #![feature(lint_reasons)]
 #![feature(never_type)]

clippy_utils/src/lib.rs

@@ -3,6 +3,7 @@
 #![allow(clippy::module_name_repetitions)]
 
 use core::ops::ControlFlow;
+use itertools::Itertools;
 use rustc_ast::ast::Mutability;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir as hir;
@@ -16,16 +17,17 @@ use rustc_infer::infer::{
 use rustc_lint::LateContext;
 use rustc_middle::mir::interpret::{ConstValue, Scalar};
 use rustc_middle::ty::{
-    self, AdtDef, AliasTy, AssocKind, Binder, BoundRegion, DefIdTree, FnSig, IntTy, List, ParamEnv, Predicate,
-    PredicateKind, Region, RegionKind, SubstsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor, UintTy,
-    VariantDef, VariantDiscr,
+    self, AdtDef, AliasTy, AssocKind, Binder, BoundRegion, DefIdTree, FnSig, GenericParamDefKind, IntTy, List,
+    ParamEnv, Predicate, PredicateKind, Region, RegionKind, SubstsRef, ToPredicate, Ty, TyCtxt, TypeSuperVisitable,
+    TypeVisitable, TypeVisitor, UintTy, VariantDef, VariantDiscr,
 };
 use rustc_middle::ty::{GenericArg, GenericArgKind};
 use rustc_span::symbol::Ident;
 use rustc_span::{sym, Span, Symbol, DUMMY_SP};
 use rustc_target::abi::{Size, VariantIdx};
-use rustc_trait_selection::infer::InferCtxtExt;
+use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;
 use rustc_trait_selection::traits::query::normalize::QueryNormalizeExt;
+use rustc_trait_selection::traits::{Obligation, ObligationCause};
 use std::iter;
 
 use crate::{match_def_path, path_res, paths};
@@ -206,15 +208,9 @@ pub fn implements_trait<'tcx>(
     cx: &LateContext<'tcx>,
     ty: Ty<'tcx>,
     trait_id: DefId,
-    ty_params: &[GenericArg<'tcx>],
+    substs: &[GenericArg<'tcx>],
 ) -> bool {
-    implements_trait_with_env(
-        cx.tcx,
-        cx.param_env,
-        ty,
-        trait_id,
-        ty_params.iter().map(|&arg| Some(arg)),
-    )
+    implements_trait_with_env_from_iter(cx.tcx, cx.param_env, ty, trait_id, substs.iter().map(|&x| Some(x)))
 }
 
 /// Same as `implements_trait` but allows using a `ParamEnv` different from the lint context.
@@ -223,7 +219,18 @@ pub fn implements_trait_with_env<'tcx>(
     param_env: ParamEnv<'tcx>,
     ty: Ty<'tcx>,
     trait_id: DefId,
-    ty_params: impl IntoIterator<Item = Option<GenericArg<'tcx>>>,
+    substs: &[GenericArg<'tcx>],
+) -> bool {
+    implements_trait_with_env_from_iter(tcx, param_env, ty, trait_id, substs.iter().map(|&x| Some(x)))
+}
+
+/// Same as `implements_trait_from_env` but takes the substitutions as an iterator.
+pub fn implements_trait_with_env_from_iter<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    param_env: ParamEnv<'tcx>,
+    ty: Ty<'tcx>,
+    trait_id: DefId,
+    substs: impl IntoIterator<Item = impl Into<Option<GenericArg<'tcx>>>>,
 ) -> bool {
     // Clippy shouldn't have infer types
     assert!(!ty.needs_infer());
@@ -232,19 +239,36 @@ pub fn implements_trait_with_env<'tcx>(
     if ty.has_escaping_bound_vars() {
         return false;
     }
+
     let infcx = tcx.infer_ctxt().build();
-    let orig = TypeVariableOrigin {
-        kind: TypeVariableOriginKind::MiscVariable,
-        span: DUMMY_SP,
-    };
-    let ty_params = tcx.mk_substs(
-        ty_params
+    let trait_ref = tcx.mk_trait_ref(
+        trait_id,
+        Some(GenericArg::from(ty))
             .into_iter()
-            .map(|arg| arg.unwrap_or_else(|| infcx.next_ty_var(orig).into())),
+            .chain(substs.into_iter().map(|subst| {
+                subst.into().unwrap_or_else(|| {
+                    let orig = TypeVariableOrigin {
+                        kind: TypeVariableOriginKind::MiscVariable,
+                        span: DUMMY_SP,
+                    };
+                    infcx.next_ty_var(orig).into()
+                })
+            })),
     );
+
+    debug_assert_eq!(tcx.def_kind(trait_id), DefKind::Trait);
+    #[cfg(debug_assertions)]
+    assert_substs_match(tcx, trait_id, trait_ref.substs);
+
+    let obligation = Obligation {
+        cause: ObligationCause::dummy(),
+        param_env,
+        recursion_depth: 0,
+        predicate: ty::Binder::dummy(trait_ref).without_const().to_predicate(tcx),
+    };
     infcx
-        .type_implements_trait(trait_id, [ty.into()].into_iter().chain(ty_params), param_env)
-        .must_apply_modulo_regions()
+        .evaluate_obligation(&obligation)
+        .is_ok_and(|x| x.must_apply_modulo_regions())
 }
 
 /// Checks whether this type implements `Drop`.
@@ -392,6 +416,11 @@ pub fn is_type_lang_item(cx: &LateContext<'_>, ty: Ty<'_>, lang_item: hir::LangI
     }
 }
 
+/// Gets the diagnostic name of the type, if it has one
+pub fn type_diagnostic_name(cx: &LateContext<'_>, ty: Ty<'_>) -> Option<Symbol> {
+    ty.ty_adt_def().and_then(|adt| cx.tcx.get_diagnostic_name(adt.did()))
+}
+
 /// Return `true` if the passed `typ` is `isize` or `usize`.
 pub fn is_isize_or_usize(typ: Ty<'_>) -> bool {
     matches!(typ.kind(), ty::Int(IntTy::Isize) | ty::Uint(UintTy::Usize))
@@ -1001,6 +1030,53 @@ pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {
     }
 }
 
+/// Asserts that the given substitutions matches the generic parameters of the given item.
+fn assert_substs_match<'tcx>(tcx: TyCtxt<'tcx>, did: DefId, substs: &[GenericArg<'tcx>]) {
+    let g = tcx.generics_of(did);
+    let parent = g.parent.map(|did| tcx.generics_of(did));
+    let count = g.parent_count + g.params.len();
+    let params = parent
+        .map_or([].as_slice(), |p| p.params.as_slice())
+        .iter()
+        .chain(&g.params)
+        .map(|x| &x.kind);
+
+    assert!(
+        count == substs.len(),
+        "wrong number substs for `{did:?}`: expected `{count}`, found {}\n\
+            note: the expected parameters are: `[{}]`\n\
+            the given substs are: `{substs:#?}`",
+        substs.len(),
+        params.clone().map(ty::GenericParamDefKind::descr).format(", "),
+    );
+
+    if let Some((idx, (param, arg))) =
+        params
+            .clone()
+            .zip(substs.iter().map(|&x| x.unpack()))
+            .enumerate()
+            .find(|(_, (param, arg))| match (param, arg) {
+                (GenericParamDefKind::Lifetime, GenericArgKind::Lifetime(_))
+                | (GenericParamDefKind::Type { .. }, GenericArgKind::Type(_))
+                | (GenericParamDefKind::Const { .. }, GenericArgKind::Const(_)) => false,
+                (
+                    GenericParamDefKind::Lifetime
+                    | GenericParamDefKind::Type { .. }
+                    | GenericParamDefKind::Const { .. },
+                    _,
+                ) => true,
+            })
+    {
+        panic!(
+            "incorrect subst for `{did:?}` at index `{idx}`: expected a {}, found `{arg:?}`\n\
+                note: the expected parameters are `[{}]`\n\
+                the given arguments are `{substs:#?}`",
+            param.descr(),
+            params.clone().map(ty::GenericParamDefKind::descr).format(", "),
+        );
+    }
+}
+
 /// Makes the projection type for the named associated type in the given impl or trait impl.
 ///
 /// This function is for associated types which are "known" to exist, and as such, will only return
@@ -1028,49 +1104,7 @@ pub fn make_projection<'tcx>(
             return None;
         };
         #[cfg(debug_assertions)]
-        {
-            let generics = tcx.generics_of(assoc_item.def_id);
-            let generic_count = generics.parent_count + generics.params.len();
-            let params = generics
-                .parent
-                .map_or([].as_slice(), |id| &*tcx.generics_of(id).params)
-                .iter()
-                .chain(&generics.params)
-                .map(|x| &x.kind);
-
-            debug_assert!(
-                generic_count == substs.len(),
-                "wrong number of substs for `{:?}`: found `{}` expected `{generic_count}`.\n\
-                    note: the expected parameters are: {:#?}\n\
-                    the given arguments are: `{substs:#?}`",
-                assoc_item.def_id,
-                substs.len(),
-                params.map(ty::GenericParamDefKind::descr).collect::<Vec<_>>(),
-            );
-
-            if let Some((idx, (param, arg))) = params
-                .clone()
-                .zip(substs.iter().map(GenericArg::unpack))
-                .enumerate()
-                .find(|(_, (param, arg))| {
-                    !matches!(
-                        (param, arg),
-                        (ty::GenericParamDefKind::Lifetime, GenericArgKind::Lifetime(_))
-                            | (ty::GenericParamDefKind::Type { .. }, GenericArgKind::Type(_))
-                            | (ty::GenericParamDefKind::Const { .. }, GenericArgKind::Const(_))
-                    )
-                })
-            {
-                debug_assert!(
-                    false,
-                    "mismatched subst type at index {idx}: expected a {}, found `{arg:?}`\n\
-                        note: the expected parameters are {:#?}\n\
-                        the given arguments are {substs:#?}",
-                    param.descr(),
-                    params.map(ty::GenericParamDefKind::descr).collect::<Vec<_>>()
-                );
-            }
-        }
+        assert_substs_match(tcx, assoc_item.def_id, substs);
 
         Some(tcx.mk_alias_ty(assoc_item.def_id, substs))
     }

clippy_utils/src/ty.rs

@@ -1,10 +1,11 @@
 use crate as utils;
-use crate::visitors::{for_each_expr, for_each_expr_with_closures, Descend};
+use crate::get_enclosing_loop_or_multi_call_closure;
+use crate::visitors::{for_each_expr, for_each_expr_with_closures, Descend, Visitable};
 use core::ops::ControlFlow;
 use rustc_hir as hir;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_hir::HirIdSet;
-use rustc_hir::{Expr, ExprKind, HirId, Node};
+use rustc_hir::{Expr, ExprKind, HirId};
 use rustc_hir_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::LateContext;
@@ -155,6 +156,17 @@ pub fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool {
     .is_some()
 }
 
+pub fn local_used_in<'tcx>(cx: &LateContext<'tcx>, local_id: HirId, v: impl Visitable<'tcx>) -> bool {
+    for_each_expr_with_closures(cx, v, |e| {
+        if utils::path_to_local_id(e, local_id) {
+            ControlFlow::Break(())
+        } else {
+            ControlFlow::Continue(())
+        }
+    })
+    .is_some()
+}
+
 pub fn local_used_after_expr(cx: &LateContext<'_>, local_id: HirId, after: &Expr<'_>) -> bool {
     let Some(block) = utils::get_enclosing_block(cx, local_id) else { return false };
 
@@ -165,32 +177,21 @@ pub fn local_used_after_expr(cx: &LateContext<'_>, local_id: HirId, after: &Expr
     // let closure = || local;
     // closure();
     // closure();
-    let in_loop_or_closure = cx
-        .tcx
-        .hir()
-        .parent_iter(after.hir_id)
-        .take_while(|&(id, _)| id != block.hir_id)
-        .any(|(_, node)| {
-            matches!(
-                node,
-                Node::Expr(Expr {
-                    kind: ExprKind::Loop(..) | ExprKind::Closure { .. },
-                    ..
-                })
-            )
-        });
-    if in_loop_or_closure {
-        return true;
-    }
+    let loop_start = get_enclosing_loop_or_multi_call_closure(cx, after).map(|e| e.hir_id);
 
     let mut past_expr = false;
     for_each_expr_with_closures(cx, block, |e| {
-        if e.hir_id == after.hir_id {
+        if past_expr {
+            if utils::path_to_local_id(e, local_id) {
+                ControlFlow::Break(())
+            } else {
+                ControlFlow::Continue(Descend::Yes)
+            }
+        } else if e.hir_id == after.hir_id {
             past_expr = true;
             ControlFlow::Continue(Descend::No)
-        } else if past_expr && utils::path_to_local_id(e, local_id) {
-            ControlFlow::Break(())
         } else {
+            past_expr = Some(e.hir_id) == loop_start;
             ControlFlow::Continue(Descend::Yes)
         }
     })

clippy_utils/src/usage.rs

@@ -52,6 +52,16 @@ pub trait Visitable<'tcx> {
     /// Calls the corresponding `visit_*` function on the visitor.
     fn visit<V: Visitor<'tcx>>(self, visitor: &mut V);
 }
+impl<'tcx, T> Visitable<'tcx> for &'tcx [T]
+where
+    &'tcx T: Visitable<'tcx>,
+{
+    fn visit<V: Visitor<'tcx>>(self, visitor: &mut V) {
+        for x in self {
+            x.visit(visitor);
+        }
+    }
+}
 macro_rules! visitable_ref {
     ($t:ident, $f:ident) => {
         impl<'tcx> Visitable<'tcx> for &'tcx $t<'tcx> {