RemoveUnusedPrivatePropertyRector should skip entities [closes… (#1923)

RemoveUnusedPrivatePropertyRector should skip entities [closes #1922]

packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php

@@ -61,21 +61,7 @@ CODE_SAMPLE
      */
     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 @@ CODE_SAMPLE
 
         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;
+    }
 }

packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_doctrine_entity_property.php.inc

@@ -0,0 +1,39 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\Property\RemoveUnusedPrivatePropertyRector\Fixture;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity
+ */
+class SkipDoctrineEntityProperty
+{
+    private $unusedProperty;
+
+    /**
+     * @ORM\Column
+     */
+    private $unusedAnnotatedProperty;
+}
+
+?>
+-----
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\Property\RemoveUnusedPrivatePropertyRector\Fixture;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity
+ */
+class SkipDoctrineEntityProperty
+{
+    /**
+     * @ORM\Column
+     */
+    private $unusedAnnotatedProperty;
+}
+
+?>

packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php

@@ -13,6 +13,8 @@ final class RemoveUnusedPrivatePropertyRectorTest extends AbstractRectorTestCase
             __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',

packages/Doctrine/src/AbstarctRector/DoctrineTrait.php

@@ -22,6 +22,11 @@ trait DoctrineTrait
         $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);

packages/Doctrine/src/PhpDocParser/DoctrineDocBlockResolver.php

@@ -72,6 +72,24 @@ final class DoctrineDocBlockResolver
         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) {

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 @@ CODE_SAMPLE
             return null;
         }
 
-        if ($this->isDoctrineEntity($node)) {
+        if ($this->isDoctrineEntityClass($node)) {
             return null;
         }
 
@@ -88,13 +87,4 @@ CODE_SAMPLE
 
         return $node;
     }
-
-    private function isDoctrineEntity(Node $node): bool
-    {
-        if ($node->getDocComment() === null) {
-            return false;
-        }
-
-        return Strings::contains($node->getDocComment()->getText(), 'Entity');
-    }
 }

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
  */