8000 [code-quality] Improve TemplateAnnotationToThisRenderRector to keep t… · JohJohan/rector-symfony@7d5f810 · GitHub
[go: up one dir, main page]
Skip to content

Commit 7d5f810

Browse files
TomasVotrubaTomasVotruba
authored
[code-quality] Improve TemplateAnnotationToThisRenderRector to keep ternary types (rectorphp#758)
* add simple ternary test fixture * improve to keep teranry types
1 parent 6de3184 commit 7d5f810

File tree

9 files changed

+134
-83
lines changed

9 files changed

+134
-83
lines changed

‎config/sets/symfony/symfony6/symfony60/symfony60-config.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,17 @@
22

33
declare(strict_types=1);
44

5+
use PHPStan\Type\ArrayType;
6+
use PHPStan\Type\BooleanType;
57
use PHPStan\Type\MixedType;
8+
use PHPStan\Type\ObjectType;
9+
use PHPStan\Type\StringType;
10+
use PHPStan\Type\UnionType;
611
use Rector\Config\RectorConfig;
712
use Rector\TypeDeclaration\Rector\ClassMethod\AddParamTypeDeclarationRector;
813
use Rector\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationRector;
914
use Rector\TypeDeclaration\ValueObject\AddParamTypeDeclaration;
1015
use Rector\TypeDeclaration\ValueObject\AddReturnTypeDeclaration;
11-
use PHPStan\Type\BooleanType;
12-
use PHPStan\Type\ObjectType;
13-
use PHPStan\Type\StringType;
14-
use PHPStan\Type\UnionType;
15-
use PHPStan\Type\ArrayType;
1616

1717
return static function (RectorConfig $rectorConfig): void {
1818
$rectorConfig->ruleWithConfiguration(AddParamTypeDeclarationRector::class, [

‎config/sets/symfony/symfony6/symfony60/symfony60-dependency-injection.php

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,22 @@
33
declare(strict_types=1);
44

55
use PhpParser\Node\Scalar\String_;
6-
use Rector\Config\RectorConfig;
7-
use Rector\Symfony\Symfony60\Rector\FuncCall\ReplaceServiceArgumentRector;
8-
use Rector\Symfony\ValueObject\ReplaceServiceArgument;
9-
use Rector\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationRector;
10-
use Rector\TypeDeclaration\ValueObject\AddReturnTypeDeclaration;
6+
use PHPStan\Type\ArrayType;
7+
use PHPStan\Type\BooleanType;
118
use PHPStan\Type\Constant\ConstantBooleanType;
9+
use PHPStan\Type\FloatType;
10+
use PHPStan\Type\IntegerType;
1211
use PHPStan\Type\MixedType;
1312
use PHPStan\Type\NullType;
1413
use PHPStan\Type\ObjectType;
1514
use PHPStan\Type\ObjectWithoutClassType;
1615
use PHPStan\Type\StringType;
1716
use PHPStan\Type\UnionType;
18-
use PHPStan\Type\ArrayType;
19-
use PHPStan\Type\BooleanType;
20-
use PHPStan\Type\FloatType;
21-
use PHPStan\Type\IntegerType;
17+
use Rector\Config\RectorConfig;
18+
use Rector\Symfony\Symfony60\Rector\FuncCall\ReplaceServiceArgumentRector;
19+
use Rector\Symfony\ValueObject\ReplaceServiceArgument;
20+
use Rector\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationRector;
21+
use Rector\TypeDeclaration\ValueObject\AddReturnTypeDeclaration;
2222

2323
return static function (RectorConfig $rectorConfig): void {
2424
$rectorConfig->ruleWithConfiguration(ReplaceServiceArgumentRector::class, [

‎config/sets/symfony/symfony6/symfony60/symfony60-security-core.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,17 @@
22

33
declare(strict_types=1);
44

5-
use Rector\Config\RectorConfig;
6-
use Rector\Renaming\Rector\MethodCall\RenameMethodRector;
7-
use Rector\Renaming\ValueObject\MethodCallRename;
8-
use Rector\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationRector;
9-
use Rector\TypeDeclaration\ValueObject\AddReturnTypeDeclaration;
105
use PHPStan\Type\ArrayType;
116
use PHPStan\Type\BooleanType;
127
use PHPStan\Type\IntegerType;
138
use PHPStan\Type\MixedType;
149
use PHPStan\Type\ObjectType;
1510
use PHPStan\Type\StringType;
11+
use Rector\Config\RectorConfig;
12+
use Rector\Renaming\Rector\MethodCall\RenameMethodRector;
13+
use Rector\Renaming\ValueObject\MethodCallRename;
14+
use Rector\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationRector;
15+
use Rector\TypeDeclaration\ValueObject\AddReturnTypeDeclaration;
1616

1717
return static function (RectorConfig $rectorConfig): void {
1818
$rectorConfig->ruleWithConfiguration(RenameMethodRector::class, [

‎rules-tests/CodeQuality/Rector/ClassMethod/TemplateAnnotationToThisRenderRector/Fixture/Attributes/cleanup_return_array_shape.php.inc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php
22

3-
namespace AppBundle\Controller\Attributes;
3+
namespace Rector\Symfony\Tests\CodeQuality\Rector\ClassMethod\TemplateAnnotationToThisRenderRector\Fixture\Attributes;
44

55
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
66
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
@@ -23,7 +23,7 @@ final class CleanupArrayReturnShape extends AbstractController
2323
-----
2424
<?php
2525

26-
namespace AppBundle\Controller\Attributes;
26+
namespace Rector\Symfony\Tests\CodeQuality\Rector\ClassMethod\TemplateAnnotationToThisRenderRector\Fixture\Attributes;
2727

2828
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
2929
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;

‎rules-tests/CodeQuality/Rector/ClassMethod/TemplateAnnotationToThisRenderRector/Fixture/Attributes/nested_ternary_redirect.php.inc

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
namespace Rector\Symfony\Tests\CodeQuality\Rector\ClassMethod\TemplateAnnotationToThisRenderRector\Fixture\Attributes;
4+
5+
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
6+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
7+
8+
final class NestedTernaryRedirect extends AbstractController
9+
{
10+
#[Template("with_some_template.twig")]
11+
public function indexAction()
12+
{
13+
return mt_rand(0, 5) ? $this->redirectToRoute('one') : $this->redirectToRoute('two');
14+
}
15+
}
16+
17+
?>
18+
-----
19+
<?php
20+
21+
namespace Rector\Symfony\Tests\CodeQuality\Rector\ClassMethod\TemplateAnnotationToThisRenderRector\Fixture\Attributes;
22+
23+
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
24+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
25+
26+
final class NestedTernaryRedirect extends AbstractController
27+
{
28+
public function indexAction(): \Symfony\Component\HttpFoundation\Response
29+
{
30+
return mt_rand(0, 5) ? $this->redirectToRoute('one') : $this->redirectToRoute('two');
31+
}
32+
}
33+
34+
?>

‎rules-tests/CodeQuality/Rector/ClassMethod/TemplateAnnotationToThisRenderRector/Fixture/Attributes/nested_ternary_redirect_then_array.php.inc

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
namespace Rector\Symfony\Tests\CodeQuality\Rector\ClassMethod\TemplateAnnotationToThisRenderRector\Fixture\Attributes;
4+
5+
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
6+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
7+
8+
final class NestedTernaryRedirectThenArray extends AbstractController
9+
{
10+
#[Template("with_some_template.twig")]
11+
public function indexAction()
12+
{
13+
if (isset($value)) {
14+
return mt_rand(0, 5) ? $this->redirectToRoute('one') : $this->redirectToRoute('two');
15+
}
16+
17+
return [
18+
'key' => 'value'
19+
];
20+
}
21+
}
22+
23+
?>
24+
-----
25+
<?php
26+
27+
namespace Rector\Symfony\Tests\CodeQuality\Rector\ClassMethod\TemplateAnnotationToThisRenderRector\Fixture\Attributes;
28+
29+
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
30+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
31+
32+
final class NestedTernaryRedirectThenArray extends AbstractController
33+
{
34+
public function indexAction(): \Symfony\Component\HttpFoundation\Response
35+
{
36+
if (isset($value)) {
37+
return mt_rand(0, 5) ? $this->redirectToRoute('one') : $this->redirectToRoute('two');
38+
}
39+
return $this->render('with_some_template.twig', [
40+
'key' => 'value'
41+
]);
42+
}
43+
}
44+
45+
?>

‎rules/CodeQuality/Rector/ClassMethod/TemplateAnnotationToThisRenderRector.php

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

55
namespace Rector\Symfony\CodeQuality\Rector\ClassMethod;
66

7-
use PhpParser\Node\Expr\New_;
87
use PhpParser\Node;
98
use PhpParser\Node\Attribute;
109
use PhpParser\Node\Expr;
@@ -29,7 +28,7 @@
2928
use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTagRemover;
3029
use Rector\Comments\NodeDocBlock\DocBlockUpdater;
3130
use Rector\Contract\PhpParser\Node\StmtsAwareInterface;
32-
use Rector\Doctrine\NodeAnalyzer\AttributeFinder;
31+
use Rector\Doctrine\NodeAnalyzer\AttrinationFinder;
3332
use Rector\PhpParser\Node\BetterNodeFinder;
3433
use Rector\Rector\AbstractRector;
3534
use Rector\Symfony\Annotation\AnnotationAnalyzer;
@@ -61,7 +60,7 @@ public function __construct(
6160
private readonly DocBlockUpdater $docBlockUpdater,
6261
private readonly BetterNodeFinder $betterNodeFinder,
6362
private readonly PhpDocInfoFactory $phpDocInfoFactory,
64-
private readonly AttributeFinder $attributeFinder,
63+
private readonly AttrinationFinder $attrinationFinder,
6564
) {
6665
}
6766

@@ -118,21 +117,14 @@ public function refactor(Node $node): ?Node
118117
return null;
119118
}
120119

121-
$this->decorateAbstractControllerParentClass($node);
122-
123120
$hasChanged = false;
124121

125-
$classDoctrineAnnotationTagValueNode = $this->annotationAnalyzer->getDoctrineAnnotationTagValueNode(
126-
$node,
127-
SymfonyAnnotation::TEMPLATE
128-
);
129-
130-
$classTemplateAttribute = $this->attributeFinder->findAttributeByClass($node, SymfonyAnnotation::TEMPLATE);
122+
$classTemplateTagValueNodeOrAttribute = $this->attrinationFinder->getByOne($node, SymfonyAnnotation::TEMPLATE);
131123

132124
foreach ($node->getMethods() as $classMethod) {
133125
$hasClassMethodChanged = $this->replaceTemplateAnnotation(
134126
$classMethod,
135-
$classDoctrineAnnotationTagValueNode ?: $classTemplateAttribute,
127+
$classTemplateTagValueNodeOrAttribute
136128
);
137129

138130
if ($hasClassMethodChanged) {
@@ -144,9 +136,11 @@ public function refactor(Node $node): ?Node
144136
return null;
145137
}
146138

139+
$this->decorateAbstractControllerParentClass($node);
140+
147141
// cleanup Class_ @Template annotation
148-
if ($classDo 325D ctrineAnnotationTagValueNode instanceof DoctrineAnnotationTagValueNode) {
149-
$this->removeDoctrineAnnotationTagValueNode($node, $classDoctrineAnnotationTagValueNode);
142+
if ($classTemplateTagValueNodeOrAttribute instanceof DoctrineAnnotationTagValueNode) {
143+
$this->removeDoctrineAnnotationTagValueNode($node, $classTemplateTagValueNodeOrAttribute);
150144
}
151145

152146
return $node;
@@ -170,18 +164,16 @@ private function replaceTemplateAnnotation(
170164
return false;
171165
}
172166

173-
$doctrineAnnotationTagValueNode = $this->annotationAnalyzer->getDoctrineAnnotationTagValueNode(
167+
$methodTemplateTagValueNodeOrAttribute = $this->attrinationFinder->getByOne(
174168
$classMethod,
175169
SymfonyAnnotation::TEMPLATE
176170
);
177171

178-
$templateAttribute = $this->attributeFinder->findAttributeByClass($classMethod, SymfonyAnnotation::TEMPLATE);
179-
180-
if ($doctrineAnnotationTagValueNode instanceof DoctrineAnnotationTagValueNode || $templateAttribute instanceof Attribute) {
181-
return $this->refactorClassMethod($classMethod, $doctrineAnnotationTagValueNode ?: $templateAttribute);
172+
if ($methodTemplateTagValueNodeOrAttribute !== null) {
173+
return $this->refactorClassMethod($classMethod, $methodTemplateTagValueNodeOrAttribute);
182174
}
183175

184-
// global @Template/#[Template] access
176+
// fallback to global @Template/#[Template] access
185177
if ($classTagValueNodeOrAttribute instanceof DoctrineAnnotationTagValueNode || $classTagValueNodeOrAttribute instanceof Attribute) {
186178
return $this->refactorClassMethod($classMethod, $classTagValueNodeOrAttribute);
187179
}
@@ -312,33 +304,30 @@ private function refactorReturnWithValue(
312304
$lastReturnExpr = $return->expr;
313305

314306
$returnStaticType = $this->getType($lastReturnExpr);
307+
$responseObjectType = new ObjectType(Response::class);
315308

316-
// is new response? keep it
317-
$isResponseType = false;
318-
if ($return->expr instanceof New_) {
319-
$new = $return->expr;
320-
if ($this->isObjectType($new->class, new ObjectType(Response::class))) {
321-
$isResponseType = true;
322-
}
323-
} elseif (! $return->expr instanceof MethodCall) {
324-
if (! $hasThisRenderOrReturnsResponse || $returnStaticType instanceof ConstantArrayType) {
309+
// change contents only if the value is not Response yet
310+
if (! $responseObjectType->isSuperTypeOf($returnStaticType)->yes()) {
311+
if (! $return->expr instanceof MethodCall) {
312+
if (! $hasThisRenderOrReturnsResponse || $returnStaticType instanceof ConstantArrayType) {
313+
$return->expr = $thisRenderMethodCall;
314+
}
315+
} elseif ($returnStaticType instanceof ArrayType) {
325316
$return->expr = $thisRenderMethodCall;
317+
} elseif ($returnStaticType instanceof MixedType) {
318+
// nothing we can do
319+
return false;
326320
}
327-
} elseif ($returnStaticType instanceof ArrayType) {
328-
$return->expr = $thisRenderMethodCall;
329-
} elseif ($returnStaticType instanceof MixedType) {
330-
// nothing we can do
331-
return false;
332-
}
333321

334-
$isArrayOrResponseType = $this->arrayUnionResponseTypeAnalyzer->isArrayUnionResponseType(
335-
$returnStaticType,
336-
SymfonyClass::RESPONSE
337-
);
322+
$isArrayOrResponseType = $this->arrayUnionResponseTypeAnalyzer->isArrayUnionResponseType(
323+
$returnStaticType,
324+
SymfonyClass::RESPONSE
325+
);
338326

339-
// skip as the original class method has to change first
340-
if ($isArrayOrResponseType && $isResponseType === false) {
341-
return false;
327+
// skip as the original class method has to change first
328+
if ($isArrayOrResponseType) {
329+
return false;
330+
}
342331
}
343332

344333
// already response
@@ -391,7 +380,7 @@ private function refactorStmtsAwareNode(
391380
continue;
392381
}
393382

394-
// just created node, skip it
383+
// just created class, skip it
395384
if ($stmt->getAttributes() === []) {
396385
return false;
397386
}

‎src/Annotation/AnnotationAnalyzer.php

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,12 @@
55
namespace Rector\Symfony\Annotation;
66

77
use PhpParser\Node\Stmt\Class_;
8-
use PhpParser\Node\Stmt\ClassMethod;
9-
use Rector\BetterPhpDocParser\PhpDoc\DoctrineAnnotationTagValueNode;
10-
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
11-
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
128
use Rector\Doctrine\NodeAnalyzer\AttrinationFinder;
139
use Rector\Symfony\Enum\SymfonyAnnotation;
1410

1511
final readonly class AnnotationAnalyzer
1612
{
1713
public function __construct(
18-
private PhpDocInfoFactory $phpDocInfoFactory,
1914
private AttrinationFinder $attrinationFinder
2015
) {
2116
}
@@ -34,16 +29,4 @@ public function hasClassMethodWithTemplateAnnotation(Class_ $class): bool
3429

3530
return false;
3631
}
37-
38-
public function getDoctrineAnnotationTagValueNode(
39-
Class_|ClassMethod $node,
40-
string $annotationClass
41-
): ?DoctrineAnnotationTagValueNode {
42-
$phpDocInfo = $this->phpDocInfoFactory->createFromNode($node);
43-
if (! $phpDocInfo instanceof PhpDocInfo) {
44-
return null;
45-
}
46-
47-
return $phpDocInfo->getByAnnotationClass($annotationClass);
48-
}
4932
}

‎src/NodeFactory/Annotations/AnnotationOrAttributeValueResolver.php

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

55
namespace Rector\Symfony\NodeFactory\Annotations;
66

7-
use PhpParser\Node\Identifier;
87
use PhpParser\Node\Arg;
98
use PhpParser\Node\Attribute;
9+
use PhpParser\Node\Identifier;
1010
use PhpParser\Node\Scalar\String_;
1111
use Rector\BetterPhpDocParser\PhpDoc\ArrayItemNode;
1212
use Rector\BetterPhpDocParser\PhpDoc\DoctrineAnnotationTagValueNode;
@@ -65,7 +65,7 @@ public function resolve(
6565

6666
private function isKeyEmptyOrMatch(Arg $attributeArg, string $desiredKey): bool
6767
{
68-
if (!$attributeArg->name instanceof Identifier) {
68+
if (! $attributeArg->name instanceof Identifier) {
6969
return true;
7070
}
7171

0 commit comments

Comments
 (0)
0