Skip to content

Commit 6cb5d82

Browse files
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
1 parent 34bd36d commit 6cb5d82

4 files changed

Lines changed: 108 additions & 15 deletions

File tree

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture;
4+
5+
use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg;
6+
7+
class NamedArgAddNew
8+
{
9+
public function run()
10+
{
11+
$obj = new SomeMultiArg();
12+
// Named arguments exist, but not the one we're adding (c)
13+
$obj->run(b: 5);
14+
}
15+
}
16+
17+
?>
18+
-----
19+
<?php
20+
21+
namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture;
22+
23+
use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg;
24+
25+
class NamedArgAddNew
26+
{
27+
public function run()
28+
{
29+
$obj = new SomeMultiArg();
30+
// Named arguments exist, but not the one we're adding (c)
31+
$obj->run(b: 5, c: 4);
32+
}
33+
}
34+
35+
?>
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture;
4+
5+
use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg;
6+
7+
class NamedArgMixedPositionalNamed
8+
{
9+
public function run()
10+
{
11+
$obj = new SomeMultiArg();
12+
// Mix of positional and named arguments
13+
$obj->run(0, c: 6);
14+
$obj->run(0, b: 6);
15+
}
16+
}
17+
18+
?>
19+
-----
20+
<?php
21+
22+
namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture;
23+
24+
use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg;
25+
26+
class NamedArgMixedPositionalNamed
27+
{
28+
public function run()
29+
{
30+
$obj = new SomeMultiArg();
31+
// Mix of positional and named arguments
32+
$obj->run(0, c: 6);
33+
$obj->run(0, b: 6, c: 4);
34+
}
35+
}
36+
37+
?>

rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arguments.php.inc renamed to rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arg_already_present.php.inc

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,13 @@ namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture;
44

55
use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg;
66

7-
class SkipNamedArguments
7+
class SkipNamedArgAlreadyPresent
88
{
99
public function run()
1010
{
1111
$obj = new SomeMultiArg();
12-
$obj->run(b: 5);
13-
$obj->run(c: 4);
14-
$obj->run(0, 1, c: 6);
12+
// Parameter 'c' is already provided as named argument
13+
$obj->run(b: 5, c: 999);
14+
$obj->run(c: 999, b: 5);
1515
}
1616
}
17-

rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use PhpParser\Node\Expr\MethodCall;
1212
use PhpParser\Node\Expr\StaticCall;
1313
use PhpParser\Node\Expr\Variable;
14+
use PhpParser\Node\Identifier;
1415
use PhpParser\Node\Name;
1516
use PhpParser\Node\Param;
1617
use PhpParser\Node\Stmt\Class_;
@@ -183,13 +184,18 @@ private function processMethodCall(
183184

184185
$defaultValue = $argumentAdder->getArgumentDefaultValue();
185186
$arg = new Arg(BuilderHelpers::normalizeValue($defaultValue));
186-
if (isset($methodCall->args[$position])) {
187-
return;
188-
}
189187

190-
$this->fillGapBetweenWithDefaultValue($methodCall, $position);
188+
if ($this->argsAnalyzer->hasNamedArg($methodCall->getArgs())) {
189+
$argumentName = $argumentAdder->getArgumentName();
190+
if ($argumentName === null) {
191+
return;
192+
}
193+
$arg->name = new Identifier($argumentName);
194+
} else {
195+
$this->fillGapBetweenWithDefaultValue($methodCall, $position);
196+
}
191197

192-
$methodCall->args[$position] = $arg;
198+
$methodCall->args[] = $arg;
193199
$this->hasChanged = true;
194200
}
195201

@@ -251,12 +257,18 @@ private function shouldSkipParameter(
251257
return $this->changedArgumentsDetector->isTypeChanged($param, $argumentAdder->getArgumentType());
252258
}
253259

260+
// If named arguments exist
254261
if ($this->argsAnalyzer->hasNamedArg($node->getArgs())) {
255-
return true;
256-
}
257-
258-
if (isset($node->args[$position])) {
259-
return true;
262+
// Check if the parameter we're trying to add is already present as a named argument
263+
foreach ($node->getArgs() as $arg) {
264+
if ($arg->name instanceof Identifier && $this->isName($arg->name, $argumentName)) {
265+
return true; // Skip - parameter already provided with name
266+
}
267+
}
268+
} else {
269+
if (isset($node->args[$position])) {
270+
return true;
271+
}
260272
}
261273

262274
// Check if default value is the same
@@ -339,6 +351,16 @@ private function processStaticCall(
339351
return;
340352
}
341353

354+
// Handle named arguments case - add as named argument at the end
355+
if ($this->argsAnalyzer->hasNamedArg($staticCall->getArgs())) {
356+
$arg = new Arg(new Variable($argumentName));
357+
$arg->name = new Identifier($argumentName);
358+
$staticCall->args[] = $arg;
359+
$this->hasChanged = true;
360+
return;
361+
}
362+
363+
// Original positional logic
342364
$this->fillGapBetweenWithDefaultValue($staticCall, $position);
343365

344366
$staticCall->args[$position] = new Arg(new Variable($argumentName));

0 commit comments

Comments
 (0)