Skip to content

Commit 4211232

Browse files
TomasVotrubaTomasVotruba
committed
[code-quality] Add ControllerMethodInjectionToConstructorRector
1 parent 290b5c7 commit 4211232

File tree

8 files changed

+325
-6
lines changed

8 files changed

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

‎stubs/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,29 @@ public function stream(string $view, array $parameters = [], StreamedResponse $r
7878
protected function isGranted($attributes, $subject = null): bool
7979
{
8080
}
81+
82+
public function set(string $id, ?object $service)
83+
{
84+
// TODO: Implement set() method.
85+
}
86+
87+
public function initialized(string $id): bool
88+
{
89+
// TODO: Implement initialized() method.
90+
}
91+
92+
public function getParameter(string $name)
93+
{
94+
// TODO: Implement getParameter() method.
95+
}
96+
97+
public function hasParameter(string $name): bool
98+
{
99+
// TODO: Implement hasParameter() method.
100+
}
101+
102+
public function setParameter(string $name, \UnitEnum|float|array|bool|int|string|null $value)
103+
{
104+
// TODO: Implement setParameter() method.
105+
}
81106
}

0 commit comments

Comments
 (0)