From 34bd36d3a7731643e17d5fa3ead74572ad17c964 Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Mon, 15 Dec 2025 13:41:14 +0100 Subject: [PATCH 1/3] Skip named arguments when adding, modifying or removing parameters --- .../Fixture/skip_named_arguments.php.inc | 17 +++++++++++++++++ .../Fixture/skip_named_arguments.php.inc | 14 ++++++++++++++ .../Fixture/skip_named_arguments.php.inc | 12 ++++++++++++ .../Fixture/skip_named_arguments.php.inc | 14 ++++++++++++++ .../Arguments/ArgumentDefaultValueReplacer.php | 8 +++++++- .../Rector/ClassMethod/ArgumentAdderRector.php | 8 +++++++- .../MethodCall/RemoveMethodCallParamRector.php | 10 ++++++++++ 7 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arguments.php.inc create mode 100644 rules-tests/Arguments/Rector/ClassMethod/ReplaceArgumentDefaultValueRector/Fixture/skip_named_arguments.php.inc create mode 100644 rules-tests/Arguments/Rector/FuncCall/FunctionArgumentDefaultValueReplacerRector/Fixture/skip_named_arguments.php.inc create mode 100644 rules-tests/Arguments/Rector/MethodCall/RemoveMethodCallParamRector/Fixture/skip_named_arguments.php.inc diff --git a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arguments.php.inc b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arguments.php.inc new file mode 100644 index 00000000000..2dcdb8e3315 --- /dev/null +++ b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arguments.php.inc @@ -0,0 +1,17 @@ +run(b: 5); + $obj->run(c: 4); + $obj->run(0, 1, c: 6); + } +} + diff --git a/rules-tests/Arguments/Rector/ClassMethod/ReplaceArgumentDefaultValueRector/Fixture/skip_named_arguments.php.inc b/rules-tests/Arguments/Rector/ClassMethod/ReplaceArgumentDefaultValueRector/Fixture/skip_named_arguments.php.inc new file mode 100644 index 00000000000..89276731361 --- /dev/null +++ b/rules-tests/Arguments/Rector/ClassMethod/ReplaceArgumentDefaultValueRector/Fixture/skip_named_arguments.php.inc @@ -0,0 +1,14 @@ +setSomeMethod(someValue: 'some value'); + } +} diff --git a/rules-tests/Arguments/Rector/FuncCall/FunctionArgumentDefaultValueReplacerRector/Fixture/skip_named_arguments.php.inc b/rules-tests/Arguments/Rector/FuncCall/FunctionArgumentDefaultValueReplacerRector/Fixture/skip_named_arguments.php.inc new file mode 100644 index 00000000000..f2a5b1563ac --- /dev/null +++ b/rules-tests/Arguments/Rector/FuncCall/FunctionArgumentDefaultValueReplacerRector/Fixture/skip_named_arguments.php.inc @@ -0,0 +1,12 @@ +process(first: 1, second: 2); + $caller->process(1, second: 2); + } +} diff --git a/rules/Arguments/ArgumentDefaultValueReplacer.php b/rules/Arguments/ArgumentDefaultValueReplacer.php index 36794967ded..7ca67bca3f1 100644 --- a/rules/Arguments/ArgumentDefaultValueReplacer.php +++ b/rules/Arguments/ArgumentDefaultValueReplacer.php @@ -15,6 +15,7 @@ use PhpParser\Node\Stmt\ClassMethod; use Rector\Arguments\Contract\ReplaceArgumentDefaultValueInterface; use Rector\Arguments\ValueObject\ReplaceArgumentDefaultValue; +use Rector\NodeAnalyzer\ArgsAnalyzer; use Rector\PhpParser\Node\NodeFactory; use Rector\PhpParser\Node\Value\ValueResolver; @@ -22,7 +23,8 @@ { public function __construct( private NodeFactory $nodeFactory, - private ValueResolver $valueResolver + private ValueResolver $valueResolver, + private ArgsAnalyzer $argsAnalyzer ) { } @@ -101,6 +103,10 @@ private function processArgs( return null; } + if ($this->argsAnalyzer->hasNamedArg($expr->getArgs())) { + return null; + } + $position = $replaceArgumentDefaultValue->getPosition(); $particularArg = $expr->getArgs()[$position] ?? null; if (! $particularArg instanceof Arg) { diff --git a/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php b/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php index d39b107f705..b97eb1ac1cc 100644 --- a/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php +++ b/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php @@ -24,6 +24,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 +49,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 ) { } @@ -249,6 +251,10 @@ private function shouldSkipParameter( return $this->changedArgumentsDetector->isTypeChanged($param, $argumentAdder->getArgumentType()); } + if ($this->argsAnalyzer->hasNamedArg($node->getArgs())) { + return true; + } + if (isset($node->args[$position])) { return true; } diff --git a/rules/Arguments/Rector/MethodCall/RemoveMethodCallParamRector.php b/rules/Arguments/Rector/MethodCall/RemoveMethodCallParamRector.php index 7427216eb84..1e174e56735 100644 --- a/rules/Arguments/Rector/MethodCall/RemoveMethodCallParamRector.php +++ b/rules/Arguments/Rector/MethodCall/RemoveMethodCallParamRector.php @@ -9,6 +9,7 @@ use PhpParser\Node\Expr\StaticCall; use Rector\Arguments\ValueObject\RemoveMethodCallParam; use Rector\Contract\Rector\ConfigurableRectorInterface; +use Rector\NodeAnalyzer\ArgsAnalyzer; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -25,6 +26,11 @@ final class RemoveMethodCallParamRector extends AbstractRector implements Config */ private array $removeMethodCallParams = []; + public function __construct( + private readonly ArgsAnalyzer $argsAnalyzer, + ) { + } + public function getRuleDefinition(): RuleDefinition { return new RuleDefinition('Remove parameter of method call', [ @@ -73,6 +79,10 @@ public function refactor(Node $node): ?Node return null; } + if ($this->argsAnalyzer->hasNamedArg($node->getArgs())) { + return null; + } + foreach ($this->removeMethodCallParams as $removeMethodCallParam) { if (! $this->isName($node->name, $removeMethodCallParam->getMethodName())) { continue; From 6cb5d825e0a749cc6069b15de7579835c3f00b81 Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Tue, 16 Dec 2025 11:12:43 +0100 Subject: [PATCH 2/3] refactor: improve ArgumentAdderRector to intelligently handle named arguments - Check if parameter is already present as named argument before adding - When named arguments exist, add new parameter as named argument at end - Add comprehensive test fixtures for named argument scenarios --- .../Fixture/named_arg_add_new.php.inc | 35 ++++++++++++++++ .../named_arg_mixed_positional_named.php.inc | 37 ++++++++++++++++ ...=> skip_named_arg_already_present.php.inc} | 9 ++-- .../ClassMethod/ArgumentAdderRector.php | 42 ++++++++++++++----- 4 files changed, 108 insertions(+), 15 deletions(-) create mode 100644 rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/named_arg_add_new.php.inc create mode 100644 rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/named_arg_mixed_positional_named.php.inc rename rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/{skip_named_arguments.php.inc => skip_named_arg_already_present.php.inc} (61%) diff --git a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/named_arg_add_new.php.inc b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/named_arg_add_new.php.inc new file mode 100644 index 00000000000..17b82f698d5 --- /dev/null +++ b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/named_arg_add_new.php.inc @@ -0,0 +1,35 @@ +run(b: 5); + } +} + +?> +----- +run(b: 5, c: 4); + } +} + +?> diff --git a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/named_arg_mixed_positional_named.php.inc b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/named_arg_mixed_positional_named.php.inc new file mode 100644 index 00000000000..7d1f11d8254 --- /dev/null +++ b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/named_arg_mixed_positional_named.php.inc @@ -0,0 +1,37 @@ +run(0, c: 6); + $obj->run(0, b: 6); + } +} + +?> +----- +run(0, c: 6); + $obj->run(0, b: 6, c: 4); + } +} + +?> diff --git a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arguments.php.inc b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arg_already_present.php.inc similarity index 61% rename from rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arguments.php.inc rename to rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arg_already_present.php.inc index 2dcdb8e3315..c59e14e3d4b 100644 --- a/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arguments.php.inc +++ b/rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arg_already_present.php.inc @@ -4,14 +4,13 @@ namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture; use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg; -class SkipNamedArguments +class SkipNamedArgAlreadyPresent { public function run() { $obj = new SomeMultiArg(); - $obj->run(b: 5); - $obj->run(c: 4); - $obj->run(0, 1, c: 6); + // Parameter 'c' is already provided as named argument + $obj->run(b: 5, c: 999); + $obj->run(c: 999, b: 5); } } - diff --git a/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php b/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php index b97eb1ac1cc..6b8b17e60ba 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_; @@ -183,13 +184,18 @@ private function processMethodCall( $defaultValue = $argumentAdder->getArgumentDefaultValue(); $arg = new Arg(BuilderHelpers::normalizeValue($defaultValue)); - if (isset($methodCall->args[$position])) { - return; - } - $this->fillGapBetweenWithDefaultValue($methodCall, $position); + if ($this->argsAnalyzer->hasNamedArg($methodCall->getArgs())) { + $argumentName = $argumentAdder->getArgumentName(); + if ($argumentName === null) { + return; + } + $arg->name = new Identifier($argumentName); + } else { + $this->fillGapBetweenWithDefaultValue($methodCall, $position); + } - $methodCall->args[$position] = $arg; + $methodCall->args[] = $arg; $this->hasChanged = true; } @@ -251,12 +257,18 @@ private function shouldSkipParameter( return $this->changedArgumentsDetector->isTypeChanged($param, $argumentAdder->getArgumentType()); } + // If named arguments exist if ($this->argsAnalyzer->hasNamedArg($node->getArgs())) { - return true; - } - - if (isset($node->args[$position])) { - return true; + // Check if the parameter we're trying to add is already present as a named argument + foreach ($node->getArgs() as $arg) { + if ($arg->name instanceof Identifier && $this->isName($arg->name, $argumentName)) { + return true; // Skip - parameter already provided with name + } + } + } else { + if (isset($node->args[$position])) { + return true; + } } // Check if default value is the same @@ -339,6 +351,16 @@ private function processStaticCall( return; } + // Handle named arguments case - add as named argument at the end + if ($this->argsAnalyzer->hasNamedArg($staticCall->getArgs())) { + $arg = new Arg(new Variable($argumentName)); + $arg->name = new Identifier($argumentName); + $staticCall->args[] = $arg; + $this->hasChanged = true; + return; + } + + // Original positional logic $this->fillGapBetweenWithDefaultValue($staticCall, $position); $staticCall->args[$position] = new Arg(new Variable($argumentName)); From 07b1784dbbb375c6112b7e250656ee233689ed7e Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Tue, 16 Dec 2025 11:25:23 +0100 Subject: [PATCH 3/3] refactor: refine ArgumentAdderRector named argument handling --- .../Rector/ClassMethod/ArgumentAdderRector.php | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php b/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php index 6b8b17e60ba..3981d5110b1 100644 --- a/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php +++ b/rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php @@ -352,18 +352,14 @@ private function processStaticCall( } // Handle named arguments case - add as named argument at the end + $arg = new Arg(new Variable($argumentName)); if ($this->argsAnalyzer->hasNamedArg($staticCall->getArgs())) { - $arg = new Arg(new Variable($argumentName)); $arg->name = new Identifier($argumentName); - $staticCall->args[] = $arg; - $this->hasChanged = true; - return; + } else { + $this->fillGapBetweenWithDefaultValue($staticCall, $position); } - // Original positional logic - $this->fillGapBetweenWithDefaultValue($staticCall, $position); - - $staticCall->args[$position] = new Arg(new Variable($argumentName)); + $staticCall->args[] = $arg; $this->hasChanged = true; }