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/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'); ----- 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 @@ +run('php', '--version', '--ansi'); + +?> +----- +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 @@ +doTestFile($filePath); + } + + /** + * @return Iterator> + */ + 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 @@ + +----- + 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 @@ +withConfiguredRule(AddNamedArgumentsRector::class, [ + AddNamedArgumentsRector::STRATEGY => DefaultStrategy::class, + AddNamedArgumentsRector::ALLOW_NAMED_VARIADIC_ARGUMENTS => false, + ]);