Cleanup stmts (#4027)

rectorphp · May 30, 2023 · 22e97d1 · 22e97d1
1 parent 953ecb9
commit 22e97d1
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 121 deletions.
diff --git a/...ode/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/array_dim_fetch_use.php.inc b/...ode/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/array_dim_fetch_use.php.inc
diff --git a/...perty/RemoveUnusedPrivatePropertyRector/Fixture/remove_same_line_comment_property.php.inc b/...perty/RemoveUnusedPrivatePropertyRector/Fixture/remove_same_line_comment_property.php.inc
diff --git a/...Property/RemoveUnusedPrivatePropertyRector/FixtureSkipSideEffect/skip_side_effect.php.inc b/...Property/RemoveUnusedPrivatePropertyRector/FixtureSkipSideEffect/skip_side_effect.php.inc
@@ -2,8 +2,6 @@
 
 namespace Rector\Tests\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector\FixtureSkipSideEffect;
 
-use Symplify\PackageBuilder\Testing\AbstractKernelTestCase;
-
 final class SkipSideEffect extends \Rector\Testing\PHPUnit\AbstractTestCase
 {
     private $someService;

diff --git a/rules/DeadCode/Rector/Property/RemoveUnusedPrivatePropertyRector.php b/rules/DeadCode/Rector/Property/RemoveUnusedPrivatePropertyRector.php
@@ -6,7 +6,6 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Stmt\Class_;
-use PhpParser\Node\Stmt\Nop;
 use PhpParser\Node\Stmt\Property;
 use PHPStan\Analyser\Scope;
 use Rector\Core\Contract\Rector\AllowEmptyConfigurableRectorInterface;
@@ -86,9 +85,9 @@ public function getNodeTypes(): array
      */
     public function refactorWithScope(Node $node, Scope $scope): ?Node
     {
-        $hasRemoved = false;
+        $hasChanged = false;
 
-        foreach ($node->stmts as $key => $property) {
+        foreach ($node->stmts as $property) {
             if (! $property instanceof Property) {
                 continue;
             }
@@ -111,29 +110,11 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
             );
 
             if ($isRemoved) {
-                $this->processRemoveSameLineComment($node, $property, $key);
-                $hasRemoved = true;
+                $hasChanged = true;
             }
         }
 
-        return $hasRemoved ? $node : null;
-    }
-
-    private function processRemoveSameLineComment(Class_ $class, Property $property, int $key): void
-    {
-        if (! isset($class->stmts[$key + 1])) {
-            return;
-        }
-
-        if (! $class->stmts[$key + 1] instanceof Nop) {
-            return;
-        }
-
-        if ($class->stmts[$key + 1]->getEndLine() !== $property->getStartLine()) {
-            return;
-        }
-
-        unset($class->stmts[$key + 1]);
+        return $hasChanged ? $node : null;
     }
 
     private function shouldSkipProperty(Property $property): bool

diff --git a/rules/Php81/Rector/Property/ReadOnlyPropertyRector.php b/rules/Php81/Rector/Property/ReadOnlyPropertyRector.php
@@ -87,32 +87,38 @@ public function getName()
      */
     public function getNodeTypes(): array
     {
-        return [Property::class, ClassMethod::class];
+        return [Class_::class];
     }
 
     /**
-     * @param Property|ClassMethod $node
+     * @param Class_ $node
      */
     public function refactorWithScope(Node $node, Scope $scope): ?Node
     {
-        if ($node instanceof ClassMethod) {
-            $hasChanged = false;
-            foreach ($node->params as $param) {
-                $justChanged = $this->refactorParam($node, $param, $scope);
-                // different variable to ensure $hasChanged not replaced
+        $hasChanged = false;
+
+        foreach ($node->getMethods() as $classMethod) {
+            foreach ($classMethod->params as $param) {
+                $justChanged = $this->refactorParam($node, $classMethod, $param, $scope);
+                // different variable to ensure $hasRemoved not replaced
                 if ($justChanged instanceof Param) {
                     $hasChanged = true;
                 }
             }
+        }
 
-            if ($hasChanged) {
-                return $node;
+        foreach ($node->getProperties() as $property) {
+            $changedProperty = $this->refactorProperty($node, $property, $scope);
+            if ($changedProperty instanceof Property) {
+                $hasChanged = true;
             }
+        }
 
-            return null;
+        if ($hasChanged) {
+            return $node;
         }
 
-        return $this->refactorProperty($node, $scope);
+        return null;
     }
 
     public function provideMinPhpVersion(): int
@@ -130,7 +136,7 @@ private function shouldSkipInReadonlyClass(Property|Param $node): bool
         return $class->isReadonly();
     }
 
-    private function refactorProperty(Property $property, Scope $scope): ?Property
+    private function refactorProperty(Class_ $class, Property $property, Scope $scope): ?Property
     {
         // 1. is property read-only?
         if ($property->isReadonly()) {
@@ -153,7 +159,7 @@ private function refactorProperty(Property $property, Scope $scope): ?Property
             return null;
         }
 
-        if ($this->propertyManipulator->isPropertyChangeableExceptConstructor($property, $scope)) {
+        if ($this->propertyManipulator->isPropertyChangeableExceptConstructor($class, $property, $scope)) {
             return null;
         }
 
@@ -175,7 +181,7 @@ private function refactorProperty(Property $property, Scope $scope): ?Property
         return $property;
     }
 
-    private function refactorParam(ClassMethod $classMethod, Param $param, Scope $scope): Param | null
+    private function refactorParam(Class_ $class, ClassMethod $classMethod, Param $param, Scope $scope): Param | null
     {
         if (! $this->visibilityManipulator->hasVisibility($param, Visibility::PRIVATE)) {
             return null;
@@ -186,7 +192,7 @@ private function refactorParam(ClassMethod $classMethod, Param $param, Scope $sc
         }
 
         // promoted property?
-        if ($this->propertyManipulator->isPropertyChangeableExceptConstructor($param, $scope)) {
+        if ($this->propertyManipulator->isPropertyChangeableExceptConstructor($class, $param, $scope)) {
             return null;
         }
 

diff --git a/src/NodeManipulator/PropertyManipulator.php b/src/NodeManipulator/PropertyManipulator.php
@@ -135,24 +135,14 @@ public function isPropertyUsedInReadContext(
         });
     }
 
-    public function isPropertyChangeableExceptConstructor(Property | Param $propertyOrParam, Scope $scope): bool
-    {
-        $class = $this->betterNodeFinder->findParentType($propertyOrParam, Class_::class);
-
-        // does not have parent type ClassLike? Possibly parent is changed by other rule
-        if (! $class instanceof Class_) {
-            return true;
-        }
-
+    public function isPropertyChangeableExceptConstructor(
+        Class_ $class,
+        Property | Param $propertyOrParam,
+        Scope $scope
+    ): bool {
         $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($class);
-        if ($phpDocInfo->hasByAnnotationClasses(self::ALLOWED_NOT_READONLY_ANNOTATION_CLASS_OR_ATTRIBUTES)) {
-            return true;
-        }
 
-        if ($this->phpAttributeAnalyzer->hasPhpAttributes(
-            $class,
-            self::ALLOWED_NOT_READONLY_ANNOTATION_CLASS_OR_ATTRIBUTES
-        )) {
+        if ($this->hasAllowedNotReadonlyAnnotationOrAttribute($phpDocInfo, $class)) {
             return true;
         }
 
@@ -327,4 +317,16 @@ private function isFoundByRefParam(MethodCall | StaticCall $node, Scope $scope):
 
         return false;
     }
+
+    private function hasAllowedNotReadonlyAnnotationOrAttribute(PhpDocInfo $phpDocInfo, Class_ $class): bool
+    {
+        if ($phpDocInfo->hasByAnnotationClasses(self::ALLOWED_NOT_READONLY_ANNOTATION_CLASS_OR_ATTRIBUTES)) {
+            return true;
+        }
+
+        return $this->phpAttributeAnalyzer->hasPhpAttributes(
+            $class,
+            self::ALLOWED_NOT_READONLY_ANNOTATION_CLASS_OR_ATTRIBUTES
+        );
+    }
 }