From dcb070a276feed296498be623773b1b7c2d7d4a7 Mon Sep 17 00:00:00 2001
From: Savin Mikhail <salazar290720035017@gmail.com>
Date: Fri, 6 Mar 2026 14:49:45 +0300
Subject: [PATCH 1/2] feat: enable variadic named arguments by default with
 opt-out

---
 README.md                                     |  7 +-
 docs/issue-15-implementation-plan.md          | 61 ++++++++++++
 src/AddNamedArgumentsRector.php               | 95 +++++++++++++++++--
 src/Config/DefaultStrategy.php                | 45 +++++++--
 .../DefaultStrategy/Fixture/variadic.php.inc  |  2 +-
 .../Fixture/variadic_mixed_signature.php.inc  | 25 +++++
 .../AddNamedArgumentsRectorTest.php           | 37 ++++++++
 .../Fixture/variadic_disabled.php.inc         | 13 +++
 .../config/rector.php                         | 13 +++
 9 files changed, 277 insertions(+), 21 deletions(-)
 create mode 100644 docs/issue-15-implementation-plan.md
 create mode 100644 tests/DefaultStrategy/Fixture/variadic_mixed_signature.php.inc
 create mode 100644 tests/VariadicDisabledStrategy/AddNamedArgumentsRectorTest.php
 create mode 100644 tests/VariadicDisabledStrategy/Fixture/variadic_disabled.php.inc
 create mode 100644 tests/VariadicDisabledStrategy/config/rector.php

diff --git a/README.md b/README.md
index 8a73716..f172f4a 100644
--- a/README.md
+++ b/README.md
@@ -67,11 +67,16 @@ use SavinMikhail\AddNamedArgumentsRector\Config\PhpyhStrategy;
 return static function (RectorConfig $rectorConfig): void {
     $rectorConfig->ruleWithConfiguration(
         AddNamedArgumentsRector::class,
-        [PhpyhStrategy::class]
+        [
+            AddNamedArgumentsRector::STRATEGY => PhpyhStrategy::class,
+            AddNamedArgumentsRector::ALLOW_NAMED_VARIADIC_ARGUMENTS => false,
+        ]
     );
 };
 ```
 
+`ALLOW_NAMED_VARIADIC_ARGUMENTS` is enabled by default. If you set it to `false`, calls that include variadic arguments are skipped.
+
 #### Implementing Your Own Strategy
 
 See `PhpyhStrategy` as example, you can create your own strategy by implementing the `ConfigStrategy` interface. For example:
diff --git a/docs/issue-15-implementation-plan.md b/docs/issue-15-implementation-plan.md
new file mode 100644
index 0000000..58052dd
--- /dev/null
+++ b/docs/issue-15-implementation-plan.md
@@ -0,0 +1,61 @@
+# Issue #15 investigation: variadic parameters
+
+## Context
+
+Issue #15 requests support for converting positional arguments that map to a variadic parameter into named arguments. The idea is to allow names like `foo1`, `foo2`, etc. for a variadic parameter named `foo`, and make this behavior configurable.
+
+## Current behavior in the codebase
+
+- `DefaultStrategy::areArgumentsSuitable()` currently rejects any call where the matched parameter is variadic (`$parameters[$index]->isVariadic()`), so variadic calls are skipped before refactoring starts.
+- The existing fixture `tests/DefaultStrategy/Fixture/variadic.php.inc` documents this: a `sprintf()` call remains unchanged.
+- `AddNamedArgumentsRector::addNamesToArgs()` assumes a 1:1 index mapping between arguments and parameters, and for arguments past the parameter list keeps them unchanged (`$parameters[$index] ?? null`). This is another reason variadic tails are not transformed.
+
+## Proposed implementation plan
+
+1. **Add explicit variadic naming mode to configuration**
+   - Extend rector configuration to accept a second option (or a small options object/array), e.g. `allowNamedVariadicArguments: bool`.
+   - Keep default `false` to preserve current behavior and avoid unexpected changes.
+
+2. **Refactor argument-to-parameter matching for variadics**
+   - Introduce a helper that resolves the *effective parameter* for each argument index:
+     - normal parameters map by index;
+     - any argument index beyond the last parameter maps to the last parameter only if it is variadic.
+   - Use this helper in both suitability checks and argument renaming to avoid divergent logic.
+
+3. **Update strategy validation logic**
+   - In `DefaultStrategy::areArgumentsSuitable()`, keep rejecting variadics when the new option is disabled.
+   - When enabled, allow variadic-mapped arguments and still keep existing safety checks:
+     - reject unpacked args (`...$x`);
+     - reject mismatched pre-existing named arguments.
+
+4. **Define deterministic naming for variadic args**
+   - For the first variadic value use `<variadicParamName>1`, then increment (`foo1`, `foo2`, ...).
+   - Never rename an argument that is already explicitly named.
+   - Apply naming only when the variadic option is enabled.
+
+5. **Preserve skip-default logic only for non-variadics**
+   - `shouldSkipArg()` should continue to operate for optional non-variadic parameters.
+   - Do not apply default-value skipping to variadic values (variadic parameters do not have a comparable default-value semantic for each collected item).
+
+6. **Add comprehensive fixtures/tests**
+   - Keep current fixture asserting default behavior (variadics unchanged).
+   - Add new fixture(s) with the option enabled covering:
+     - pure variadic function (`sprintf`-like examples as allowed by PHP);
+     - mixed signature (`function f($a, ...$rest)`) where only variadic tail gets `rest1`, `rest2`;
+     - partial pre-named args and stability of existing names;
+     - unpacking remains skipped.
+
+7. **Document behavior and limitations in README**
+   - Explain that variadic argument naming is opt-in.
+   - Mention that names for variadic arguments are synthetic keys and chosen for deterministic output.
+
+## Risks and edge cases to validate
+
+- Calls that already contain named variadic arguments with arbitrary names should remain valid and stable.
+- Interaction with mixed positional + named arguments ordering rules in PHP 8+.
+- Reflection differences between internal and userland functions when detecting variadic parameters.
+
+## Suggested rollout
+
+- Implement behind opt-in config, ship with tests first.
+- Collect feedback before considering changing defaults.
diff --git a/src/AddNamedArgumentsRector.php b/src/AddNamedArgumentsRector.php
index 447e4a8..24b822a 100644
--- a/src/AddNamedArgumentsRector.php
+++ b/src/AddNamedArgumentsRector.php
@@ -40,8 +40,14 @@
  */
 final class AddNamedArgumentsRector extends AbstractRector implements MinPhpVersionInterface, ConfigurableRectorInterface
 {
+    public const STRATEGY = 0;
+
+    public const ALLOW_NAMED_VARIADIC_ARGUMENTS = 1;
+
     private string $configStrategy = DefaultStrategy::class;
 
+    private bool $allowNamedVariadicArguments = true;
+
     private readonly Reflection $reflectionService;
 
     private readonly ConstExprEvaluator $constExprEvaluator;
@@ -95,6 +101,10 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
+        if (! $this->allowNamedVariadicArguments && $this->hasVariadicArguments($node, $parameters)) {
+            return null;
+        }
+
         $hasChanges = $this->addNamesToArgs(node: $node, parameters: $parameters);
 
         if (! $hasChanges) {
@@ -112,9 +122,10 @@ private function addNamesToArgs(
         array $parameters,
     ): bool {
         $namedArgs = [];
+        $variadicArgCounters = [];
         $hasChanges = false;
         foreach ($node->args as $index => $arg) {
-            $parameter = $parameters[$index] ?? null;
+            $parameter = $this->resolveParameterForArgumentIndex(parameters: $parameters, index: $index);
             if ($parameter === null) {
                 $namedArgs[] = $arg;
 
@@ -127,13 +138,21 @@ private function addNamesToArgs(
                 continue;
             }
 
-            if ($this->shouldSkipArg($arg, $parameter)) {
+            if (! $parameter->isVariadic() && $this->shouldSkipArg($arg, $parameter)) {
                 $hasChanges = true;
 
                 continue;
             }
 
-            $arg->name = new Identifier(name: $parameter->getName());
+            if ($parameter->isVariadic()) {
+                $variadicParameterName = $parameter->getName();
+                $variadicIndex = ($variadicArgCounters[$variadicParameterName] ?? 0) + 1;
+                $variadicArgCounters[$variadicParameterName] = $variadicIndex;
+
+                $arg->name = new Identifier(name: $variadicParameterName . $variadicIndex);
+            } else {
+                $arg->name = new Identifier(name: $parameter->getName());
+            }
             $namedArgs[] = $arg;
             $hasChanges = true;
         }
@@ -172,6 +191,49 @@ private function shouldSkipArg(Arg $arg, ExtendedParameterReflection $parameter)
         return $argValue === $defaultValue;
     }
 
+    /**
+     * @param ExtendedParameterReflection[] $parameters
+     */
+    private function hasVariadicArguments(
+        FuncCall|StaticCall|MethodCall|New_ $node,
+        array $parameters,
+    ): bool {
+        foreach ($node->args as $index => $arg) {
+            if ($arg instanceof Arg && $arg->unpack) {
+                continue;
+            }
+
+            $parameter = $this->resolveParameterForArgumentIndex(parameters: $parameters, index: $index);
+            if ($parameter !== null && $parameter->isVariadic()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * @param ExtendedParameterReflection[] $parameters
+     */
+    private function resolveParameterForArgumentIndex(array $parameters, int $index): ?ExtendedParameterReflection
+    {
+        if (isset($parameters[$index])) {
+            return $parameters[$index];
+        }
+
+        if ($parameters === []) {
+            return null;
+        }
+
+        $lastParameter = $parameters[array_key_last($parameters)];
+
+        if (! $lastParameter->isVariadic()) {
+            return null;
+        }
+
+        return $lastParameter;
+    }
+
     public function provideMinPhpVersion(): int
     {
         return PhpVersion::PHP_80;
@@ -179,20 +241,33 @@ public function provideMinPhpVersion(): int
 
     public function configure(array $configuration): void
     {
-        Assert::lessThan(value: count(value: $configuration), limit: 2, message: 'You can pass only 1 strategy');
+        Assert::lessThan(value: count(value: $configuration), limit: 3, message: 'You can pass only 1 strategy and 1 variadic option');
         if ($configuration === []) {
             return;
         }
-        $strategyClass = $configuration[0];
 
-        if (!class_exists(class: $strategyClass)) {
-            throw new InvalidArgumentException(message: "Class {$strategyClass} does not exist.");
+        $strategyClass = $configuration[self::STRATEGY] ?? null;
+        if (is_bool($strategyClass)) {
+            $this->allowNamedVariadicArguments = $strategyClass;
+
+            return;
         }
 
-        $strategy = new $strategyClass();
+        if ($strategyClass !== null) {
+            if (! is_string($strategyClass) || ! class_exists(class: $strategyClass)) {
+                throw new InvalidArgumentException(message: "Class {$strategyClass} does not exist.");
+            }
+
+            $strategy = new $strategyClass();
 
-        Assert::isInstanceOf(value: $strategy, class: ConfigStrategy::class, message: 'Your strategy must implement ConfigStrategy interface');
+            Assert::isInstanceOf(value: $strategy, class: ConfigStrategy::class, message: 'Your strategy must implement ConfigStrategy interface');
 
-        $this->configStrategy = $strategyClass;
+            $this->configStrategy = $strategyClass;
+        }
+
+        if (array_key_exists(self::ALLOW_NAMED_VARIADIC_ARGUMENTS, $configuration)) {
+            Assert::boolean(value: $configuration[self::ALLOW_NAMED_VARIADIC_ARGUMENTS], message: 'Variadic option must be boolean');
+            $this->allowNamedVariadicArguments = $configuration[self::ALLOW_NAMED_VARIADIC_ARGUMENTS];
+        }
     }
 }
diff --git a/src/Config/DefaultStrategy.php b/src/Config/DefaultStrategy.php
index b671429..4fe9bcd 100644
--- a/src/Config/DefaultStrategy.php
+++ b/src/Config/DefaultStrategy.php
@@ -12,6 +12,7 @@
 use PHPStan\BetterReflection\Reflection\Adapter\ReflectionClass;
 use PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum;
 use PHPStan\Reflection\ClassReflection;
+use PHPStan\Reflection\ExtendedParameterReflection;
 use ReflectionFunctionAbstract;
 use SavinMikhail\AddNamedArgumentsRector\Reflection\Reflection;
 
@@ -57,15 +58,15 @@ private static function classAllowsNamedArguments(ClassReflection $classReflecti
         return true;
     }
 
+    /**
+     * @param Node[] $args
+     * @param ExtendedParameterReflection[] $parameters
+     */
     private static function areArgumentsSuitable(array $args, array $parameters): bool
     {
         foreach ($args as $index => $arg) {
-            if (!isset($parameters[$index])) {
-                return false;
-            }
-
-            // Skip variadic parameters (...$param)
-            if ($parameters[$index]->isVariadic()) {
+            $parameter = self::resolveParameterForArgumentIndex(parameters: $parameters, index: $index);
+            if ($parameter === null) {
                 return false;
             }
 
@@ -74,15 +75,19 @@ private static function areArgumentsSuitable(array $args, array $parameters): bo
             }
 
             // Skip unpacking arguments (...$var)
-            if ($arg instanceof Node\Arg && $arg->unpack) {
+            if ($arg->unpack) {
                 return false;
             }
 
             // Allow already named arguments as long as they reference the parameter in the same position
             if ($arg->name !== null) {
-                $argName = $arg->name instanceof Node\Identifier ? $arg->name->toString() : null;
+                if ($parameter->isVariadic()) {
+                    continue;
+                }
 
-                if ($argName !== $parameters[$index]->getName()) {
+                $argName = $arg->name->toString();
+
+                if ($argName !== $parameter->getName()) {
                     return false;
                 }
 
@@ -93,6 +98,28 @@ private static function areArgumentsSuitable(array $args, array $parameters): bo
         return true;
     }
 
+    /**
+     * @param ExtendedParameterReflection[] $parameters
+     */
+    private static function resolveParameterForArgumentIndex(array $parameters, int $index): ?ExtendedParameterReflection
+    {
+        if (isset($parameters[$index])) {
+            return $parameters[$index];
+        }
+
+        if ($parameters === []) {
+            return null;
+        }
+
+        $lastParameter = $parameters[array_key_last($parameters)];
+
+        if (! $lastParameter->isVariadic()) {
+            return null;
+        }
+
+        return $lastParameter;
+    }
+
     private static function hasNoNamedArgumentsTag(ReflectionFunctionAbstract|ReflectionClass|ReflectionEnum $reflection): bool
     {
         $docComment = $reflection->getDocComment();
diff --git a/tests/DefaultStrategy/Fixture/variadic.php.inc b/tests/DefaultStrategy/Fixture/variadic.php.inc
index ef468d2..eaf24e1 100644
--- a/tests/DefaultStrategy/Fixture/variadic.php.inc
+++ b/tests/DefaultStrategy/Fixture/variadic.php.inc
@@ -6,6 +6,6 @@ sprintf('%s/%s', 'foo', 'bar');
 -----
 <?php
 
-sprintf('%s/%s', 'foo', 'bar');
+sprintf(format: '%s/%s', values1: 'foo', values2: 'bar');
 
 ?>
diff --git a/tests/DefaultStrategy/Fixture/variadic_mixed_signature.php.inc b/tests/DefaultStrategy/Fixture/variadic_mixed_signature.php.inc
new file mode 100644
index 0000000..30bae71
--- /dev/null
+++ b/tests/DefaultStrategy/Fixture/variadic_mixed_signature.php.inc
@@ -0,0 +1,25 @@
+<?php
+
+final readonly class VariadicRunner
+{
+    public function run(string $command, ...$args): void
+    {
+    }
+}
+
+(new VariadicRunner())->run('php', '--version', '--ansi');
+
+?>
+-----
+<?php
+
+final readonly class VariadicRunner
+{
+    public function run(string $command, ...$args): void
+    {
+    }
+}
+
+(new VariadicRunner())->run(command: 'php', args1: '--version', args2: '--ansi');
+
+?>
diff --git a/tests/VariadicDisabledStrategy/AddNamedArgumentsRectorTest.php b/tests/VariadicDisabledStrategy/AddNamedArgumentsRectorTest.php
new file mode 100644
index 0000000..3ea886a
--- /dev/null
+++ b/tests/VariadicDisabledStrategy/AddNamedArgumentsRectorTest.php
@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SavinMikhail\Tests\AddNamedArgumentsRector\VariadicDisabledStrategy;
+
+use Iterator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+use SavinMikhail\AddNamedArgumentsRector\AddNamedArgumentsRector;
+
+final class AddNamedArgumentsRectorTest extends AbstractRectorTestCase
+{
+    #[DataProvider('provideCases')]
+    public function test(string $filePath): void
+    {
+        $this->doTestFile($filePath);
+    }
+
+    /**
+     * @return Iterator<array<int, string>>
+     */
+    public static function provideCases(): iterable
+    {
+        return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
+    }
+
+    public function provideConfigFilePath(): string
+    {
+        return __DIR__ . '/config/rector.php';
+    }
+
+    protected function getRectorClass(): string
+    {
+        return AddNamedArgumentsRector::class;
+    }
+}
diff --git a/tests/VariadicDisabledStrategy/Fixture/variadic_disabled.php.inc b/tests/VariadicDisabledStrategy/Fixture/variadic_disabled.php.inc
new file mode 100644
index 0000000..dc376bb
--- /dev/null
+++ b/tests/VariadicDisabledStrategy/Fixture/variadic_disabled.php.inc
@@ -0,0 +1,13 @@
+<?php
+
+str_contains('foobar', 'foo');
+sprintf('%s/%s', 'foo', 'bar');
+
+?>
+-----
+<?php
+
+str_contains(haystack: 'foobar', needle: 'foo');
+sprintf('%s/%s', 'foo', 'bar');
+
+?>
diff --git a/tests/VariadicDisabledStrategy/config/rector.php b/tests/VariadicDisabledStrategy/config/rector.php
new file mode 100644
index 0000000..28cb7ab
--- /dev/null
+++ b/tests/VariadicDisabledStrategy/config/rector.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use SavinMikhail\AddNamedArgumentsRector\AddNamedArgumentsRector;
+use SavinMikhail\AddNamedArgumentsRector\Config\DefaultStrategy;
+
+return RectorConfig::configure()
+        ->withConfiguredRule(AddNamedArgumentsRector::class, [
+            AddNamedArgumentsRector::STRATEGY => DefaultStrategy::class,
+            AddNamedArgumentsRector::ALLOW_NAMED_VARIADIC_ARGUMENTS => false,
+        ]);

From 9d6c5cb8b17f47823a2b66173de089438bf0a9f8 Mon Sep 17 00:00:00 2001
From: Savin Mikhail <mikhail.d.savin@gmail.com>
Date: Sat, 7 Mar 2026 11:16:31 +0300
Subject: [PATCH 2/2] Delete docs/issue-15-implementation-plan.md

---
 docs/issue-15-implementation-plan.md | 61 ----------------------------
 1 file changed, 61 deletions(-)
 delete mode 100644 docs/issue-15-implementation-plan.md

diff --git a/docs/issue-15-implementation-plan.md b/docs/issue-15-implementation-plan.md
deleted file mode 100644
index 58052dd..0000000
--- a/docs/issue-15-implementation-plan.md
+++ /dev/null
@@ -1,61 +0,0 @@
-# Issue #15 investigation: variadic parameters
-
-## Context
-
-Issue #15 requests support for converting positional arguments that map to a variadic parameter into named arguments. The idea is to allow names like `foo1`, `foo2`, etc. for a variadic parameter named `foo`, and make this behavior configurable.
-
-## Current behavior in the codebase
-
-- `DefaultStrategy::areArgumentsSuitable()` currently rejects any call where the matched parameter is variadic (`$parameters[$index]->isVariadic()`), so variadic calls are skipped before refactoring starts.
-- The existing fixture `tests/DefaultStrategy/Fixture/variadic.php.inc` documents this: a `sprintf()` call remains unchanged.
-- `AddNamedArgumentsRector::addNamesToArgs()` assumes a 1:1 index mapping between arguments and parameters, and for arguments past the parameter list keeps them unchanged (`$parameters[$index] ?? null`). This is another reason variadic tails are not transformed.
-
-## Proposed implementation plan
-
-1. **Add explicit variadic naming mode to configuration**
-   - Extend rector configuration to accept a second option (or a small options object/array), e.g. `allowNamedVariadicArguments: bool`.
-   - Keep default `false` to preserve current behavior and avoid unexpected changes.
-
-2. **Refactor argument-to-parameter matching for variadics**
-   - Introduce a helper that resolves the *effective parameter* for each argument index:
-     - normal parameters map by index;
-     - any argument index beyond the last parameter maps to the last parameter only if it is variadic.
-   - Use this helper in both suitability checks and argument renaming to avoid divergent logic.
-
-3. **Update strategy validation logic**
-   - In `DefaultStrategy::areArgumentsSuitable()`, keep rejecting variadics when the new option is disabled.
-   - When enabled, allow variadic-mapped arguments and still keep existing safety checks:
-     - reject unpacked args (`...$x`);
-     - reject mismatched pre-existing named arguments.
-
-4. **Define deterministic naming for variadic args**
-   - For the first variadic value use `<variadicParamName>1`, then increment (`foo1`, `foo2`, ...).
-   - Never rename an argument that is already explicitly named.
-   - Apply naming only when the variadic option is enabled.
-
-5. **Preserve skip-default logic only for non-variadics**
-   - `shouldSkipArg()` should continue to operate for optional non-variadic parameters.
-   - Do not apply default-value skipping to variadic values (variadic parameters do not have a comparable default-value semantic for each collected item).
-
-6. **Add comprehensive fixtures/tests**
-   - Keep current fixture asserting default behavior (variadics unchanged).
-   - Add new fixture(s) with the option enabled covering:
-     - pure variadic function (`sprintf`-like examples as allowed by PHP);
-     - mixed signature (`function f($a, ...$rest)`) where only variadic tail gets `rest1`, `rest2`;
-     - partial pre-named args and stability of existing names;
-     - unpacking remains skipped.
-
-7. **Document behavior and limitations in README**
-   - Explain that variadic argument naming is opt-in.
-   - Mention that names for variadic arguments are synthetic keys and chosen for deterministic output.
-
-## Risks and edge cases to validate
-
-- Calls that already contain named variadic arguments with arbitrary names should remain valid and stable.
-- Interaction with mixed positional + named arguments ordering rules in PHP 8+.
-- Reflection differences between internal and userland functions when detecting variadic parameters.
-
-## Suggested rollout
-
-- Implement behind opt-in config, ship with tests first.
-- Collect feedback before considering changing defaults.