Skip to content

Commit 067b131

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

File tree

10 files changed

+333
-12
lines changed

10 files changed

+333
-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/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\Identifier;
8+
use PhpParser\Node\Name;
9+
use PhpParser\Node\Expr\PropertyFetch;
10+
use PhpParser\Node\Expr\Variable;
11+
use PhpParser\Node;
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+
}

‎rules/Symfony61/Rector/StaticPropertyFetch/ErrorNamesPropertyToConstantRector.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,10 @@ public function refactor(Node $node): ?Node
111111
}
112112

113113
private function refactorStaticPropertyFetch(
114-
StaticPropertyFetch $node,
114+
StaticPropertyFetch $staticPropertyFetch,
115115
ClassReflection $classReflection
116116
): ?ClassConstFetch {
117-
if (! $this->isName($node->name, 'errorNames')) {
117+
if (! $this->isName($staticPropertyFetch->name, 'errorNames')) {
118118
return null;
119119
}
120120

‎rules/Symfony73/GetMethodToAsTwigAttributeTransformer.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,10 +182,8 @@ private function getArgumentsFromOptionArray(?Arg $optionArgument, array $additi
182182
continue;
183183
}
184184

185-
if ($mappedName === 'isSafeCallback') {
186-
if ($item->value instanceof MethodCall && $item->value->isFirstClassCallable()) {
187-
continue;
188-
}
185+
if ($mappedName === 'isSafeCallback' && ($item->value instanceof MethodCall && $item->value->isFirstClassCallable())) {
186+
continue;
189187
}
190188

191189
$arg = new Arg($item->value);

‎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)