Skip to content

Commit cf3936e

Browse files
authored
[CodeQuality] Handle different parameter type same name used on ControllerMethodInjectionToConstructorRector (#929)
1 parent ba413d9 commit cf3936e

File tree

2 files changed

+77
-6
lines changed

2 files changed

+77
-6
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
6+
7+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
8+
9+
class DifferentParameterTypeSameNameUsed extends AbstractController
10+
{
11+
public function one(Foo $service)
12+
{
13+
dump($service);
14+
}
15+
16+
public function two(Foo $service)
17+
{
18+
dump($service);
19+
}
20+
21+
public function three(Bar $service)
22+
{
23+
dump($service);
24+
}
25+
}
26+
27+
?>
28+
-----
29+
<?php
30+
31+
declare(strict_types=1);
32+
33+
namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
34+
35+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
36+
37+
class DifferentParameterTypeSameNameUsed extends AbstractController
38+
{
39+
public function __construct(private readonly \Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture\Foo $service)
40+
{
41+
}
42+
public function one()
43+
{
44+
dump($this->service);
45+
}
46+
47+
public function two()
48+
{
49+
dump($this->service);
50+
}
51+
52+
public function three(Bar $service)
53+
{
54+
dump($service);
55+
}
56+
}
57+
58+
?>

rules/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector.php

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,12 @@ public function refactor(Node $node): ?Node
131131
}
132132
}
133133

134+
/** @var array<array{ClassMethod, int}> $paramsToRemove */
135+
$paramsToRemove = [];
136+
137+
/** @var array<string, string[]> $methodParamNamesToReplace */
138+
$methodParamNamesToReplace = [];
139+
134140
foreach ($node->getMethods() as $classMethod) {
135141
if ($this->shouldSkipClassMethod($classMethod)) {
136142
continue;
@@ -213,8 +219,10 @@ public function refactor(Node $node): ?Node
213219
continue;
214220
}
215221

216-
unset($classMethod->params[$key]);
217-
$propertyMetadatas[] = new PropertyMetadata($this->getName($param->var), $paramType);
222+
$paramName = $this->getName($param->var);
223+
$paramsToRemove[] = [$classMethod, $key];
224+
$propertyMetadatas[$paramName] = new PropertyMetadata($paramName, $paramType);
225+
$methodParamNamesToReplace[$classMethod->name->toString()][] = $paramName;
218226
}
219227
}
220228

@@ -223,9 +231,9 @@ public function refactor(Node $node): ?Node
223231
return null;
224232
}
225233

226-
$paramNamesToReplace = [];
227-
foreach ($propertyMetadatas as $propertyMetadata) {
228-
$paramNamesToReplace[] = $propertyMetadata->getName();
234+
// defer param removal to after collection to avoid mutation during iteration
235+
foreach ($paramsToRemove as [$methodToModify, $paramKey]) {
236+
unset($methodToModify->params[$paramKey]);
229237
}
230238

231239
// 1. update constructor
@@ -238,7 +246,12 @@ public function refactor(Node $node): ?Node
238246
continue;
239247
}
240248

241-
$this->replaceParamUseWithPropertyFetch($classMethod, $paramNamesToReplace);
249+
$methodName = $classMethod->name->toString();
250+
if (! isset($methodParamNamesToReplace[$methodName])) {
251+
continue;
252+
}
253+
254+
$this->replaceParamUseWithPropertyFetch($classMethod, $methodParamNamesToReplace[$methodName]);
242255
}
243256

244257
return $node;

0 commit comments

Comments
 (0)