From 3883bbb0018a266a79a28288fb412abed6dda7f5 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 29 Aug 2019 21:22:52 +0200 Subject: [PATCH] RemoveUnusedPrivatePropertyRector should skip entities [closes #1922] --- .../RemoveUnusedPrivatePropertyRector.php | 44 ++++++++++++------- .../skip_doctrine_entity_property.php.inc | 39 ++++++++++++++++ .../RemoveUnusedPrivatePropertyRectorTest.php | 2 + .../src/AbstarctRector/DoctrineTrait.php | 5 +++ .../PhpDocParser/DoctrineDocBlockResolver.php | 18 ++++++++ .../FinalizeClassesWithoutChildrenRector.php | 12 +---- .../Fixture/entity.php.inc | 2 + 7 files changed, 96 insertions(+), 26 deletions(-) create mode 100644 packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_doctrine_entity_property.php.inc diff --git a/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php b/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php index 22d03b3353c2..4cced6fa1d32 100644 --- a/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php +++ b/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php @@ -61,21 +61,7 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if (! $node->isPrivate()) { - return null; - } - - /** @var Class_|Interface_|Trait_|null $classNode */ - $classNode = $node->getAttribute(AttributeKey::CLASS_NODE); - if ($classNode === null || $classNode instanceof Trait_ || $classNode instanceof Interface_) { - return null; - } - - if ($classNode->isAnonymous()) { - return null; - } - - if (count($node->props) !== 1) { + if ($this->shouldSkipProperty($node)) { return null; } @@ -121,4 +107,32 @@ private function resolveUselessAssignNode(array $propertyFetches): array return $uselessAssigns; } + + private function shouldSkipProperty(Property $property): bool + { + if (! $property->isPrivate()) { + return true; + } + + /** @var Class_|Interface_|Trait_|null $classNode */ + $classNode = $property->getAttribute(AttributeKey::CLASS_NODE); + + if ($classNode === null || $classNode instanceof Trait_ || $classNode instanceof Interface_) { + return true; + } + + if ($this->isDoctrineProperty($property)) { + return true; + } + + if ($classNode->isAnonymous()) { + return true; + } + + if (count($property->props) !== 1) { + return true; + } + + return false; + } } diff --git a/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_doctrine_entity_property.php.inc b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_doctrine_entity_property.php.inc new file mode 100644 index 000000000000..d1eea3bac94b --- /dev/null +++ b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_doctrine_entity_property.php.inc @@ -0,0 +1,39 @@ + +----- + diff --git a/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php index a9ee020c9df9..6ac4d7be9689 100644 --- a/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php +++ b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php @@ -13,6 +13,8 @@ public function test(): void __DIR__ . '/Fixture/fixture.php.inc', __DIR__ . '/Fixture/property_assign.php.inc', __DIR__ . '/Fixture/with_trait.php.inc', + // skip + __DIR__ . '/Fixture/skip_doctrine_entity_property.php.inc', __DIR__ . '/Fixture/skip_anonymous_class.php.inc', __DIR__ . '/Fixture/skip_anonymous_function.php.inc', __DIR__ . '/Fixture/skip_nested_closure.php.inc', diff --git a/packages/Doctrine/src/AbstarctRector/DoctrineTrait.php b/packages/Doctrine/src/AbstarctRector/DoctrineTrait.php index d2d02ec40bb4..7f83e65ce301 100644 --- a/packages/Doctrine/src/AbstarctRector/DoctrineTrait.php +++ b/packages/Doctrine/src/AbstarctRector/DoctrineTrait.php @@ -22,6 +22,11 @@ public function autowireDoctrineTrait(DoctrineDocBlockResolver $doctrineDocBlock $this->doctrineDocBlockResolver = $doctrineDocBlockResolver; } + protected function isDoctrineProperty(Property $property): bool + { + return $this->doctrineDocBlockResolver->isDoctrineProperty($property); + } + protected function isDoctrineEntityClass(Class_ $class): bool { return $this->doctrineDocBlockResolver->isDoctrineEntityClass($class); diff --git a/packages/Doctrine/src/PhpDocParser/DoctrineDocBlockResolver.php b/packages/Doctrine/src/PhpDocParser/DoctrineDocBlockResolver.php index 0a56ceae3350..d191ea633c29 100644 --- a/packages/Doctrine/src/PhpDocParser/DoctrineDocBlockResolver.php +++ b/packages/Doctrine/src/PhpDocParser/DoctrineDocBlockResolver.php @@ -72,6 +72,24 @@ public function getDoctrineTableTagValueNode(Class_ $class): ?TableTagValueNode return $classPhpDocInfo->getDoctrineTableTagValueNode(); } + public function isDoctrineProperty(Property $property): bool + { + $propertyPhpDocInfo = $this->getPhpDocInfo($property); + if ($propertyPhpDocInfo === null) { + return false; + } + + if ($propertyPhpDocInfo->getDoctrineColumnTagValueNode()) { + return true; + } + + if ($propertyPhpDocInfo->getDoctrineRelationTagValueNode()) { + return true; + } + + return false; + } + private function getPhpDocInfo(Node $node): ?PhpDocInfo { if ($node->getDocComment() === null) { diff --git a/packages/SOLID/src/Rector/Class_/FinalizeClassesWithoutChildrenRector.php b/packages/SOLID/src/Rector/Class_/FinalizeClassesWithoutChildrenRector.php index acdbc24f44d6..5db9c653f401 100644 --- a/packages/SOLID/src/Rector/Class_/FinalizeClassesWithoutChildrenRector.php +++ b/packages/SOLID/src/Rector/Class_/FinalizeClassesWithoutChildrenRector.php @@ -2,7 +2,6 @@ namespace Rector\SOLID\Rector\Class_; -use Nette\Utils\Strings; use PhpParser\Node; use PhpParser\Node\Stmt\Class_; use Rector\NodeContainer\ParsedNodesByType; @@ -74,7 +73,7 @@ public function refactor(Node $node): ?Node return null; } - if ($this->isDoctrineEntity($node)) { + if ($this->isDoctrineEntityClass($node)) { return null; } @@ -88,13 +87,4 @@ public function refactor(Node $node): ?Node return $node; } - - private function isDoctrineEntity(Node $node): bool - { - if ($node->getDocComment() === null) { - return false; - } - - return Strings::contains($node->getDocComment()->getText(), 'Entity'); - } } diff --git a/packages/SOLID/tests/Rector/Class_/FinalizeClassesWithoutChildrenRector/Fixture/entity.php.inc b/packages/SOLID/tests/Rector/Class_/FinalizeClassesWithoutChildrenRector/Fixture/entity.php.inc index cf09d3d4016d..344dc77458dd 100644 --- a/packages/SOLID/tests/Rector/Class_/FinalizeClassesWithoutChildrenRector/Fixture/entity.php.inc +++ b/packages/SOLID/tests/Rector/Class_/FinalizeClassesWithoutChildrenRector/Fixture/entity.php.inc @@ -2,6 +2,8 @@ namespace Rector\SOLID\Tests\Rector\Class_\FinalizeClassesWithoutChildrenRector\Fixture; +use Doctrine\ORM\Mapping as ORM; + /** * @ORM\Entity */