diff --git a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/by_pass_2nd_arg_value.php.inc b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/by_pass_2nd_arg_value.php.inc index 1bd75535c2d..f2d5254c2d3 100644 --- a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/by_pass_2nd_arg_value.php.inc +++ b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/by_pass_2nd_arg_value.php.inc @@ -9,7 +9,7 @@ class ByPass2ndArgValue public function run() { $containerBuilder = new SomeMultiArg(); - $containerBuilder->run(1); + $containerBuilder->thirdArgument(1); } } @@ -26,7 +26,7 @@ class ByPass2ndArgValue public function run() { $containerBuilder = new SomeMultiArg(); - $containerBuilder->run(1, 2, 4); + $containerBuilder->thirdArgument(1, 2, 6); } } diff --git a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/named_arguments.php.inc b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/named_arguments.php.inc new file mode 100644 index 00000000000..0713619f103 --- /dev/null +++ b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/named_arguments.php.inc @@ -0,0 +1,59 @@ +firstArgument(b: 1); + $containerBuilder->firstArgument(c: 1); + $containerBuilder->firstArgument(b: 1, c: 2); + $containerBuilder->firstArgument(c: 1, b: 2); + $containerBuilder->secondArgument(a: 1); + $containerBuilder->secondArgument(c: 1); + $containerBuilder->secondArgument(a: 1, c: 2); + $containerBuilder->secondArgument(c: 1, a: 2); + $containerBuilder->secondArgument(1, c: 2); + $containerBuilder->thirdArgument(a: 1); + $containerBuilder->thirdArgument(b: 1); + $containerBuilder->thirdArgument(a: 1, b: 2); + $containerBuilder->thirdArgument(b: 1, a: 2); + $containerBuilder->thirdArgument(1, b: 2); + } +} + +?> +----- +firstArgument(b: 1, a: 4); + $containerBuilder->firstArgument(c: 1, a: 4); + $containerBuilder->firstArgument(b: 1, c: 2, a: 4); + $containerBuilder->firstArgument(c: 1, b: 2, a: 4); + $containerBuilder->secondArgument(a: 1, b: 5); + $containerBuilder->secondArgument(c: 1, b: 5); + $containerBuilder->secondArgument(a: 1, c: 2, b: 5); + $containerBuilder->secondArgument(c: 1, a: 2, b: 5); + $containerBuilder->secondArgument(1, c: 2, b: 5); + $containerBuilder->thirdArgument(a: 1, c: 6); + $containerBuilder->thirdArgument(b: 1, c: 6); + $containerBuilder->thirdArgument(a: 1, b: 2, c: 6); + $containerBuilder->thirdArgument(b: 1, a: 2, c: 6); + $containerBuilder->thirdArgument(1, b: 2, c: 6); + } +} + +?> diff --git a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arguments.inc b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arguments.inc new file mode 100644 index 00000000000..e4bbac86b95 --- /dev/null +++ b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arguments.inc @@ -0,0 +1,21 @@ +firstArgument(a: 7); + $containerBuilder->firstArgument(1, b: 1); + $containerBuilder->secondArgument(a: 7, b: 8); + $containerBuilder->secondArgument(b: 7, a: 8); + $containerBuilder->secondArgument(1, 2, c: 8); + $containerBuilder->thirdArgument(c: 8); + $containerBuilder->thirdArgument(b: 7, c: 8, a: 9); + $containerBuilder->thirdArgument(1, b: 8, c: 9); + } +} diff --git a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_static_named_arguments.php.inc b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_static_named_arguments.php.inc new file mode 100644 index 00000000000..f813f873d2a --- /dev/null +++ b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_static_named_arguments.php.inc @@ -0,0 +1,28 @@ + +----- + diff --git a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Source/SomeMultiArg.php b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Source/SomeMultiArg.php index 0ee943d217d..4f677f921d3 100644 --- a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Source/SomeMultiArg.php +++ b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Source/SomeMultiArg.php @@ -6,7 +6,15 @@ class SomeMultiArg { - public function run($a = 1, $b = 2, $c = 3) + public function firstArgument($a = 1, $b = 2, $c = 3) + { + } + + public function secondArgument($a = 1, $b = 2, $c = 3) + { + } + + public function thirdArgument($a = 1, $b = 2, $c = 3) { } } diff --git a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/config/configured_rule.php b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/config/configured_rule.php index ebcedc0090b..2904f2b05b3 100644 --- a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/config/configured_rule.php +++ b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/config/configured_rule.php @@ -53,7 +53,9 @@ ArgumentAddingScope::SCOPE_CLASS_METHOD ), new ArgumentAdder(SomeClass::class, 'withoutTypeOrDefaultValue', 0, 'arguments', [], $arrayType), - new ArgumentAdder(SomeMultiArg::class, 'run', 2, 'c', 4), + new ArgumentAdder(SomeMultiArg::class, 'firstArgument', 0, 'a', 4), + new ArgumentAdder(SomeMultiArg::class, 'secondArgument', 1, 'b', 5), + new ArgumentAdder(SomeMultiArg::class, 'thirdArgument', 2, 'c', 6), new ArgumentAdder(SomeClass::class, 'someMethod', 0, 'default', 1), new ArgumentAdderWithoutDefaultValue(WithoutDefaultValue::class, 'someMethod', 0, 'foo', $arrayType), ]); diff --git a/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php b/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php index d39b107f705..1c18f35cf74 100644 --- a/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php +++ b/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php @@ -11,6 +11,7 @@ use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Expr\Variable; +use PhpParser\Node\Identifier; use PhpParser\Node\Name; use PhpParser\Node\Param; use PhpParser\Node\Stmt\Class_; @@ -24,6 +25,7 @@ use Rector\Contract\Rector\ConfigurableRectorInterface; use Rector\Enum\ObjectReference; use Rector\Exception\ShouldNotHappenException; +use Rector\NodeAnalyzer\ArgsAnalyzer; use Rector\PhpParser\AstResolver; use Rector\PHPStanStaticTypeMapper\Enum\TypeKind; use Rector\Rector\AbstractRector; @@ -48,7 +50,8 @@ public function __construct( private readonly ArgumentAddingScope $argumentAddingScope, private readonly ChangedArgumentsDetector $changedArgumentsDetector, private readonly AstResolver $astResolver, - private readonly StaticTypeMapper $staticTypeMapper + private readonly StaticTypeMapper $staticTypeMapper, + private readonly ArgsAnalyzer $argsAnalyzer ) { } @@ -181,13 +184,21 @@ private function processMethodCall( $defaultValue = $argumentAdder->getArgumentDefaultValue(); $arg = new Arg(BuilderHelpers::normalizeValue($defaultValue)); - if (isset($methodCall->args[$position])) { - return; + + // if there are named argyments, we just add it at the end as a new named argument + if ($this->argsAnalyzer->hasNamedArg($methodCall->getArgs())) { + $argumentName = $argumentAdder->getArgumentName(); + if ($argumentName === null) { + throw new ShouldNotHappenException(); + } + + $arg->name = new Identifier($argumentName); + } else { + $this->fillGapBetweenWithDefaultValue($methodCall, $position); } - $this->fillGapBetweenWithDefaultValue($methodCall, $position); + $methodCall->args[] = $arg; - $methodCall->args[$position] = $arg; $this->hasChanged = true; } @@ -249,7 +260,20 @@ private function shouldSkipParameter( return $this->changedArgumentsDetector->isTypeChanged($param, $argumentAdder->getArgumentType()); } - if (isset($node->args[$position])) { + $arguments = $node->getArgs(); + $firstNamedArgumentPosition = $this->argsAnalyzer->resolveFirstNamedArgPosition($arguments); + // If named arguments exist + if ($firstNamedArgumentPosition !== null) { + // Check if the parameter we're trying to add is before the first named argument + if ($position < $firstNamedArgumentPosition) { + return true; //if that is the case, the parameter already exists, skip + } + + // Check if the parameter we're trying to add is already present as a named argument + if ($this->argsAnalyzer->resolveArgPosition($arguments, $argumentName, -1) !== -1) { + return true; // if it exists as a named argument, skip + } + } elseif (isset($node->args[$position])) { return true; } @@ -333,9 +357,15 @@ private function processStaticCall( return; } - $this->fillGapBetweenWithDefaultValue($staticCall, $position); + // if there are named arguments, we just add it at the end as a new named argument + $arg = new Arg(new Variable($argumentName)); + if ($this->argsAnalyzer->hasNamedArg($staticCall->getArgs())) { + $arg->name = new Identifier($argumentName); + } else { + $this->fillGapBetweenWithDefaultValue($staticCall, $position); + } - $staticCall->args[$position] = new Arg(new Variable($argumentName)); + $staticCall->args[] = $arg; $this->hasChanged = true; } diff --git a/src/NodeAnalyzer/ArgsAnalyzer.php b/src/NodeAnalyzer/ArgsAnalyzer.php index 504900b95ba..d5ff196b867 100644 --- a/src/NodeAnalyzer/ArgsAnalyzer.php +++ b/src/NodeAnalyzer/ArgsAnalyzer.php @@ -48,4 +48,21 @@ public function resolveArgPosition(array $args, string $name, int $defaultPositi return $defaultPosition; } + + /** + * @param Arg[] $args + */ + public function resolveFirstNamedArgPosition(array $args): ?int + { + $position = 0; + foreach ($args as $arg) { + if ($arg->name instanceof Identifier) { + return $position; + } + + ++$position; + } + + return null; + } }