Auto merge of #12082 - PartiallyTyped:1553, r=dswij

bors · bors · commit 8905f78832c4 · 2024-01-27T09:26:35.000Z
Fixed 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.
diff --git 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};
@@ -13,6 +14,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::ExpnKind;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -89,7 +91,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
@@ -110,6 +117,7 @@ fn find_innermost_closure<'tcx>(
             } else {
                 ty::Asyncness::No
             },
+            body.params,
         ));
         steps -= 1;
     }
@@ -152,7 +160,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.
+            && (!matches!(expr.span.ctxt().outer_expn_data().kind, ExpnKind::Macro(_, _)) || 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
@@ -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
@@ -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}"));
+}
