Skip to content

Commit 0d0cd6c

Browse files
TomasVotrubaTomasVotruba
authored
[code-quality] Add ControllerMethodInjectionToConstructorRector (#865)
1 parent 290b5c7 commit 0d0cd6c

File tree

11 files changed

+385
-12
lines changed

11 files changed

+385
-12
lines changed

‎config/sets/symfony/symfony-code-quality.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Rector\Symfony\CodeQuality\Rector\AttributeGroup\SingleConditionSecurityAttributeToIsGrantedRector;
77
use Rector\Symfony\CodeQuality\Rector\BinaryOp\RequestIsMainRector;
88
use Rector\Symfony\CodeQuality\Rector\BinaryOp\ResponseStatusCodeRector;
9+
use Rector\Symfony\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector;
910
use Rector\Symfony\CodeQuality\Rector\Class_\EventListenerToEventSubscriberRector;
1011
use Rector\Symfony\CodeQuality\Rector\Class_\InlineClassRoutePrefixRector;
1112
use Rector\Symfony\CodeQuality\Rector\Class_\LoadValidatorMetadataToAnnotationRector;
@@ -32,7 +33,10 @@
3233

3334
RemoveUnusedRequestParamRector::class,
3435
ParamTypeFromRouteRequiredRegexRector::class,
36+
37+
// controller
3538
ActionSuffixRemoverRector::class,
39+
ControllerMethodInjectionToConstructorRector::class,
3640
LoadValidatorMetadataToAnnotationRector::class,
3741

3842
// request method

‎config/sets/symfony/symfony7/symfony74/symfony74-json-streamer.php‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,35 @@
99
return static function (RectorConfig $rectorConfig): void {
1010
// @see https://github.com/symfony/symfony/blob/7.4/UPGRADE-7.4.md#jsonstreamer
1111
$rectorConfig->ruleWithConfiguration(RenameMethodRector::class, [
12-
new MethodCallRename('Symfony\Component\JsonStreamer\Mapping\PropertyMetadata', 'getNativeToStreamValueTransformer', 'getValueTransformers'),
13-
new MethodCallRename('Symfony\Component\JsonStreamer\Mapping\PropertyMetadata', 'getStreamToNativeValueTransformers', 'getValueTransformers'),
14-
new MethodCallRename('Symfony\Component\JsonStreamer\Mapping\PropertyMetadata', 'withNativeToStreamValueTransformers', 'withValueTransformers'),
15-
new MethodCallRename('Symfony\Component\JsonStreamer\Mapping\PropertyMetadata', 'withStreamToNativeValueTransformers', 'withValueTransformers'),
16-
new MethodCallRename('Symfony\Component\JsonStreamer\Mapping\PropertyMetadata', 'withAdditionalNativeToStreamValueTransformer', 'withAdditionalValueTransformer'),
17-
new MethodCallRename('Symfony\Component\JsonStreamer\Mapping\PropertyMetadata', 'withAdditionalStreamToNativeValueTransformer', 'withAdditionalValueTransformer'),
12+
new MethodCallRename(
13+
'Symfony\Component\JsonStreamer\Mapping\PropertyMetadata',
14+
'getNativeToStreamValueTransformer',
15+
'getValueTransformers'
16+
),
17+
new MethodCallRename(
18+
'Symfony\Component\JsonStreamer\Mapping\PropertyMetadata',
19+
'getStreamToNativeValueTransformers',
20+
'getValueTransformers'
21+
),
22+
new MethodCallRename(
23+
'Symfony\Component\JsonStreamer\Mapping\PropertyMetadata',
24+
'withNativeToStreamValueTransformers',
25+
'withValueTransformers'
26+
),
27+
new MethodCallRename(
28+
'Symfony\Component\JsonStreamer\Mapping\PropertyMetadata',
29+
'withStreamToNativeValueTransformers',
30+
'withValueTransformers'
31+
),
32+
new MethodCallRename(
33+
'Symfony\Component\JsonStreamer\Mapping\PropertyMetadata',
34+
'withAdditionalNativeToStreamValueTransformer',
35+
'withAdditionalValueTransformer'
36+
),
37+
new MethodCallRename(
38+
'Symfony\Component\JsonStreamer\Mapping\PropertyMetadata',
39+
'withAdditionalStreamToNativeValueTransformer',
40+
'withAdditionalValueTransformer'
41+
),
1842
]);
1943
};

‎rules-tests/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector/ControllerMethodInjectionToConstructorRectorTest.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\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector;
6+
7+
use Iterator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
10+
11+
final class ControllerMethodInjectionToConstructorRectorTest 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/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector/Fixture/re_use_existing_service.php.inc‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
6+
7+
use Psr\Log\LoggerInterface;
8+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
9+
use Symfony\Component\Routing\Annotation\Route;
10+
11+
final class ReUseExistingService extends AbstractController
12+
{
13+
public function __construct(private LoggerInterface $logger)
14+
{
15+
}
16+
17+
#[Route('/some-action', name: 'some_action')]
18+
public function someAction(
19+
\Symfony\Component\HttpFoundation\Request $request,
20+
\Psr\Log\LoggerInterface $logger
21+
) {
22+
$logger->log('level', 'value');
23+
}
24+
}
25+
26+
?>
27+
-----
28+
<?php
29+
30+
declare(strict_types=1);
31+
32+
namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
33+
34+
use Psr\Log\LoggerInterface;
35+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
36+
use Symfony\Component\Routing\Annotation\Route;
37+
38+
final class ReUseExistingService extends AbstractController
39+
{
40+
public function __construct(private readonly \Psr\Log\LoggerInterface $logger)
41+
{
42+
}
43+
44+
#[Route('/some-action', name: 'some_action')]
45+
public function someAction(
46+
\Symfony\Component\HttpFoundation\Request $request
47+
) {
48+
$this->logger->log('level', 'value');
49+
}
50+
}
51+
52+
?>

‎rules-tests/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector/Fixture/skip_scalar_and_request_params.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\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
6+
7+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
8+
use Symfony\Component\Routing\Annotation\Route;
9+
10+
final class SkipScalarAndRequestParams extends AbstractController
11+
{
12+
#[Route('/some-action', name: 'some_action')]
13+
public function someAction(
14+
\Symfony\Component\HttpFoundation\Request $request,
15+
int $number
16+
) {
17+
}
18+
}

‎rules-tests/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector/Fixture/some_controller_with_method_injection.php.inc‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
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+
use Symfony\Component\Routing\Annotation\Route;
9+
10+
final class SomeControllerWithMethodInjection extends AbstractController
11+
{
12+
#[Route('/some-action', name: 'some_action')]
13+
public function someAction(
14+
\Symfony\Component\HttpFoundation\Request $request,
15+
\Psr\Log\LoggerInterface $logger
16+
) {
17+
$logger->log('level', 'value');
18+
}
19+
}
20+
21+
?>
22+
-----
23+
<?php
24+
25+
declare(strict_types=1);
26+
27+
namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
28+
29+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
30+
use Symfony\Component\Routing\Annotation\Route;
31+
32+
final class SomeControllerWithMethodInjection extends AbstractController
33+
{
34+
public function __construct(private readonly \Psr\Log\LoggerInterface $logger)
35+
{
36+
}
37+
#[Route('/some-action', name: 'some_action')]
38+
public function someAction(
39+
\Symfony\Component\HttpFoundation\Request $request
40+
) {
41+
$this->logger->log('level', 'value');
42+
}
43+
}
44+
45+
?>

‎rules-tests/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector/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\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector;
7+
8+
return static function (RectorConfig $rectorConfig): void {
9+
$rectorConfig->rule(ControllerMethodInjectionToConstructorRector::class);
10+
};

‎rules/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector.php‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Symfony\CodeQuality\Rector\Class_;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr\PropertyFetch;
9+
use PhpParser\Node\Expr\Variable;
10+
use PhpParser\Node\Identifier;
11+
use PhpParser\Node\Name;
12+
use PhpParser\Node\Stmt\Class_;
13+
use Rector\NodeManipulator\ClassDependencyManipulator;
14+
use Rector\PostRector\ValueObject\PropertyMetadata;
15+
use Rector\Rector\AbstractRector;
16+
use Rector\StaticTypeMapper\StaticTypeMapper;
17+
use Rector\Symfony\Bridge\NodeAnalyzer\ControllerMethodAnalyzer;
18+
use Rector\Symfony\Enum\SymfonyClass;
19+
use Rector\Symfony\TypeAnalyzer\ControllerAnalyzer;
20+
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
21+
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
22+
23+
/**
24+
* @see \Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\ControllerMethodInjectionToConstructorRectorTest
25+
*/
26+
final class ControllerMethodInjectionToConstructorRector extends AbstractRector
27+
{
28+
public function __construct(
29+
private readonly ControllerAnalyzer $controllerAnalyzer,
30+
private readonly ControllerMethodAnalyzer $controllerMethodAnalyzer,
31+
private readonly ClassDependencyManipulator $classDependencyManipulator,
32+
private readonly StaticTypeMapper $staticTypeMapper,
33+
) {
34+
}
35+
36+
public function getRuleDefinition(): RuleDefinition
37+
{
38+
return new RuleDefinition(
39+
'Change Symfony controller method injection to direct constructor dependency, to separate params and services clearly',
40+
[
41+
new CodeSample(
42+
<<<'CODE_SAMPLE'
43+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
44+
use Symfony\Component\HttpFoundation\Request;
45+
use Symfony\Component\Routing\Annotation\Route;
46+
47+
final class SomeController extends AbstractController
48+
{
49+
#[Route('/some-path', name: 'some_name')]
50+
public function someAction(
51+
Request $request,
52+
SomeService $someService
53+
) {
54+
$data = $someService->getData();
55+
}
56+
}
57+
CODE_SAMPLE
58+
,
59+
<<<'CODE_SAMPLE'
60+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
61+
use Symfony\Component\HttpFoundation\Request;
62+
use Symfony\Component\Routing\Annotation\Route;
63+
64+
final class SomeController extends AbstractController
65+
{
66+
public function __construct(
67+
private readonly SomeService $someService
68+
) {
69+
}
70+
71+
#[Route('/some-path', name: 'some_name')]
72+
public function someAction(
73+
Request $request
74+
) {
75+
$data = $this->someService->getData();
76+
}
77+
}
78+
CODE_SAMPLE
79+
),
80+
]
81+
);
82+
}
83+
84+
/**
85+
* @return array<class-string<Node>>
86+
*/
87+
public function getNodeTypes(): array
88+
{
89+
return [Class_::class];
90+
}
91+
92+
/**
93+
* @param Class_ $node
94+
*/
95+
public function refactor(Node $node): ?Node
96+
{
97+
if (! $this->controllerAnalyzer->isController($node)) {
98+
return null;
99+
}
100+
101+
$propertyMetadatas = [];
102+
103+
foreach ($node->getMethods() as $classMethod) {
104+
if (! $this->controllerMethodAnalyzer->isAction($classMethod)) {
105+
continue;
106+
}
107+
108+
foreach ($classMethod->getParams() as $key => $param) {
109+
// skip scalar and empty values, as not services
110+
if ($param->type === null || $param->type instanceof Identifier) {
111+
continue;
112+
}
113+
114+
// request is allowed
115+
if ($param->type instanceof Name && $this->isName($param->type, SymfonyClass::REQUEST)) {
116+
continue;
117+
}
118+
119+
// @todo allow parameter converter
120+
unset($classMethod->params[$key]);
121+
122+
$paramType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($param->type);
123+
$propertyMetadatas[] = new PropertyMetadata($this->getName($param->var), $paramType);
124+
}
125+
}
126+
127+
// nothing to move
128+
if ($propertyMetadatas === []) {
129+
return null;
130+
}
131+
132+
$paramNamesToReplace = [];
133+
foreach ($propertyMetadatas as $propertyMetadata) {
134+
$paramNamesToReplace[] = $propertyMetadata->getName();
135+
}
136+
137+
// 1. update constructor
138+
foreach ($propertyMetadatas as $propertyMetadata) {
139+
$this->classDependencyManipulator->addConstructorDependency($node, $propertyMetadata);
140+
}
141+
142+
foreach ($node->getMethods() as $classMethod) {
143+
if (! $this->controllerMethodAnalyzer->isAction($classMethod)) {
144+
continue;
145+
}
146+
147+
// replace param use with property fetch
148+
$this->traverseNodesWithCallable((array) $classMethod->stmts, function (Node $node) use (
149+
$paramNamesToReplace
150+
): ?PropertyFetch {
151+
if (! $node instanceof Variable) {
152+
return null;
153+
}
154+
155+
if (! $this->isNames($node, $paramNamesToReplace)) {
156+
return null;
157+
158+
}
159+
160+
$propertyName = $this->getName($node);
161+
return new PropertyFetch(new Variable('this'), $propertyName);
162+
});
163+
}
164+
165+
// 2. replace in method bodies
166+
167+
return $node;
168+
}
169+
}

0 commit comments

Comments
 (0)