Skip to content

Commit cda0ade

Browse files
TomasVotrubaTomasVotruba
committed
[config] Add RemoveConstructorAutowireServiceRector
1 parent 6337a1b commit cda0ade

File tree

15 files changed

+436
-9
lines changed

15 files changed

+436
-9
lines changed

‎config/sets/symfony/configs.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Rector\Config\RectorConfig;
66
use Rector\Symfony\Configs\Rector\Closure\MergeServiceNameTypeRector;
7+
use Rector\Symfony\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector;
78
use Rector\Symfony\Configs\Rector\Closure\ServiceArgsToServiceNamedArgRector;
89
use Rector\Symfony\Configs\Rector\Closure\ServiceSetStringNameToClassNameRector;
910
use Rector\Symfony\Configs\Rector\Closure\ServiceSettersToSettersAutodiscoveryRector;
@@ -16,5 +17,6 @@
1617
ServiceSetStringNameToClassNameRector::class,
1718
ServiceSettersToSettersAutodiscoveryRector::class,
1819
ServiceTagsToDefaultsAutoconfigureRector::class,
20+
RemoveConstructorAutowireServiceRector::class,
1921
]);
2022
};

‎rules-tests/Configs/Rector/Closure/RemoveConstructorAutowireServiceRector/Fixture/avoid_different_type.php.inc‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Fixture;
6+
7+
use Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Source\AnotherClassWithoutConstructor;
8+
use Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Source\DifferentType;
9+
use Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Source\PassedAsDependency;
10+
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
11+
use function Symfony\Component\DependencyInjection\Loader\Configurator\service;
12+
13+
return static function (ContainerConfigurator $containerConfigurator): void {
14+
$services = $containerConfigurator->services();
15+
16+
$services->set(AnotherClassWithoutConstructor::class)
17+
->arg('$passedAsDependency', service(DifferentType::class));
18+
};

‎rules-tests/Configs/Rector/Closure/RemoveConstructorAutowireServiceRector/Fixture/skip_no_service_type.php.inc‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Fixture;
6+
7+
use Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Source\AnotherClassWithoutConstructor;
8+
use Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Source\PassedAsDependency;
9+
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
10+
use function Symfony\Component\DependencyInjection\Loader\Configurator\service;
11+
12+
return static function (ContainerConfigurator $containerConfigurator): void {
13+
$services = $containerConfigurator->services();
14+
15+
$services->set(AnotherClassWithoutConstructor::class)
16+
->arg('$passedAsDependency', 123);
17+
};

‎rules-tests/Configs/Rector/Closure/RemoveConstructorAutowireServiceRector/Fixture/some_class.php.inc‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Fixture;
6+
7+
use Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Source\AnotherClassWithoutConstructor;
8+
use Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Source\PassedAsDependency;
9+
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
10+
use function Symfony\Component\DependencyInjection\Loader\Configurator\service;
11+
12+
return static function (ContainerConfigurator $containerConfigurator): void {
13+
$services = $containerConfigurator->services();
14+
15+
$services->set(AnotherClassWithoutConstructor::class)
16+
->arg('$passedAsDependency', service(PassedAsDependency::class));
17+
};
18+
19+
?>
20+
-----
21+
<?php
22+
23+
declare(strict_types=1);
24+
25+
namespace Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Fixture;
26+
27+
use Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Source\AnotherClassWithoutConstructor;
28+
use Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Source\PassedAsDependency;
29+
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
30+
use function Symfony\Component\DependencyInjection\Loader\Configurator\service;
31+
32+
return static function (ContainerConfigurator $containerConfigurator): void {
33+
$services = $containerConfigurator->services();
34+
35+
$services->set(AnotherClassWithoutConstructor::class);
36+
};
37+
38+
?>

‎rules-tests/Configs/Rector/Closure/RemoveConstructorAutowireServiceRector/RemoveConstructorAutowireServiceRectorTest.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector;
6+
7+
use Iterator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
10+
11+
final class RemoveConstructorAutowireServiceRectorTest extends AbstractRectorTestCase
12+
{
13+
#[DataProvider('provideData')]
14+
public function test(string $filePath): void
15+
{
16+
$this->doTestFile($filePath);
17+
}
18+
19+
public static function provideData(): Iterator
20+
{
21+
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
22+
}
23+
24+
public function provideConfigFilePath(): string
25+
{
26+
return __DIR__ . '/config/configured_rule.php';
27+
}
28+
}

‎rules-tests/Configs/Rector/Closure/RemoveConstructorAutowireServiceRector/Source/AnotherClassWithoutConstructor.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
3+
namespace Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Source;
4+
5+
final class AnotherClassWithoutConstructor
6+
{
7+
public function __construct(
8+
PassedAsDependency $passedAsDependency
9+
) {
10+
}
11+
}

‎rules-tests/Configs/Rector/Closure/RemoveConstructorAutowireServiceRector/Source/DifferentType.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Source;
4+
5+
final class DifferentType
6+
{
7+
}

‎rules-tests/Configs/Rector/Closure/RemoveConstructorAutowireServiceRector/Source/PassedAsDependency.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
namespace Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\Source;
4+
5+
final class PassedAsDependency
6+
{
7+
}

‎rules-tests/Configs/Rector/Closure/RemoveConstructorAutowireServiceRector/config/configured_rule.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Rector\Config\RectorConfig;
6+
use Rector\Symfony\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector;
7+
8+
return static function (RectorConfig $rectorConfig): void {
9+
$rectorConfig->rule(RemoveConstructorAutowireServiceRector::class);
10+
};

‎rules/Configs/Rector/Closure/RemoveConstructorAutowireServiceRector.php‎

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Symfony\Configs\Rector\Closure;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr;
9+
use PhpParser\Node\Expr\ClassConstFetch;
10+
use PhpParser\Node\Expr\Closure;
11+
use PhpParser\Node\Expr\FuncCall;
12+
use PhpParser\Node\Expr\MethodCall;
13+
use PhpParser\Node\Scalar\String_;
14+
use PHPStan\Type\ObjectType;
15+
use Rector\PhpParser\Node\Value\ValueResolver;
16+
use Rector\Rector\AbstractRector;
17+
use Rector\Symfony\Configs\Rector\Reflection\ConstructorReflectionTypesResolver;
18+
use Rector\Symfony\Enum\SymfonyClass;
19+
use Rector\Symfony\Enum\SymfonyFunction;
20+
use Rector\Symfony\NodeAnalyzer\SymfonyPhpClosureDetector;
21+
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
22+
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
23+
24+
/**
25+
* @see \Rector\Symfony\Tests\Configs\Rector\Closure\RemoveConstructorAutowireServiceRector\RemoveConstructorAutowireServiceRectorTest
26+
*/
27+
final class RemoveConstructorAutowireServiceRector extends AbstractRector
28+
{
29+
private bool $hasChanged = false;
30+
31+
public function __construct(
32+
private readonly SymfonyPhpClosureDetector $symfonyPhpClosureDetector,
33+
private readonly ValueResolver $valueResolver,
34+
private readonly ConstructorReflectionTypesResolver $constructorReflectionTypesResolver,
35+
) {
36+
}
37+
38+
public function getRuleDefinition(): RuleDefinition
39+
{
40+
return new RuleDefinition(
41+
'Remove service that is passed as arg, but already autowired via constructor',
42+
[
43+
new CodeSample(
44+
<<<'CODE_SAMPLE'
45+
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
46+
47+
return static function (ContainerConfigurator $containerConfigurator): void {
48+
$services = $containerConfigurator->services();
49+
50+
$services->defaults()
51+
->autowire();
52+
53+
$services->set(\App\SomeClass::class)
54+
->arg('$someService', ref(\App\SomeService::class));
55+
};
56+
57+
final class SomeClass
58+
{
59+
public function __construct(private SomeService $someService)
60+
{
61+
}
62+
}
63+
CODE_SAMPLE
64+
65+
,
66+
<<<'CODE_SAMPLE'
67+
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
68+
69+
return static function (ContainerConfigurator $containerConfigurator): void {
70+
$services = $containerConfigurator->services();
71+
72+
$services->defaults()
73+
->autowire();
74+
75+
$services->set(\App\SomeClass::class);
76+
};
77+
78+
final class SomeClass
79+
{
80+
public function __construct(private SomeService $someService)
81+
{
82+
}
83+
}
84+
CODE_SAMPLE
85+
),
86+
87+
]
88+
);
89+
}
90+
91+
/**
92+
* @return array<class-string<Node>>
93+
*/
94+
public function getNodeTypes(): array
95+
{
96+
return [Closure::class];
97+
}
98+
99+
/**
100+
* @param Closure $node
101+
*/
102+
public function refactor(Node $node): ?Node
103+
{
104+
$this->hasChanged = false;
105+
106+
if (! $this->symfonyPhpClosureDetector->detect($node)) {
107+
return null;
108+
}
109+
110+
$this->handleSetServices($node);
111+
112+
if ($this->hasChanged) {
113+
return $node;
114+
}
115+
116+
return null;
117+
}
118+
119+
private function handleSetServices(Closure $closure): void
120+
{
121+
$this->traverseNodesWithCallable($closure->stmts, function (Node $node): ?Expr {
122+
if (! $node instanceof MethodCall) {
123+
return null;
124+
}
125+
126+
if (! $this->isName($node->name, 'arg')) {
127+
return null;
128+
}
129+
130+
$serviceClass = $this->matchSetServicesClass($node);
131+
if (! is_string($serviceClass)) {
132+
return null;
133+
}
134+
135+
$constructorTypesByParameterName = $this->constructorReflectionTypesResolver->resolve($serviceClass);
136+
if ($constructorTypesByParameterName === null) {
137+
return null;
138+
}
139+
140+
$argName = $node->getArgs()[0]
141+
->value;
142+
$serviceArgExpr = $node->getArgs()[1]
143+
->value;
144+
145+
if (! $argName instanceof String_) {
146+
return null;
147+
}
148+
149+
$bareParameterName = ltrim($argName->value, '$');
150+
$knownParameterType = $constructorTypesByParameterName[$bareParameterName] ?? null;
151+
if (! $knownParameterType instanceof ObjectType) {
152+
return null;
153+
}
154+
155+
if (! $this->isParameterTypeMatchingPassedArgExprClass($serviceArgExpr, $knownParameterType)) {
156+
return null;
157+
}
158+
159+
return $node->var;
160+
161+
// return NodeVisitor::REMOVE_NODE;
162+
});
163+
}
164+
165+
private function isSetServices(MethodCall $methodCall): bool
166+
{
167+
if (! $this->isName($methodCall->name, 'set')) {
168+
return false;
169+
}
170+
171+
return $this->isObjectType($methodCall->var, new ObjectType(SymfonyClass::SERVICES_CONFIGURATOR));
172+
}
173+
174+
private function matchSetServicesClass(MethodCall $methodCall): ?string
175+
{
176+
while ($methodCall instanceof MethodCall) {
177+
if ($this->isSetServices($methodCall)) {
178+
break;
179+
}
180+
181+
$methodCall = $methodCall->var;
182+
}
183+
184+
/** @var MethodCall $methodCall */
185+
$firstArg = $methodCall->getArgs()[0];
186+
if (! $firstArg->value instanceof ClassConstFetch) {
187+
return null;
188+
}
189+
190+
return $this->valueResolver->getValue($firstArg->value);
191+
}
192+
193+
private function isParameterTypeMatchingPassedArgExprClass(
194+
Expr $serviceArgExpr,
195+
ObjectType $objectType
196+
): bool {
197+
if (! $serviceArgExpr instanceof FuncCall) {
198+
return false;
199+
}
200+
201+
if (! $this->isName($serviceArgExpr->name, SymfonyFunction::SERVICE)) {
202+
return false;
203+
}
204+
205+
$dependencyServiceExpr = $serviceArgExpr->getArgs()[0]
206+
->value;
207+
$dependencyService = $this->valueResolver->getValue($dependencyServiceExpr);
208+
209+
return $dependencyService === $objectType->getClassName();
210+
}
211+
}

0 commit comments

Comments
 (0)