[CodingStyle] Skip callee arity mismatch on ArrowFunctionDelegatingCallToFirstClassCallableRector

[jblairy]

ArrowFunctionDelegatingCallToFirstClassCallableRector silently changes behavior when the callee has more parameters than the arrow function

Summary

ArrowFunctionDelegatingCallToFirstClassCallableRector (via
ArrowFunctionAndClosureFirstClassCallableGuard::shouldSkip()) converts a
zero-argument arrow function delegating to a method into a first-class
callable reference, even when the target method declares an optional
parameter that the arrow function never had. When the resulting callable
is stored (e.g. as a \Closure default value) and later invoked elsewhere
with an argument, that argument — which the original arrow function would
have silently ignored (its arity was 0) — now flows into the method's
optional parameter. This is a silent behavior change, not just a style
rewrite.

• Rector version: 2.6.3
• Rule: Rector\CodingStyle\Rector\ArrowFunction\ArrowFunctionDelegatingCallToFirstClassCallableRector
• Guard: Rector\CodingStyle\Guard\ArrowFunctionAndClosureFirstClassCallableGuard::shouldSkip()

Reproduction

<?php

declare(strict_types=1);

final class Catalogue
{
}

final class CatalogueFactory
{
    public function build(array $items = []): Catalogue
    {
        // ... uses $items when present
        return new Catalogue();
    }
}

final class Consumer
{
    public function __construct(private CatalogueFactory $factory)
    {
    }

    public function make(?\Closure $builder = null): Catalogue
    {
        $builder ??= fn (): Catalogue => $this->factory->build();

        // invoked elsewhere with an argument the fallback closure
        // intentionally ignores (it declares zero parameters)
        return $builder(['not', 'catalogue', 'items']);
    }
}

Rector rewrites the fallback to:

$builder ??= $this->factory->build(...);

Before the rewrite, $builder(['not', 'catalogue', 'items']) calls the
0-arity arrow function, which ignores the argument and returns
$this->factory->build() (empty catalogue). After the rewrite, the same
call forwards ['not', 'catalogue', 'items'] into build()'s $items
parameter — a different, incorrect result. Nothing else in the surrounding
code changed; only the closure default was "simplified".

We hit this for real in application code: a test double defaulted to
fn (): Catalogue => $this->catalogue() as a fallback for a
\Closure collaborator; the collaborator is invoked elsewhere with one
argument. Rector's rewrite made that argument leak into catalogue()'s
optional $items parameter, corrupting the built collection and breaking
several tests with Call to a member function getItem() on int.

Root cause

In ArrowFunctionAndClosureFirstClassCallableGuard::shouldSkip():

$params = $arrowFunctionOrClosure->getParams();
if (count($params) !== count($callLike->getArgs())) {
    return true; // skip
}
$args = $callLike->getArgs();
...
$parameters = ParametersAcceptorSelectorVariantsWrapper::select($reflection, $callLike, $scope)->getParameters();
...
return count($parameters) > 1;

The guard only compares:

1. the arrow function/closure's own parameter count against the literal
   call's argument count (both 0 in the repro — matches, so it doesn't
   skip), and
2. the resolved callee's total declared parameter count against a fixed
   threshold of 1 (build(array $items = []) has exactly 1 parameter, so
   1 > 1 is false — again doesn't skip).

It never checks that the arrow function's own arity matches the callee's
arity. When the callee has one parameter the literal call never used
(picked up via a default value), converting to first-class-callable syntax
silently exposes that parameter to any external caller of the produced
callable — which is exactly what breaks here, since the callable is stored
and invoked elsewhere with a different argument count than the original
arrow function declared.

Suggested fix

Since count($args) === count($params) is already guaranteed earlier in
the method, the final check should compare the callee's total parameter
count against that same number, instead of an unrelated fixed threshold.

[TomasVotruba]

LGTM 👍