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.

2 files changed, 213 insertions, 130 deletions

diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs
index 22e10accd35..255859da495 100644
--- a/clippy_lints/src/eta_reduction.rs
+++ b/clippy_lints/src/eta_reduction.rs
@@ -1,19 +1,22 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::higher::VecArgs;
 use clippy_utils::source::snippet_opt;
-use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
-use clippy_utils::usage::local_used_after_expr;
+use clippy_utils::ty::type_diagnostic_name;
+use clippy_utils::usage::{local_used_after_expr, local_used_in};
 use clippy_utils::{higher, is_adjusted, path_to_local, path_to_local_id};
-use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::def_id::DefId;
-use rustc_hir::{Closure, Expr, ExprKind, Param, PatKind, Unsafety};
+use rustc_hir::{BindingAnnotation, Expr, ExprKind, FnRetTy, Param, PatKind, TyKind, Unsafety};
+use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
-use rustc_middle::ty::binding::BindingMode;
-use rustc_middle::ty::{self, EarlyBinder, GenericArgsRef, Ty, TypeVisitableExt};
+use rustc_middle::ty::{
+    self, Binder, BoundConstness, ClosureArgs, ClosureKind, EarlyBinder, FnSig, GenericArg, GenericArgKind,
+    GenericArgsRef, ImplPolarity, List, Region, RegionKind, Ty, TypeVisitableExt, TypeckResults,
+};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::symbol::sym;
+use rustc_target::spec::abi::Abi;
+use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -72,14 +75,18 @@ declare_clippy_lint! {
 declare_lint_pass!(EtaReduction => [REDUNDANT_CLOSURE, REDUNDANT_CLOSURE_FOR_METHOD_CALLS]);
 
 impl<'tcx> LateLintPass<'tcx> for EtaReduction {
+    #[allow(clippy::too_many_lines)]
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        if expr.span.from_expansion() {
+        let body = if let ExprKind::Closure(c) = expr.kind
+            && c.fn_decl.inputs.iter().all(|ty| matches!(ty.kind, TyKind::Infer))
+            && matches!(c.fn_decl.output, FnRetTy::DefaultReturn(_))
+            && !expr.span.from_expansion()
+        {
+            cx.tcx.hir().body(c.body)
+        } else {
             return;
-        }
-        let body = match expr.kind {
-            ExprKind::Closure(&Closure { body, .. }) => cx.tcx.hir().body(body),
-            _ => return,
         };
+
         if body.value.span.from_expansion() {
             if body.params.is_empty() {
                 if let Some(VecArgs::Vec(&[])) = higher::VecArgs::hir(cx, body.value) {
@@ -99,140 +106,217 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
             return;
         }
 
-        let closure_ty = cx.typeck_results().expr_ty(expr);
+        let typeck = cx.typeck_results();
+        let closure = if let ty::Closure(_, closure_subs) = typeck.expr_ty(expr).kind() {
+            closure_subs.as_closure()
+        } else {
+            return;
+        };
 
-        if_chain!(
-            if !is_adjusted(cx, body.value);
-            if let ExprKind::Call(callee, args) = body.value.kind;
-            if let ExprKind::Path(_) = callee.kind;
-            if check_inputs(cx, body.params, None, args);
-            let callee_ty = cx.typeck_results().expr_ty_adjusted(callee);
-            let call_ty = cx.typeck_results().type_dependent_def_id(body.value.hir_id)
-                .map_or(callee_ty, |id| cx.tcx.type_of(id).instantiate_identity());
-            if check_sig(cx, closure_ty, call_ty);
-            let args = cx.typeck_results().node_args(callee.hir_id);
-            // This fixes some false positives that I don't entirely understand
-            if args.is_empty() || !cx.typeck_results().expr_ty(expr).has_late_bound_regions();
-            // A type param function ref like `T::f` is not 'static, however
-            // it is if cast like `T::f as fn()`. This seems like a rustc bug.
-            if !args.types().any(|t| matches!(t.kind(), ty::Param(_)));
-            let callee_ty_unadjusted = cx.typeck_results().expr_ty(callee).peel_refs();
-            if !is_type_diagnostic_item(cx, callee_ty_unadjusted, sym::Arc);
-            if !is_type_diagnostic_item(cx, callee_ty_unadjusted, sym::Rc);
-            if let ty::Closure(_, args) = *closure_ty.kind();
-            // Don't lint if this is an inclusive range expression.
-            // They desugar to a call to `RangeInclusiveNew` which would have odd suggestions. (#10684)
-            if !matches!(higher::Range::hir(body.value), Some(higher::Range {
-                start: Some(_),
-                end: Some(_),
-                limits: rustc_ast::RangeLimits::Closed
-            }));
-            then {
-                span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| {
-                    if let Some(mut snippet) = snippet_opt(cx, callee.span) {
-                        if let Some(fn_mut_id) = cx.tcx.lang_items().fn_mut_trait()
-                            && let args = cx.tcx.erase_late_bound_regions(args.as_closure().sig()).inputs()
-                            && implements_trait(
-                                   cx,
-                                   callee_ty.peel_refs(),
-                                   fn_mut_id,
-                                   &args.iter().copied().map(Into::into).collect::<Vec<_>>(),
-                               )
-                            && path_to_local(callee).map_or(false, |l| local_used_after_expr(cx, l, expr))
-                        {
-                                // Mutable closure is used after current expr; we cannot consume it.
-                                snippet = format!("&mut {snippet}");
-                        }
+        if is_adjusted(cx, body.value) {
+            return;
+        }
 
-                        diag.span_suggestion(
-                            expr.span,
-                            "replace the closure with the function itself",
-                            snippet,
-                            Applicability::MachineApplicable,
-                        );
-                    }
-                });
-            }
-        );
+        match body.value.kind {
+            ExprKind::Call(callee, args)
+                if matches!(callee.kind, ExprKind::Path(..)) =>
+            {
+                if matches!(higher::Range::hir(body.value), Some(higher::Range {
+                    start: Some(_),
+                    end: Some(_),
+                    limits: rustc_ast::RangeLimits::Closed
+                })) {
+                    return;
+                }
+
+                let callee_ty = typeck.expr_ty(callee).peel_refs();
+                if matches!(
+                    type_diagnostic_name(cx, callee_ty),
+                    Some(sym::Arc | sym::Rc)
+                ) || !check_inputs(typeck, body.params, None, args) {
+                    return;
+                }
+                let callee_ty_adjusted = typeck.expr_adjustments(callee).last().map_or(
+                    callee_ty,
+                    |a| a.target.peel_refs(),
+                );
 
-        if_chain!(
-            if !is_adjusted(cx, body.value);
-            if let ExprKind::MethodCall(path, receiver, args, _) = body.value.kind;
-            if check_inputs(cx, body.params, Some(receiver), args);
-            let method_def_id = cx.typeck_results().type_dependent_def_id(body.value.hir_id).unwrap();
-            let args = cx.typeck_results().node_args(body.value.hir_id);
-            let call_ty = cx.tcx.type_of(method_def_id).instantiate(cx.tcx, args);
-            if check_sig(cx, closure_ty, call_ty);
-            then {
-                span_lint_and_then(cx, REDUNDANT_CLOSURE_FOR_METHOD_CALLS, expr.span, "redundant closure", |diag| {
-                    let name = get_ufcs_type_name(cx, method_def_id, args);
-                    diag.span_suggestion(
+                let sig = match callee_ty_adjusted.kind() {
+                    ty::FnDef(def, _) => cx.tcx.fn_sig(def).skip_binder().skip_binder(),
+                    ty::FnPtr(sig) => sig.skip_binder(),
+                    ty::Closure(_, subs) => cx
+                        .tcx
+                        .signature_unclosure(subs.as_closure().sig(), Unsafety::Normal)
+                        .skip_binder(),
+                    _ => {
+                        if typeck.type_dependent_def_id(body.value.hir_id).is_some()
+                            && let subs = typeck.node_args(body.value.hir_id)
+                            && let output = typeck.expr_ty(body.value)
+                            && let ty::Tuple(tys) = *subs.type_at(1).kind()
+                        {
+                            cx.tcx.mk_fn_sig(tys, output, false, Unsafety::Normal, Abi::Rust)
+                        } else {
+                            return;
+                        }
+                    },
+                };
+                if check_sig(cx, closure, sig)
+                    && let generic_args = typeck.node_args(callee.hir_id)
+                    // Given some trait fn `fn f() -> ()` and some type `T: Trait`, `T::f` is not
+                    // `'static` unless `T: 'static`. The cast `T::f as fn()` will, however, result
+                    // in a type which is `'static`.
+                    // For now ignore all callee types which reference a type parameter.
+                    && !generic_args.types().any(|t| matches!(t.kind(), ty::Param(_)))
+                {
+                    span_lint_and_then(
+                        cx,
+                        REDUNDANT_CLOSURE,
                         expr.span,
-                        "replace the closure with the method itself",
-                        format!("{name}::{}", path.ident.name),
-                        Applicability::MachineApplicable,
+                        "redundant closure",
+                        |diag| {
+                            if let Some(mut snippet) = snippet_opt(cx, callee.span) {
+                                if let Ok((ClosureKind::FnMut, _))
+                                    = cx.tcx.infer_ctxt().build().type_implements_fn_trait(
+                                        cx.param_env,
+                                        Binder::bind_with_vars(callee_ty_adjusted, List::empty()),
+                                        BoundConstness::NotConst,
+                                        ImplPolarity::Positive,
+                                    ) && path_to_local(callee)
+                                        .map_or(
+                                            false,
+                                            |l| local_used_in(cx, l, args) || local_used_after_expr(cx, l, expr),
+                                        )
+                                {
+                                    // Mutable closure is used after current expr; we cannot consume it.
+                                    snippet = format!("&mut {snippet}");
+                                }
+                                diag.span_suggestion(
+                                    expr.span,
+                                    "replace the closure with the function itself",
+                                    snippet,
+                                    Applicability::MachineApplicable,
+                                );
+                            }
+                        }
                     );
-                })
-            }
-        );
+                }
+            },
+            ExprKind::MethodCall(path, self_, args, _) if check_inputs(typeck, body.params, Some(self_), args) => {
+                if let Some(method_def_id) = typeck.type_dependent_def_id(body.value.hir_id)
+                    && check_sig(cx, closure, cx.tcx.fn_sig(method_def_id).skip_binder().skip_binder())
+                {
+                    span_lint_and_then(
+                        cx,
+                        REDUNDANT_CLOSURE_FOR_METHOD_CALLS,
+                        expr.span,
+                        "redundant closure",
+                        |diag| {
+                            let args = typeck.node_args(body.value.hir_id);
+                            let name = get_ufcs_type_name(cx, method_def_id, args);
+                            diag.span_suggestion(
+                                expr.span,
+                                "replace the closure with the method itself",
+                                format!("{}::{}", name, path.ident.name),
+                                Applicability::MachineApplicable,
+                            );
+                        },
+                    );
+                }
+            },
+            _ => (),
+        }
     }
 }
 
 fn check_inputs(
-    cx: &LateContext<'_>,
+    typeck: &TypeckResults<'_>,
     params: &[Param<'_>],
-    receiver: Option<&Expr<'_>>,
-    call_args: &[Expr<'_>],
+    self_arg: Option<&Expr<'_>>,
+    args: &[Expr<'_>],
 ) -> bool {
-    if receiver.map_or(params.len() != call_args.len(), |_| params.len() != call_args.len() + 1) {
-        return false;
+    params.len() == self_arg.map_or(0, |_| 1) + args.len()
+        && params.iter().zip(self_arg.into_iter().chain(args)).all(|(p, arg)| {
+            matches!(
+                p.pat.kind,PatKind::Binding(BindingAnnotation::NONE, id, _, None)
+                if path_to_local_id(arg, id)
+            )
+            // Only allow adjustments which change regions (i.e. re-borrowing).
+            && typeck
+                .expr_adjustments(arg)
+                .last()
+                .map_or(true, |a| a.target == typeck.expr_ty(arg))
+        })
+}
+
+fn check_sig<'tcx>(cx: &LateContext<'tcx>, closure: ClosureArgs<'tcx>, call_sig: FnSig<'_>) -> bool {
+    call_sig.unsafety == Unsafety::Normal
+        && !has_late_bound_to_non_late_bound_regions(
+            cx.tcx
+                .signature_unclosure(closure.sig(), Unsafety::Normal)
+                .skip_binder(),
+            call_sig,
+        )
+}
+
+/// This walks through both signatures and checks for any time a late-bound region is expected by an
+/// `impl Fn` type, but the target signature does not have a late-bound region in the same position.
+///
+/// This is needed because rustc is unable to late bind early-bound regions in a function signature.
+fn has_late_bound_to_non_late_bound_regions(from_sig: FnSig<'_>, to_sig: FnSig<'_>) -> bool {
+    fn check_region(from_region: Region<'_>, to_region: Region<'_>) -> bool {
+        matches!(from_region.kind(), RegionKind::ReLateBound(..))
+            && !matches!(to_region.kind(), RegionKind::ReLateBound(..))
     }
-    let binding_modes = cx.typeck_results().pat_binding_modes();
-    let check_inputs = |param: &Param<'_>, arg| {
-        match param.pat.kind {
-            PatKind::Binding(_, id, ..) if path_to_local_id(arg, id) => {},
-            _ => return false,
-        }
-        // checks that parameters are not bound as `ref` or `ref mut`
-        if let Some(BindingMode::BindByReference(_)) = binding_modes.get(param.pat.hir_id) {
-            return false;
-        }
 
-        match *cx.typeck_results().expr_adjustments(arg) {
-            [] => true,
-            [
-                Adjustment {
-                    kind: Adjust::Deref(None),
-                    ..
+    fn check_subs(from_subs: &[GenericArg<'_>], to_subs: &[GenericArg<'_>]) -> bool {
+        if from_subs.len() != to_subs.len() {
+            return true;
+        }
+        for (from_arg, to_arg) in to_subs.iter().zip(from_subs) {
+            match (from_arg.unpack(), to_arg.unpack()) {
+                (GenericArgKind::Lifetime(from_region), GenericArgKind::Lifetime(to_region)) => {
+                    if check_region(from_region, to_region) {
+                        return true;
+                    }
                 },
-                Adjustment {
-                    kind: Adjust::Borrow(AutoBorrow::Ref(_, mu2)),
-                    ..
+                (GenericArgKind::Type(from_ty), GenericArgKind::Type(to_ty)) => {
+                    if check_ty(from_ty, to_ty) {
+                        return true;
+                    }
                 },
-            ] => {
-                // re-borrow with the same mutability is allowed
-                let ty = cx.typeck_results().expr_ty(arg);
-                matches!(*ty.kind(), ty::Ref(.., mu1) if mu1 == mu2.into())
-            },
-            _ => false,
+                (GenericArgKind::Const(_), GenericArgKind::Const(_)) => (),
+                _ => return true,
+            }
         }
-    };
-    std::iter::zip(params, receiver.into_iter().chain(call_args.iter())).all(|(param, arg)| check_inputs(param, arg))
-}
-
-fn check_sig<'tcx>(cx: &LateContext<'tcx>, closure_ty: Ty<'tcx>, call_ty: Ty<'tcx>) -> bool {
-    let call_sig = call_ty.fn_sig(cx.tcx);
-    if call_sig.unsafety() == Unsafety::Unsafe {
-        return false;
+        false
     }
-    if !closure_ty.has_late_bound_regions() {
-        return true;
+
+    fn check_ty(from_ty: Ty<'_>, to_ty: Ty<'_>) -> bool {
+        match (from_ty.kind(), to_ty.kind()) {
+            (&ty::Adt(_, from_subs), &ty::Adt(_, to_subs)) => check_subs(from_subs, to_subs),
+            (&ty::Array(from_ty, _), &ty::Array(to_ty, _)) | (&ty::Slice(from_ty), &ty::Slice(to_ty)) => {
+                check_ty(from_ty, to_ty)
+            },
+            (&ty::Ref(from_region, from_ty, _), &ty::Ref(to_region, to_ty, _)) => {
+                check_region(from_region, to_region) || check_ty(from_ty, to_ty)
+            },
+            (&ty::Tuple(from_tys), &ty::Tuple(to_tys)) => {
+                from_tys.len() != to_tys.len()
+                    || from_tys
+                        .iter()
+                        .zip(to_tys)
+                        .any(|(from_ty, to_ty)| check_ty(from_ty, to_ty))
+            },
+            _ => from_ty.has_late_bound_regions(),
+        }
     }
-    let ty::Closure(_, args) = closure_ty.kind() else {
-        return false;
-    };
-    let closure_sig = cx.tcx.signature_unclosure(args.as_closure().sig(), Unsafety::Normal);
-    cx.tcx.erase_late_bound_regions(closure_sig) == cx.tcx.erase_late_bound_regions(call_sig)
+
+    assert!(from_sig.inputs_and_output.len() == to_sig.inputs_and_output.len());
+    from_sig
+        .inputs_and_output
+        .iter()
+        .zip(to_sig.inputs_and_output)
+        .any(|(from_ty, to_ty)| check_ty(from_ty, to_ty))
 }
 
 fn get_ufcs_type_name<'tcx>(cx: &LateContext<'tcx>, method_def_id: DefId, args: GenericArgsRef<'tcx>) -> String {
@@ -241,7 +325,7 @@ fn get_ufcs_type_name<'tcx>(cx: &LateContext<'tcx>, method_def_id: DefId, args:
     match assoc_item.container {
         ty::TraitContainer => cx.tcx.def_path_str(def_id),
         ty::ImplContainer => {
-            let ty = cx.tcx.type_of(def_id).skip_binder();
+            let ty = cx.tcx.type_of(def_id).instantiate_identity();
             match ty.kind() {
                 ty::Adt(adt, _) => cx.tcx.def_path_str(adt.did()),
                 ty::Array(..)
diff --git a/clippy_lints/src/incorrect_impls.rs b/clippy_lints/src/incorrect_impls.rs
index f4edb17ae80..c19a46f4e97 100644
--- a/clippy_lints/src/incorrect_impls.rs
+++ b/clippy_lints/src/incorrect_impls.rs
@@ -189,7 +189,6 @@ impl LateLintPass<'_> for IncorrectImpls {
                 .diagnostic_items(trait_impl.def_id.krate)
                 .name_to_id
                 .get(&sym::Ord)
-            && trait_impl.self_ty() == trait_impl.args.type_at(1)
             && implements_trait(cx, hir_ty_to_ty(cx.tcx, imp.self_ty), *ord_def_id, &[])
         {
             // If the `cmp` call likely needs to be fully qualified in the suggestion