Skip to content

Commit 650c104

Browse files
carlos-granadoscarlos-granados
committed
Manage named arguments in ArgumentAdderRector rule
1 parent 40d87af commit 650c104

File tree

9 files changed

+250
-12
lines changed

9 files changed

+250
-12
lines changed

‎rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/by_pass_2nd_arg_value.php.inc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class ByPass2ndArgValue
99
public function run()
1010
{
1111
$containerBuilder = new SomeMultiArg();
12-
$containerBuilder->run(1);
12+
$containerBuilder->thirdArgument(1);
1313
}
1414
}
1515

@@ -26,7 +26,7 @@ class ByPass2ndArgValue
2626
public function run()
2727
{
2828
$containerBuilder = new SomeMultiArg();
29-
$containerBuilder->run(1, 2, 4);
29+
$containerBuilder->thirdArgument(1, 2, 6);
3030
}
3131
}
3232

‎rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/named_arguments.php.inc‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
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 NamedArguments
8+
{
9+
public function run()
10+
{
11+
$containerBuilder = new SomeMultiArg();
12+
$containerBuilder->firstArgument(b: 1);
13+
$containerBuilder->firstArgument(c: 1);
14+
$containerBuilder->firstArgument(b: 1, c: 2);
15+
$containerBuilder->firstArgument(c: 1, b: 2);
16+
$containerBuilder->secondArgument(a: 1);
17+
$containerBuilder->secondArgument(c: 1);
18+
$containerBuilder->secondArgument(a: 1, c: 2);
19+
$containerBuilder->secondArgument(c: 1, a: 2);
20+
$containerBuilder->secondArgument(1, c: 2);
21+
$containerBuilder->thirdArgument(a: 1);
22+
$containerBuilder->thirdArgument(b: 1);
23+
$containerBuilder->thirdArgument(a: 1, b: 2);
24+
$containerBuilder->thirdArgument(b: 1, a: 2);
25+
$containerBuilder->thirdArgument(1, b: 2);
26+
}
27+
}
28+
29+
?>
30+
-----
31+
<?php
32+
33+
namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture;
34+
35+
use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg;
36+
37+
class NamedArguments
38+
{
39+
public function run()
40+
{
41+
$containerBuilder = new SomeMultiArg();
42+
$containerBuilder->firstArgument(b: 1, a: 4);
43+
$containerBuilder->firstArgument(c: 1, a: 4);
44+
$containerBuilder->firstArgument(b: 1, c: 2, a: 4);
45+
$containerBuilder->firstArgument(c: 1, b: 2, a: 4);
46+
$containerBuilder->secondArgument(a: 1, b: 5);
47+
$containerBuilder->secondArgument(c: 1, b: 5);
48+
$containerBuilder->secondArgument(a: 1, c: 2, b: 5);
49+
$containerBuilder->secondArgument(c: 1, a: 2, b: 5);
50+
$containerBuilder->secondArgument(1, c: 2, b: 5);
51+
$containerBuilder->thirdArgument(a: 1, c: 6);
52+
$containerBuilder->thirdArgument(b: 1, c: 6);
53+
$containerBuilder->thirdArgument(a: 1, b: 2, c: 6);
54+
$containerBuilder->thirdArgument(b: 1, a: 2, c: 6);
55+
$containerBuilder->thirdArgument(1, b: 2, c: 6);
56+
}
57+
}
58+
59+
?>

‎rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_named_arguments.inc‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
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 SkipNamedArguments
8+
{
9+
public function run()
10+
{
11+
$containerBuilder = new SomeMultiArg();
12+
$containerBuilder->firstArgument(a: 7);
13+
$containerBuilder->firstArgument(1, b: 1);
14+
$containerBuilder->secondArgument(a: 7, b: 8);
15+
$containerBuilder->secondArgument(b: 7, a: 8);
16+
$containerBuilder->secondArgument(1, 2, c: 8);
17+
$containerBuilder->thirdArgument(c: 8);
18+
$containerBuilder->thirdArgument(b: 7, c: 8, a: 9);
19+
$containerBuilder->thirdArgument(1, b: 8, c: 9);
20+
}
21+
}

‎rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/skip_static_named_arguments.php.inc‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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 SkipStaticNamedArguments extends SomeMultiArg
8+
{
9+
public function firstArgument($a = 1, $b = 2, $c = 3)
10+
{
11+
parent::firstArgument(a: 7);
12+
parent::firstArgument(1, b: 1);
13+
}
14+
15+
public function secondArgument($a = 1, $b = 2, $c = 3)
16+
{
17+
parent::secondArgument(a: 7, b: 8);
18+
parent::secondArgument(b: 7, a: 8);
19+
parent::secondArgument(1, 2, c: 8);
20+
}
21+
22+
public function thirdArgument($a = 1, $b = 2, $c = 3)
23+
{
24+
parent::thirdArgument(c: 8);
25+
parent::thirdArgument(b: 7, c: 8, a: 9);
26+
parent::thirdArgument(1, b: 8, c: 9);
27+
}
28+
}

‎rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Fixture/static_named_arguments.php.inc‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
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 StaticNamedArguments extends SomeMultiArg
8+
{
9+
public function firstArgument($a = 1, $b = 2, $c = 3)
10+
{
11+
parent::firstArgument(b: 1);
12+
parent::firstArgument(c: 1);
13+
parent::firstArgument(b: 1, c: 2);
14+
parent::firstArgument(c: 1, b: 2);
15+
}
16+
17+
public function secondArgument($a = 1, $b = 2, $c = 3)
18+
{
19+
parent::secondArgument(a: 1);
20+
parent::secondArgument(c: 1);
21+
parent::secondArgument(a: 1, c: 2);
22+
parent::secondArgument(c: 1, a: 2);
23+
parent::secondArgument(1, c: 2);
24+
}
25+
26+
public function thirdArgument($a = 1, $b = 2, $c = 3)
27+
{
28+
parent::thirdArgument(a: 1);
29+
parent::thirdArgument(b: 1);
30+
parent::thirdArgument(a: 1, b: 2);
31+
parent::thirdArgument(b: 1, a: 2);
32+
parent::thirdArgument(1, b: 2);
33+
}
34+
}
35+
36+
?>
37+
-----
38+
<?php
39+
40+
namespace Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Fixture;
41+
42+
use Rector\Tests\Arguments\Rector\ClassMethod\ArgumentAdderRector\Source\SomeMultiArg;
43+
44+
class StaticNamedArguments extends SomeMultiArg
45+
{
46+
public function firstArgument($a = 1, $b = 2, $c = 3)
47+
{
48+
parent::firstArgument(b: 1, a: $a);
49+
parent::firstArgument(c: 1, a: $a);
50+
parent::firstArgument(b: 1, c: 2, a: $a);
51+
parent::firstArgument(c: 1, b: 2, a: $a);
52+
}
53+
54+
public function secondArgument($a = 1, $b = 2, $c = 3)
55+
{
56+
parent::secondArgument(a: 1, b: $b);
57+
parent::secondArgument(c: 1, b: $b);
58+
parent::secondArgument(a: 1, c: 2, b: $b);
59+
parent::secondArgument(c: 1, a: 2, b: $b);
60+
parent::secondArgument(1, c: 2, b: $b);
61+
}
62+
63+
public function thirdArgument($a = 1, $b = 2, $c = 3)
64+
{
65+
parent::thirdArgument(a: 1, c: $c);
66+
parent::thirdArgument(b: 1, c: $c);
67+
parent::thirdArgument(a: 1, b: 2, c: $c);
68+
parent::thirdArgument(b: 1, a: 2, c: $c);
69+
parent::thirdArgument(1, b: 2, c: $c);
70+
}
71+
}
72+
73+
?>

‎rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/Source/SomeMultiArg.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,15 @@
66

77
class SomeMultiArg
88
{
9-
public function run($a = 1, $b = 2, $c = 3)
9+
public function firstArgument($a = 1, $b = 2, $c = 3)
10+
{
11+
}
12+
13+
public function secondArgument($a = 1, $b = 2, $c = 3)
14+
{
15+
}
16+
17+
public function thirdArgument($a = 1, $b = 2, $c = 3)
1018
{
1119
}
1220
}

‎rules-tests/Arguments/Rector/ClassMethod/ArgumentAdderRector/config/configured_rule.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@
5353
ArgumentAddingScope::SCOPE_CLASS_METHOD
5454
),
5555
new ArgumentAdder(SomeClass::class, 'withoutTypeOrDefaultValue', 0, 'arguments', [], $arrayType),
56-
new ArgumentAdder(SomeMultiArg::class, 'run', 2, 'c', 4),
56+
new ArgumentAdder(SomeMultiArg::class, 'firstArgument', 0, 'a', 4),
57+
new ArgumentAdder(SomeMultiArg::class, 'secondArgument', 1, 'b', 5),
58+
new ArgumentAdder(SomeMultiArg::class, 'thirdArgument', 2, 'c', 6),
5759
new ArgumentAdder(SomeClass::class, 'someMethod', 0, 'default', 1),
5860
new ArgumentAdderWithoutDefaultValue(WithoutDefaultValue::class, 'someMethod', 0, 'foo', $arrayType),
5961
]);

‎rules/Arguments/Rector/ClassMethod/ArgumentAdderRector.php‎

Lines changed: 38 additions & 8 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_;
@@ -24,6 +25,7 @@
2425
use Rector\Contract\Rector\ConfigurableRectorInterface;
2526
use Rector\Enum\ObjectReference;
2627
use Rector\Exception\ShouldNotHappenException;
28+
use Rector\NodeAnalyzer\ArgsAnalyzer;
2729
use Rector\PhpParser\AstResolver;
2830
use Rector\PHPStanStaticTypeMapper\Enum\TypeKind;
2931
use Rector\Rector\AbstractRector;
@@ -48,7 +50,8 @@ public function __construct(
4850
private readonly ArgumentAddingScope $argumentAddingScope,
4951
private readonly ChangedArgumentsDetector $changedArgumentsDetector,
5052
private readonly AstResolver $astResolver,
51-
private readonly StaticTypeMapper $staticTypeMapper
53+
private readonly StaticTypeMapper $staticTypeMapper,
54+
private readonly ArgsAnalyzer $argsAnalyzer
5255
) {
5356
}
5457

@@ -181,13 +184,21 @@ private function processMethodCall(
181184

182185
$defaultValue = $argumentAdder->getArgumentDefaultValue();
183186
$arg = new Arg(BuilderHelpers::normalizeValue($defaultValue));
184-
if (isset($methodCall->args[$position])) {
185-
return;
187+
188+
// if there are named argyments, we just add it at the end as a new named argument
189+
if ($this->argsAnalyzer->hasNamedArg($methodCall->getArgs())) {
190+
$argumentName = $argumentAdder->getArgumentName();
191+
if ($argumentName === null) {
192+
throw new ShouldNotHappenException();
193+
}
194+
195+
$arg->name = new Identifier($argumentName);
196+
} else {
197+
$this->fillGapBetweenWithDefaultValue($methodCall, $position);
186198
}
187199

188-
$this->fillGapBetweenWithDefaultValue($methodCall, $position);
200+
$methodCall->args[] = $arg;
189201

190-
$methodCall->args[$position] = $arg;
191202
$this->hasChanged = true;
192203
}
193204

@@ -249,7 +260,20 @@ private function shouldSkipParameter(
249260
return $this->changedArgumentsDetector->isTypeChanged($param, $argumentAdder->getArgumentType());
250261
}
251262

252-
if (isset($node->args[$position])) {
263+
$arguments = $node->getArgs();
264+
$firstNamedArgumentPosition = $this->argsAnalyzer->resolveFirstNamedArgPosition($arguments);
265+
// If named arguments exist
266+
if ($firstNamedArgumentPosition !== null) {
267+
// Check if the parameter we're trying to add is before the first named argument
268+
if ($position < $firstNamedArgumentPosition) {
269+
return true; //if that is the case, the parameter already exists, skip
270+
}
271+
272+
// Check if the parameter we're trying to add is already present as a named argument
273+
if ($this->argsAnalyzer->resolveArgPosition($arguments, $argumentName, -1) !== -1) {
274+
return true; // if it exists as a named argument, skip
275+
}
276+
} elseif (isset($node->args[$position])) {
253277
return true;
254278
}
255279

@@ -333,9 +357,15 @@ private function processStaticCall(
333357
return;
334358
}
335359

336-
$this->fillGapBetweenWithDefaultValue($staticCall, $position);
360+
// if there are named arguments, we just add it at the end as a new named argument
361+
$arg = new Arg(new Variable($argumentName));
362+
if ($this->argsAnalyzer->hasNamedArg($staticCall->getArgs())) {
363+
$arg->name = new Identifier($argumentName);
364+
} else {
365+
$this->fillGapBetweenWithDefaultValue($staticCall, $position);
366+
}
337367

338-
$staticCall->args[$position] = new Arg(new Variable($argumentName));
368+
$staticCall->args[] = $arg;
339369
$this->hasChanged = true;
340370
}
341371

‎src/NodeAnalyzer/ArgsAnalyzer.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,21 @@ public function resolveArgPosition(array $args, string $name, int $defaultPositi
4848

4949
return $defaultPosition;
5050
}
51+
52+
/**
53+
* @param Arg[] $args
54+
*/
55+
public function resolveFirstNamedArgPosition(array $args): ?int
56+
{
57+
$position = 0;
58+
foreach ($args as $arg) {
59+
if ($arg->name instanceof Identifier) {
60+
return $position;
61+
}
62+
63+
++$position;
64+
}
65+
66+
return null;
67+
}
5168
}

0 commit comments

Comments
 (0)