Remove PARENT_NODE from ReadOnlyPropertyRector (#4150)

phpstan.neon

@@ -703,3 +703,4 @@ parameters:
         - '#Cognitive complexity for "Rector\\TypeDeclaration\\Rector\\ClassMethod\\ParamTypeByMethodCallTypeRector\:\:refactorWithScope\(\)" is 13, keep it under 11#'
 
         - '#Function "class_exists\(\)" cannot be used/left in the code\: use ReflectionProvider\->has\*\(\) instead#'
+        - '#Cognitive complexity for "Rector\\Php81\\Rector\\Property\\ReadOnlyPropertyRector\:\:refactorWithScope\(\)" is 12, keep it under 11#'

rules/DeadCode/Rector/If_/RemoveDeadInstanceOfRector.php

@@ -23,7 +23,6 @@
 use Rector\Core\NodeManipulator\IfManipulator;
 use Rector\Core\Rector\AbstractRector;
 use Rector\NodeNestingScope\ContextAnalyzer;
-use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\Php80\NodeAnalyzer\PromotedPropertyResolver;
 use Rector\TypeDeclaration\AlreadyAssignDetector\ConstructorAssignDetector;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -96,11 +95,6 @@ public function refactor(Node $node)
             return null;
         }
 
-        $originalCondNode = $node->cond->getAttribute(AttributeKey::ORIGINAL_NODE);
-        if (! $originalCondNode instanceof Node) {
-            return null;
-        }
-
         if ($node->cond instanceof BooleanNot && $node->cond->expr instanceof Instanceof_) {
             return $this->refactorStmtAndInstanceof($node, $node->cond->expr);
         }

rules/DeadCode/Rector/MethodCall/RemoveEmptyMethodCallRector.php

@@ -15,10 +15,10 @@
 use PhpParser\Node\Stmt\If_;
 use PhpParser\Node\Stmt\Interface_;
 use PhpParser\Node\Stmt\Trait_;
+use PhpParser\NodeTraverser;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Type\ThisType;
 use PHPStan\Type\TypeWithClassName;
-use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\Core\NodeAnalyzer\CallAnalyzer;
 use Rector\Core\PhpParser\AstResolver;
 use Rector\Core\Rector\AbstractRector;
@@ -73,57 +73,39 @@ public function callThis()
      */
     public function getNodeTypes(): array
     {
-        return [StmtsAwareInterface::class];
+        return [If_::class, Expression::class];
     }
 
     /**
-     * @param StmtsAwareInterface $node
+     * @param If_|Expression $node
      */
     public function refactor(Node $node)
     {
-        if ($node->stmts === null) {
-            return null;
+        if ($node instanceof If_) {
+            return $this->refactorIf($node);
         }
 
-        $hasChanged = false;
-
-        foreach ($node->stmts as $key => $stmt) {
-            if ($stmt instanceof If_) {
-                $if = $this->refactorIf($stmt);
-                if ($if instanceof If_) {
-                    $hasChanged = true;
-                    continue;
-                }
+        if ($node->expr instanceof Assign) {
+            $assign = $node->expr;
+            if (! $assign->expr instanceof MethodCall) {
+                return null;
             }
 
-            if (! $stmt instanceof Expression) {
-                continue;
-            }
-
-            if ($stmt->expr instanceof Assign && $stmt->expr->expr instanceof MethodCall) {
-                $methodCall = $stmt->expr->expr;
-                if (! $this->shouldRemoveMethodCall($methodCall)) {
-                    continue;
-                }
-
-                $stmt->expr->expr = $this->nodeFactory->createFalse();
-                $hasChanged = true;
-                continue;
+            if (! $this->shouldRemoveMethodCall($assign->expr)) {
+                return null;
             }
 
-            if ($stmt->expr instanceof MethodCall) {
-                $methodCall = $stmt->expr;
-                if (! $this->shouldRemoveMethodCall($methodCall)) {
-                    continue;
-                }
+            $assign->expr = $this->nodeFactory->createFalse();
+            return $node;
+        }
 
-                unset($node->stmts[$key]);
-                $hasChanged = true;
+        if ($node->expr instanceof MethodCall) {
+            $methodCall = $node->expr;
+            if (! $this->shouldRemoveMethodCall($methodCall)) {
+                return null;
             }
-        }
 
-        if ($hasChanged) {
-            return $node;
+            return NodeTraverser::REMOVE_NODE;
         }
 
         return null;

rules/Php71/Rector/FuncCall/CountOnNullRector.php

@@ -78,14 +78,19 @@ public function getRuleDefinition(): RuleDefinition
      */
     public function getNodeTypes(): array
     {
-        return [FuncCall::class, Ternary::class];
+        return [Trait_::class, FuncCall::class, Ternary::class];
     }
 
     /**
-     * @param FuncCall|Ternary $node
+     * @param Trait_|FuncCall|Ternary $node
      */
     public function refactorWithScope(Node $node, Scope $scope): int|Ternary|null|FuncCall
     {
+        if ($node instanceof Trait_) {
+            // skip contents in traits, as hard to analyze
+            return NodeTraverser::STOP_TRAVERSAL;
+        }
+
         if ($node instanceof Ternary) {
             if ($this->shouldSkipTernaryIfElseCountFuncCall($node)) {
                 return NodeTraverser::DONT_TRAVERSE_CHILDREN;
@@ -160,12 +165,6 @@ private function shouldSkipFuncCall(FuncCall $funcCall): bool
             return true;
         }
 
-        // skip ternary in trait, as impossible to analyse
-        $trait = $this->betterNodeFinder->findParentType($funcCall, Trait_::class);
-        if ($trait instanceof Trait_) {
-            return true;
-        }
-
         $firstArg = $funcCall->getArgs()[0];
 
         // just added node, lets skip it to be sure we're not using mixing

rules/Php81/Rector/Property/ReadOnlyPropertyRector.php

@@ -12,7 +12,6 @@
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\Class_;
-use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Property;
 use PhpParser\NodeTraverser;
@@ -98,6 +97,10 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
     {
         $hasChanged = false;
 
+        if ($node->isReadonly()) {
+            return null;
+        }
+
         // skip "clone $this" cases, as can create unexpected write to local constructor property
 
         if ($this->hasCloneThis($node)) {
@@ -133,16 +136,6 @@ public function provideMinPhpVersion(): int
         return PhpVersionFeature::READONLY_PROPERTY;
     }
 
-    private function shouldSkipInReadonlyClass(Property|Param $node): bool
-    {
-        $class = $this->betterNodeFinder->findParentType($node, Class_::class);
-        if (! $class instanceof Class_) {
-            return true;
-        }
-
-        return $class->isReadonly();
-    }
-
     private function refactorProperty(Class_ $class, Property $property, Scope $scope): ?Property
     {
         // 1. is property read-only?
@@ -174,10 +167,6 @@ private function refactorProperty(Class_ $class, Property $property, Scope $scop
             return null;
         }
 
-        if ($this->shouldSkipInReadonlyClass($property)) {
-            return null;
-        }
-
         $this->visibilityManipulator->makeReadonly($property);
 
         $attributeGroups = $property->attrGroups;
@@ -211,26 +200,17 @@ private function refactorParam(Class_ $class, ClassMethod $classMethod, Param $p
             return null;
         }
 
-        if ($this->isPromotedPropertyAssigned($param)) {
-            return null;
-        }
-
-        if ($this->shouldSkipInReadonlyClass($param)) {
+        if ($this->isPromotedPropertyAssigned($class, $param)) {
             return null;
         }
 
         $this->visibilityManipulator->makeReadonly($param);
         return $param;
     }
 
-    private function isPromotedPropertyAssigned(Param $param): bool
+    private function isPromotedPropertyAssigned(Class_ $class, Param $param): bool
     {
-        $classLike = $this->betterNodeFinder->findParentType($param, ClassLike::class);
-        if (! $classLike instanceof Class_) {
-            return false;
-        }
-
-        $constructClassMethod = $classLike->getMethod(MethodName::CONSTRUCT);
+        $constructClassMethod = $class->getMethod(MethodName::CONSTRUCT);
         if (! $constructClassMethod instanceof ClassMethod) {
             return false;
         }
@@ -241,10 +221,13 @@ private function isPromotedPropertyAssigned(Param $param): bool
 
         $propertyFetch = new PropertyFetch(new Variable('this'), $this->getName($param));
 
-        $stmts = $classLike->stmts;
         $isAssigned = false;
-        $this->traverseNodesWithCallable($stmts, function (Node $node) use ($propertyFetch, &$isAssigned): ?int {
-            if ($node instanceof Assign && $this->nodeComparator->areNodesEqual($propertyFetch, $node->var)) {
+        $this->traverseNodesWithCallable($class->stmts, function (Node $node) use ($propertyFetch, &$isAssigned): ?int {
+            if (! $node instanceof Assign) {
+                return null;
+            }
+
+            if ($this->nodeComparator->areNodesEqual($propertyFetch, $node->var)) {
                 $isAssigned = true;
                 return NodeTraverser::STOP_TRAVERSAL;
             }

rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeBasedOnPHPUnitDataProviderRector.php

@@ -100,48 +100,53 @@ public static function provideData()
      */
     public function getNodeTypes(): array
     {
-        return [ClassMethod::class];
+        return [Class_::class];
     }
 
     /**
-     * @param ClassMethod $node
+     * @param Class_ $node
      */
-    public function refactor(Node $node): ?ClassMethod
+    public function refactor(Node $node)
     {
-        if (! $node->isPublic()) {
-            return null;
-        }
-
-        if ($node->getParams() === []) {
-            return null;
-        }
-
         if (! $this->testsNodeAnalyzer->isInTestClass($node)) {
             return null;
         }
 
-        $dataProviderPhpDocTagNode = $this->resolveDataProviderPhpDocTagNode($node);
-        if (! $dataProviderPhpDocTagNode instanceof PhpDocTagNode) {
-            return null;
-        }
-
         $hasChanged = false;
 
-        foreach ($node->getParams() as $param) {
-            if ($param->type instanceof Node) {
+        foreach ($node->getMethods() as $classMethod) {
+            if (! $classMethod->isPublic()) {
                 continue;
             }
 
-            $paramTypeDeclaration = $this->inferParam($param, $dataProviderPhpDocTagNode);
-            if ($paramTypeDeclaration instanceof MixedType) {
+            if ($classMethod->getParams() === []) {
                 continue;
             }
 
-            $param->type = $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode(
-                $paramTypeDeclaration,
-                TypeKind::PARAM
-            );
-            $hasChanged = true;
+            $dataProviderPhpDocTagNode = $this->resolveDataProviderPhpDocTagNode($classMethod);
+            if (! $dataProviderPhpDocTagNode instanceof PhpDocTagNode) {
+                return null;
+            }
+
+            $hasChanged = false;
+
+            foreach ($classMethod->getParams() as $param) {
+                if ($param->type instanceof Node) {
+                    continue;
+                }
+
+                $paramTypeDeclaration = $this->inferParam($node, $param, $dataProviderPhpDocTagNode);
+                if ($paramTypeDeclaration instanceof MixedType) {
+                    continue;
+                }
+
+                $param->type = $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode(
+                    $paramTypeDeclaration,
+                    TypeKind::PARAM
+                );
+
+                $hasChanged = true;
+            }
         }
 
         if ($hasChanged) {
@@ -151,9 +156,9 @@ public function refactor(Node $node): ?ClassMethod
         return null;
     }
 
-    private function inferParam(Param $param, PhpDocTagNode $dataProviderPhpDocTagNode): Type
+    private function inferParam(Class_ $class, Param $param, PhpDocTagNode $dataProviderPhpDocTagNode): Type
     {
-        $dataProviderClassMethod = $this->resolveDataProviderClassMethod($param, $dataProviderPhpDocTagNode);
+        $dataProviderClassMethod = $this->resolveDataProviderClassMethod($class, $dataProviderPhpDocTagNode);
         if (! $dataProviderClassMethod instanceof ClassMethod) {
             return new MixedType();
         }
@@ -175,14 +180,9 @@ private function inferParam(Param $param, PhpDocTagNode $dataProviderPhpDocTagNo
     }
 
     private function resolveDataProviderClassMethod(
-        Param $param,
+        Class_ $class,
         PhpDocTagNode $dataProviderPhpDocTagNode
     ): ?ClassMethod {
-        $class = $this->betterNodeFinder->findParentType($param, Class_::class);
-        if (! $class instanceof Class_) {
-            return null;
-        }
-
         if (! $dataProviderPhpDocTagNode->value instanceof GenericTagValueNode) {
             return null;
         }