From 8993b6bc14622f46bd87786fdc5bbe99c4eab11f Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 21 Sep 2019 17:22:02 +0200 Subject: [PATCH 1/2] [Doctrine] Add ChangeReturnTypeOfClassMethodWithGetIdRector --- .../doctrine/doctrine-id-to-uuid-step-3.yaml | 1 + .../Doctrine/DoctrineEntityManipulator.php | 42 +++++++- ...ReturnTypeOfClassMethodWithGetIdRector.php | 102 ++++++++++++++++++ .../ChangeGetUuidMethodCallToGetIdRector.php | 28 +++-- .../ChangeSetIdToUuidValueRector.php | 24 ++--- ...rnTypeOfClassMethodWithGetIdRectorTest.php | 30 ++++++ .../Fixture/fixture.php.inc | 35 ++++++ .../Source/Building.php | 18 ++++ 8 files changed, 250 insertions(+), 30 deletions(-) create mode 100644 packages/Doctrine/src/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector.php create mode 100644 packages/Doctrine/tests/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector/ChangeReturnTypeOfClassMethodWithGetIdRectorTest.php create mode 100644 packages/Doctrine/tests/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector/Fixture/fixture.php.inc create mode 100644 packages/Doctrine/tests/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector/Source/Building.php diff --git a/config/set/doctrine/doctrine-id-to-uuid-step-3.yaml b/config/set/doctrine/doctrine-id-to-uuid-step-3.yaml index 5255742e6c4a..0c59fa79e6b2 100644 --- a/config/set/doctrine/doctrine-id-to-uuid-step-3.yaml +++ b/config/set/doctrine/doctrine-id-to-uuid-step-3.yaml @@ -1,3 +1,4 @@ services: Rector\Doctrine\Rector\MethodCall\ChangeSetIdToUuidValueRector: ~ Rector\Doctrine\Rector\MethodCall\ChangeGetUuidMethodCallToGetIdRector: ~ + Rector\Doctrine\Rector\ClassMethod\ChangeReturnTypeOfClassMethodWithGetIdRector: ~ diff --git a/packages/DeadCode/src/Doctrine/DoctrineEntityManipulator.php b/packages/DeadCode/src/Doctrine/DoctrineEntityManipulator.php index f5b8ff3376f0..8e71590c44a0 100644 --- a/packages/DeadCode/src/Doctrine/DoctrineEntityManipulator.php +++ b/packages/DeadCode/src/Doctrine/DoctrineEntityManipulator.php @@ -3,13 +3,17 @@ namespace Rector\DeadCode\Doctrine; use Doctrine\ORM\Mapping\Entity; +use PhpParser\Node; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\Property; +use PHPStan\Type\ObjectType; +use Rector\Doctrine\PhpDocParser\DoctrineDocBlockResolver; use Rector\DoctrinePhpDocParser\Ast\PhpDoc\Class_\EntityTagValueNode; use Rector\DoctrinePhpDocParser\Ast\PhpDoc\Class_\InheritanceTypeTagValueNode; use Rector\DoctrinePhpDocParser\Contract\Ast\PhpDoc\DoctrineRelationTagValueNodeInterface; use Rector\DoctrinePhpDocParser\Contract\Ast\PhpDoc\InversedByNodeInterface; use Rector\DoctrinePhpDocParser\Contract\Ast\PhpDoc\MappedByNodeInterface; +use Rector\NodeTypeResolver\NodeTypeResolver; use Rector\NodeTypeResolver\PhpDoc\NodeAnalyzer\DocBlockManipulator; use Rector\PhpParser\Node\Resolver\NameResolver; @@ -25,10 +29,26 @@ final class DoctrineEntityManipulator */ private $docBlockManipulator; - public function __construct(NameResolver $nameResolver, DocBlockManipulator $docBlockManipulator) - { + /** + * @var DoctrineDocBlockResolver + */ + private $doctrineDocBlockResolver; + + /** + * @var NodeTypeResolver + */ + private $nodeTypeResolver; + + public function __construct( + NameResolver $nameResolver, + DocBlockManipulator $docBlockManipulator, + DoctrineDocBlockResolver $doctrineDocBlockResolver, + NodeTypeResolver $nodeTypeResolver + ) { $this->nameResolver = $nameResolver; $this->docBlockManipulator = $docBlockManipulator; + $this->doctrineDocBlockResolver = $doctrineDocBlockResolver; + $this->nodeTypeResolver = $nodeTypeResolver; } public function resolveOtherProperty(Property $property): ?string @@ -132,4 +152,22 @@ public function resolveRelationPropertyNames(Class_ $class): array return $manyToOnePropertyNames; } + + public function isMethodCallOnDoctrineEntity(Node $node, string $methodName): bool + { + if (! $node instanceof Node\Expr\MethodCall) { + return false; + } + + if (! $this->nameResolver->isName($node->name, $methodName)) { + return false; + } + + $objectType = $this->nodeTypeResolver->getObjectType($node->var); + if (! $objectType instanceof ObjectType) { + return false; + } + + return $this->doctrineDocBlockResolver->isDoctrineEntityClass($objectType->getClassName()); + } } diff --git a/packages/Doctrine/src/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector.php b/packages/Doctrine/src/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector.php new file mode 100644 index 000000000000..52bb5e3e9e1f --- /dev/null +++ b/packages/Doctrine/src/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector.php @@ -0,0 +1,102 @@ +doctrineEntityManipulator = $doctrineEntityManipulator; + } + + public function getDefinition(): RectorDefinition + { + return new RectorDefinition('Change getUuid() method call to getId()', [ + new CodeSample( + <<<'PHP' +class SomeClass +{ + public function getBuildingId(): int + { + $building = new Building(); + + return $building->getId(); + } +} +PHP + , + <<<'PHP' +class SomeClass +{ + public function getBuildingId(): \Ramsey\Uuid\UuidInterface + { + $building = new Building(); + + return $building->getId(); + } +} +PHP + ), + ]); + } + + /** + * @return string[] + */ + public function getNodeTypes(): array + { + return [ClassMethod::class]; + } + + /** + * @param ClassMethod $node + */ + public function refactor(Node $node): ?Node + { + if ($node->returnType === null) { + return null; + } + + $hasEntityGetIdMethodCall = $this->hasEntityGetIdMethodCall($node); + if ($hasEntityGetIdMethodCall === false) { + return null; + } + + $node->returnType = new FullyQualified(DoctrineClass::RAMSEY_UUID_INTERFACE); + + return $node; + } + + private function hasEntityGetIdMethodCall(Node $node): bool + { + return (bool) $this->betterNodeFinder->findFirst((array) $node->stmts, function (Node $node): bool { + if (! $node instanceof Return_) { + return false; + } + + if ($node->expr === null) { + return false; + } + + return $this->doctrineEntityManipulator->isMethodCallOnDoctrineEntity($node->expr, 'getId'); + }); + } +} diff --git a/packages/Doctrine/src/Rector/MethodCall/ChangeGetUuidMethodCallToGetIdRector.php b/packages/Doctrine/src/Rector/MethodCall/ChangeGetUuidMethodCallToGetIdRector.php index 015c381e5a05..8b181f3e60e2 100644 --- a/packages/Doctrine/src/Rector/MethodCall/ChangeGetUuidMethodCallToGetIdRector.php +++ b/packages/Doctrine/src/Rector/MethodCall/ChangeGetUuidMethodCallToGetIdRector.php @@ -5,7 +5,7 @@ use PhpParser\Node; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Identifier; -use PHPStan\Type\ObjectType; +use Rector\DeadCode\Doctrine\DoctrineEntityManipulator; use Rector\Rector\AbstractRector; use Rector\RectorDefinition\CodeSample; use Rector\RectorDefinition\RectorDefinition; @@ -15,6 +15,16 @@ */ final class ChangeGetUuidMethodCallToGetIdRector extends AbstractRector { + /** + * @var DoctrineEntityManipulator + */ + private $doctrineEntityManipulator; + + public function __construct(DoctrineEntityManipulator $doctrineEntityManipulator) + { + $this->doctrineEntityManipulator = $doctrineEntityManipulator; + } + public function getDefinition(): RectorDefinition { return new RectorDefinition('Change getUuid() method call to getId()', [ @@ -91,7 +101,7 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if ($this->shouldSkip($node)) { + if (! $this->doctrineEntityManipulator->isMethodCallOnDoctrineEntity($node, 'getUuid')) { return null; } @@ -99,18 +109,4 @@ public function refactor(Node $node): ?Node return $node; } - - private function shouldSkip(Node\Expr\MethodCall $methodCall): bool - { - if (! $this->isName($methodCall->name, 'getUuid')) { - return true; - } - - $methodVarObjectType = $this->getObjectType($methodCall->var); - if (! $methodVarObjectType instanceof ObjectType) { - return true; - } - - return ! $this->isDoctrineEntityClass($methodVarObjectType->getClassName()); - } } diff --git a/packages/Doctrine/src/Rector/MethodCall/ChangeSetIdToUuidValueRector.php b/packages/Doctrine/src/Rector/MethodCall/ChangeSetIdToUuidValueRector.php index 7a53c8248b21..62d63f388de5 100644 --- a/packages/Doctrine/src/Rector/MethodCall/ChangeSetIdToUuidValueRector.php +++ b/packages/Doctrine/src/Rector/MethodCall/ChangeSetIdToUuidValueRector.php @@ -11,6 +11,7 @@ use PHPStan\Type\ObjectType; use PHPStan\Type\StringType; use Ramsey\Uuid\Uuid; +use Rector\DeadCode\Doctrine\DoctrineEntityManipulator; use Rector\Doctrine\ValueObject\DoctrineClass; use Rector\NodeContainer\ParsedNodesByType; use Rector\NodeTypeResolver\Node\AttributeKey; @@ -30,9 +31,17 @@ final class ChangeSetIdToUuidValueRector extends AbstractRector */ private $parsedNodesByType; - public function __construct(ParsedNodesByType $parsedNodesByType) - { + /** + * @var DoctrineEntityManipulator + */ + private $doctrineEntityManipulator; + + public function __construct( + ParsedNodesByType $parsedNodesByType, + DoctrineEntityManipulator $doctrineEntityManipulator + ) { $this->parsedNodesByType = $parsedNodesByType; + $this->doctrineEntityManipulator = $doctrineEntityManipulator; } public function getDefinition(): RectorDefinition @@ -142,16 +151,7 @@ public function refactor(Node $node): ?Node private function shouldSkip(MethodCall $methodCall): bool { - if (! $this->isName($methodCall->name, 'setId')) { - return true; - } - - $objectType = $this->getObjectType($methodCall); - if (! $objectType instanceof ObjectType) { - return true; - } - - if (! $this->isDoctrineEntityClass($objectType->getClassName())) { + if (! $this->doctrineEntityManipulator->isMethodCallOnDoctrineEntity($methodCall, 'setId')) { return true; } diff --git a/packages/Doctrine/tests/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector/ChangeReturnTypeOfClassMethodWithGetIdRectorTest.php b/packages/Doctrine/tests/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector/ChangeReturnTypeOfClassMethodWithGetIdRectorTest.php new file mode 100644 index 000000000000..aa8c2e927dd4 --- /dev/null +++ b/packages/Doctrine/tests/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector/ChangeReturnTypeOfClassMethodWithGetIdRectorTest.php @@ -0,0 +1,30 @@ +doTestFile($file); + } + + /** + * @return string[] + */ + public function provideDataForTest(): iterable + { + yield [__DIR__ . '/Fixture/fixture.php.inc']; + } + + protected function getRectorClass(): string + { + return ChangeReturnTypeOfClassMethodWithGetIdRector::class; + } +} diff --git a/packages/Doctrine/tests/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector/Fixture/fixture.php.inc b/packages/Doctrine/tests/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector/Fixture/fixture.php.inc new file mode 100644 index 000000000000..fff230edce0c --- /dev/null +++ b/packages/Doctrine/tests/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector/Fixture/fixture.php.inc @@ -0,0 +1,35 @@ +getId(); + } +} + +?> +----- +getId(); + } +} + +?> diff --git a/packages/Doctrine/tests/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector/Source/Building.php b/packages/Doctrine/tests/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector/Source/Building.php new file mode 100644 index 000000000000..add6f1df9674 --- /dev/null +++ b/packages/Doctrine/tests/Rector/ClassMethod/ChangeReturnTypeOfClassMethodWithGetIdRector/Source/Building.php @@ -0,0 +1,18 @@ +id; + } +} From d727b0181ef72abc25a58eee5358e27765de5792 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 21 Sep 2019 19:23:55 +0200 Subject: [PATCH 2/2] [Doctrine] Add ChangeIdenticalUuidToEqualsMethodCallRector --- .../doctrine/doctrine-id-to-uuid-step-3.yaml | 1 + .../Doctrine/DoctrineEntityManipulator.php | 3 +- ...eIdenticalUuidToEqualsMethodCallRector.php | 119 ++++++++++++++++++ ...nticalUuidToEqualsMethodCallRectorTest.php | 30 +++++ .../Fixture/fixture.php.inc | 37 ++++++ .../Source/Building.php | 13 ++ tests/bootstrap.php | 4 +- 7 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 packages/Doctrine/src/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector.php create mode 100644 packages/Doctrine/tests/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector/ChangeIdenticalUuidToEqualsMethodCallRectorTest.php create mode 100644 packages/Doctrine/tests/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector/Fixture/fixture.php.inc create mode 100644 packages/Doctrine/tests/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector/Source/Building.php diff --git a/config/set/doctrine/doctrine-id-to-uuid-step-3.yaml b/config/set/doctrine/doctrine-id-to-uuid-step-3.yaml index 0c59fa79e6b2..967d2c891ca6 100644 --- a/config/set/doctrine/doctrine-id-to-uuid-step-3.yaml +++ b/config/set/doctrine/doctrine-id-to-uuid-step-3.yaml @@ -2,3 +2,4 @@ services: Rector\Doctrine\Rector\MethodCall\ChangeSetIdToUuidValueRector: ~ Rector\Doctrine\Rector\MethodCall\ChangeGetUuidMethodCallToGetIdRector: ~ Rector\Doctrine\Rector\ClassMethod\ChangeReturnTypeOfClassMethodWithGetIdRector: ~ + Rector\Doctrine\Rector\Identical\ChangeIdenticalUuidToEqualsMethodCallRector: ~ diff --git a/packages/DeadCode/src/Doctrine/DoctrineEntityManipulator.php b/packages/DeadCode/src/Doctrine/DoctrineEntityManipulator.php index 8e71590c44a0..146127251468 100644 --- a/packages/DeadCode/src/Doctrine/DoctrineEntityManipulator.php +++ b/packages/DeadCode/src/Doctrine/DoctrineEntityManipulator.php @@ -4,6 +4,7 @@ use Doctrine\ORM\Mapping\Entity; use PhpParser\Node; +use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\Property; use PHPStan\Type\ObjectType; @@ -155,7 +156,7 @@ public function resolveRelationPropertyNames(Class_ $class): array public function isMethodCallOnDoctrineEntity(Node $node, string $methodName): bool { - if (! $node instanceof Node\Expr\MethodCall) { + if (! $node instanceof MethodCall) { return false; } diff --git a/packages/Doctrine/src/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector.php b/packages/Doctrine/src/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector.php new file mode 100644 index 000000000000..518ca08e5b71 --- /dev/null +++ b/packages/Doctrine/src/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector.php @@ -0,0 +1,119 @@ +doctrineEntityManipulator = $doctrineEntityManipulator; + } + + public function getDefinition(): RectorDefinition + { + return new RectorDefinition('Change $uuid === 1 to $uuid->equals(\Ramsey\Uuid\Uuid::fromString(1))', [ + new CodeSample( + <<<'PHP' +class SomeClass +{ + public function match($checkedId): int + { + $building = new Building(); + + return $building->getId() === $checkedId; + } +} +PHP + , + <<<'PHP' +class SomeClass +{ + public function match($checkedId): int + { + $building = new Building(); + + return $building->getId()->equals(\Ramsey\Uuid\Uuid::fromString($checkedId)); + } +} +PHP + ), + ]); + } + + /** + * @return string[] + */ + public function getNodeTypes(): array + { + return [Identical::class]; + } + + /** + * @param Identical $node + */ + public function refactor(Node $node): ?Node + { + $match = $this->matchEntityCallAndComparedVariable($node); + if ($match === null) { + return null; + } + + [$entityMethodCall, $comparedVariable] = $match; + + $fromStringValue = $this->createStaticCall(DoctrineClass::RAMSEY_UUID, 'fromString', [$comparedVariable]); + + return $this->createMethodCall($entityMethodCall, 'equals', [$fromStringValue]); + } + + private function isAlreadyUuidType(Expr $expr): bool + { + $comparedValueObjectType = $this->getStaticType($expr); + if (! $comparedValueObjectType instanceof ObjectType) { + return false; + } + + return $comparedValueObjectType->getClassName() === DoctrineClass::RAMSEY_UUID_INTERFACE; + } + + /** + * @return Expr[]|null + */ + private function matchEntityCallAndComparedVariable(Node $node): ?array + { + if ($this->doctrineEntityManipulator->isMethodCallOnDoctrineEntity($node->left, 'getId')) { + if ($this->isAlreadyUuidType($node->right)) { + return null; + } + + return [$node->left, $node->right]; + } + + if ($this->doctrineEntityManipulator->isMethodCallOnDoctrineEntity($node->right, 'getId')) { + if ($this->isAlreadyUuidType($node->left)) { + return null; + } + + return [$node->right, $node->left]; + } + + return null; + } +} diff --git a/packages/Doctrine/tests/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector/ChangeIdenticalUuidToEqualsMethodCallRectorTest.php b/packages/Doctrine/tests/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector/ChangeIdenticalUuidToEqualsMethodCallRectorTest.php new file mode 100644 index 000000000000..f5034bb8beea --- /dev/null +++ b/packages/Doctrine/tests/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector/ChangeIdenticalUuidToEqualsMethodCallRectorTest.php @@ -0,0 +1,30 @@ +doTestFile($file); + } + + /** + * @return string[] + */ + public function provideDataForTest(): iterable + { + yield [__DIR__ . '/Fixture/fixture.php.inc']; + } + + protected function getRectorClass(): string + { + return ChangeIdenticalUuidToEqualsMethodCallRector::class; + } +} diff --git a/packages/Doctrine/tests/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector/Fixture/fixture.php.inc b/packages/Doctrine/tests/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector/Fixture/fixture.php.inc new file mode 100644 index 000000000000..1ca5219c0907 --- /dev/null +++ b/packages/Doctrine/tests/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector/Fixture/fixture.php.inc @@ -0,0 +1,37 @@ +getId() === $checkedId; + $isRight = $checkedId === $building->getId(); + } +} + +?> +----- +getId()->equals(\Ramsey\Uuid\Uuid::fromString($checkedId)); + $isRight = $building->getId()->equals(\Ramsey\Uuid\Uuid::fromString($checkedId)); + } +} + +?> diff --git a/packages/Doctrine/tests/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector/Source/Building.php b/packages/Doctrine/tests/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector/Source/Building.php new file mode 100644 index 000000000000..8e2242f29a93 --- /dev/null +++ b/packages/Doctrine/tests/Rector/Identical/ChangeIdenticalUuidToEqualsMethodCallRector/Source/Building.php @@ -0,0 +1,13 @@ +