From 6558956301bb314c14542e264fde83c10e1a5422 Mon Sep 17 00:00:00 2001
From: Quinn Sinclair <me@partiallytyped.dev>
Date: Wed, 3 Jan 2024 13:36:44 +0200
Subject: [PATCH] Fixes FP in `redundant_closure_call` when closures are passed
 to macros

There are cases where the closure call is needed in some macros, this in
particular occurs when the closure has parameters. To handle this case,
we allow the lint when there are no parameters in the closure, or the
closure is outside a macro invocation.

fixes: #11274, #1553
changelog: FP: [`redundant_closure_call`] when closures with parameters
are passed in macros.
---
 clippy_lints/src/redundant_closure_call.rs    | 31 +++++++++++++++++--
 tests/ui/redundant_closure_call_fixable.fixed |  9 ++++++
 tests/ui/redundant_closure_call_fixable.rs    |  9 ++++++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/clippy_lints/src/redundant_closure_call.rs b/clippy_lints/src/redundant_closure_call.rs
index cde08dfcc748..087feb552666 100644
--- a/clippy_lints/src/redundant_closure_call.rs
+++ b/clippy_lints/src/redundant_closure_call.rs
@@ -2,6 +2,7 @@ use crate::rustc_lint::LintContext;
 use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
 use clippy_utils::get_parent_expr;
 use clippy_utils::sugg::Sugg;
+use hir::Param;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_hir::intravisit::{Visitor as HirVisitor, Visitor};
@@ -11,6 +12,7 @@ use rustc_middle::hir::nested_filter;
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty;
 use rustc_session::declare_lint_pass;
+use rustc_span::{DesugaringKind, ExpnKind};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -87,7 +89,12 @@ fn find_innermost_closure<'tcx>(
     cx: &LateContext<'tcx>,
     mut expr: &'tcx hir::Expr<'tcx>,
     mut steps: usize,
-) -> Option<(&'tcx hir::Expr<'tcx>, &'tcx hir::FnDecl<'tcx>, ty::Asyncness)> {
+) -> Option<(
+    &'tcx hir::Expr<'tcx>,
+    &'tcx hir::FnDecl<'tcx>,
+    ty::Asyncness,
+    &'tcx [Param<'tcx>],
+)> {
     let mut data = None;
 
     while let hir::ExprKind::Closure(closure) = expr.kind
@@ -108,6 +115,7 @@ fn find_innermost_closure<'tcx>(
             } else {
                 ty::Asyncness::No
             },
+            body.params,
         ));
         steps -= 1;
     }
@@ -136,6 +144,23 @@ fn get_parent_call_exprs<'tcx>(
     (expr, depth)
 }
 
+/// This has been pulled from `in_external_macro`, and the checks for being outside the
+/// current crate have been removed.
+fn in_macro(expr: &'_ hir::Expr<'_>) -> bool {
+    let expn_data = expr.span.ctxt().outer_expn_data();
+    !matches!(
+        expn_data.kind,
+        ExpnKind::Root
+            | ExpnKind::Desugaring(
+                DesugaringKind::ForLoop
+                    | DesugaringKind::WhileLoop
+                    | DesugaringKind::OpaqueTy
+                    | DesugaringKind::Async
+                    | DesugaringKind::Await,
+            )
+    )
+}
+
 impl<'tcx> LateLintPass<'tcx> for RedundantClosureCall {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
         if in_external_macro(cx.sess(), expr.span) {
@@ -150,7 +175,9 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClosureCall {
             // without this check, we'd end up linting twice.
             && !matches!(recv.kind, hir::ExprKind::Call(..))
             && let (full_expr, call_depth) = get_parent_call_exprs(cx, expr)
-            && let Some((body, fn_decl, coroutine_kind)) = find_innermost_closure(cx, recv, call_depth)
+            && let Some((body, fn_decl, coroutine_kind, params)) = find_innermost_closure(cx, recv, call_depth)
+            // outside macros we lint properly. Inside macros, we lint only ||() style closures.
+            && (!in_macro(expr) || params.is_empty())
         {
             span_lint_and_then(
                 cx,
diff --git a/tests/ui/redundant_closure_call_fixable.fixed b/tests/ui/redundant_closure_call_fixable.fixed
index f272d8359a38..ce5c7f2600be 100644
--- a/tests/ui/redundant_closure_call_fixable.fixed
+++ b/tests/ui/redundant_closure_call_fixable.fixed
@@ -102,3 +102,12 @@ mod issue11707 {
 fn avoid_double_parens() {
     std::convert::identity(13_i32 + 36_i32).leading_zeros();
 }
+
+fn fp_11274() {
+    macro_rules! m {
+        ($closure:expr) => {
+            $closure(1)
+        };
+    }
+    m!(|x| println!("{x}"));
+}
diff --git a/tests/ui/redundant_closure_call_fixable.rs b/tests/ui/redundant_closure_call_fixable.rs
index f45db8c9cff5..ac09390e6eae 100644
--- a/tests/ui/redundant_closure_call_fixable.rs
+++ b/tests/ui/redundant_closure_call_fixable.rs
@@ -102,3 +102,12 @@ mod issue11707 {
 fn avoid_double_parens() {
     std::convert::identity((|| 13_i32 + 36_i32)()).leading_zeros();
 }
+
+fn fp_11274() {
+    macro_rules! m {
+        ($closure:expr) => {
+            $closure(1)
+        };
+    }
+    m!(|x| println!("{x}"));
+}