Simplify PropertyFetchByMethodAnalyzer

.github/workflows/rector.yaml

@@ -25,10 +25,8 @@ jobs:
             fail-fast: false
             matrix:
                 directories:
-                    - src tests
-                    - packages packages-tests
+                    - src tests rules-tests packages packages-tests
                     - rules
-                    - rules-tests
 
         runs-on: ubuntu-latest
         steps:

phpstan.neon

@@ -527,8 +527,4 @@ parameters:
             message: '#\$this as argument is not allowed\. Refactor method to service composition#'
             path: rules/TypeDeclaration/Rector/ClassMethod/ParamTypeFromStrictTypedPropertyRector.php
 
-        -
-            message: '#Class cognitive complexity is 32, keep it under 30#'
-            path: rules/Privatization/Rector/Class_/ChangeLocalPropertyToVariableRector.php
-
         - '#Method Rector\\Core\\PhpParser\\Node\\BetterNodeFinder\:\:findParentTypes\(\) should return T of PhpParser\\Node\|null but returns class\-string<T of PhpParser\\Node\>\|T of PhpParser\\Node#'

rules/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector.php

@@ -262,6 +262,7 @@ private function isLocalPropertyOfNamesNotIdenticalToNull(Expr $expr, array $pro
         ) && $this->valueResolver->isNull($expr->right)) {
             return true;
         }
+
         if (! $this->propertyFetchAnalyzer->isLocalPropertyOfNames($expr->right, $propertyNames)) {
             return false;
         }

rules/DeadCode/Rector/ClassLike/RemoveAnnotationRector.php

@@ -11,6 +11,7 @@
 use PhpParser\Node\Stmt\Property;
 use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagValueNode;
 use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTagRemover;
+use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
 use Rector\Core\Contract\Rector\ConfigurableRectorInterface;
 use Rector\Core\Rector\AbstractRector;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
@@ -96,6 +97,7 @@ public function refactor(Node $node): ?Node
         }
 
         if ($phpDocInfo->hasChanged()) {
+            $this->file->addRectorClassWithLine(new RectorWithLineChange($this, $node->getLine()));
             return null;
         }
 

rules/DeadCode/Rector/ClassMethod/RemoveUselessParamTagRector.php

@@ -7,6 +7,7 @@
 use PhpParser\Node;
 use PhpParser\Node\Stmt\ClassMethod;
 use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTagRemover;
+use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
 use Rector\Core\Rector\AbstractRector;
 use Rector\DeadCode\PhpDoc\DeadParamTagValueNodeAnalyzer;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -93,6 +94,7 @@ public function refactor(Node $node): ?Node
         }
 
         if ($phpDocInfo->hasChanged()) {
+            $this->file->addRectorClassWithLine(new RectorWithLineChange($this, $node->getLine()));
             return $node;
         }
 

rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnTagRector.php

@@ -6,6 +6,7 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Stmt\ClassMethod;
+use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
 use Rector\Core\Rector\AbstractRector;
 use Rector\DeadCode\PhpDoc\TagRemover\ReturnTagRemover;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -77,6 +78,7 @@ public function refactor(Node $node): ?Node
         $this->returnTagRemover->removeReturnTagIfUseless($phpDocInfo, $node);
 
         if ($phpDocInfo->hasChanged()) {
+            $this->file->addRectorClassWithLine(new RectorWithLineChange($this, $node->getLine()));
             return $node;
         }
 

rules/DeadCode/Rector/Property/RemoveUselessVarTagRector.php

@@ -6,6 +6,7 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Stmt\Property;
+use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
 use Rector\Core\Rector\AbstractRector;
 use Rector\DeadCode\PhpDoc\TagRemover\VarTagRemover;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -69,6 +70,7 @@ public function refactor(Node $node): ?Node
         $this->varTagRemover->removeVarTagIfUseless($phpDocInfo, $node);
 
         if ($phpDocInfo->hasChanged()) {
+            $this->file->addRectorClassWithLine(new RectorWithLineChange($this, $node->getLine()));
             return $node;
         }
 

rules/Privatization/NodeAnalyzer/PropertyFetchByMethodAnalyzer.php

@@ -0,0 +1,192 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Privatization\NodeAnalyzer;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Stmt;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Do_;
+use PhpParser\Node\Stmt\Else_;
+use PhpParser\Node\Stmt\If_;
+use PhpParser\Node\Stmt\While_;
+use PhpParser\NodeTraverser;
+use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
+use Rector\Core\ValueObject\MethodName;
+use Rector\NodeNameResolver\NodeNameResolver;
+use Rector\NodeTypeResolver\Node\AttributeKey;
+use Symplify\Astral\NodeTraverser\SimpleCallableNodeTraverser;
+
+final class PropertyFetchByMethodAnalyzer
+{
+    /**
+     * @var array<class-string<Stmt>>
+     */
+    private const SCOPE_CHANGING_NODE_TYPES = [Do_::class, While_::class, If_::class, Else_::class];
+
+    /**
+     * @var NodeNameResolver
+     */
+    private $nodeNameResolver;
+
+    /**
+     * @var PropertyFetchAnalyzer
+     */
+    private $propertyFetchAnalyzer;
+
+    /**
+     * @var SimpleCallableNodeTraverser
+     */
+    private $simpleCallableNodeTraverser;
+
+    public function __construct(
+        NodeNameResolver $nodeNameResolver,
+        PropertyFetchAnalyzer $propertyFetchAnalyzer,
+        SimpleCallableNodeTraverser $simpleCallableNodeTraverser
+    ) {
+        $this->nodeNameResolver = $nodeNameResolver;
+        $this->propertyFetchAnalyzer = $propertyFetchAnalyzer;
+        $this->simpleCallableNodeTraverser = $simpleCallableNodeTraverser;
+    }
+
+    /**
+     * @param string[] $propertyNames
+     * @return array<string, string[]>
+     */
+    public function collectPropertyFetchByMethods(Class_ $class, array $propertyNames): array
+    {
+        $propertyUsageByMethods = [];
+
+        foreach ($propertyNames as $propertyName) {
+            foreach ($class->getMethods() as $classMethod) {
+                // assigned in constructor injection → skip
+                if ($this->nodeNameResolver->isName($classMethod, MethodName::CONSTRUCT)) {
+                    return [];
+                }
+
+                if (! $this->propertyFetchAnalyzer->containsLocalPropertyFetchName($classMethod, $propertyName)) {
+                    continue;
+                }
+
+                if ($this->isPropertyChangingInMultipleMethodCalls($classMethod, $propertyName)) {
+                    continue;
+                }
+
+                $classMethodName = $this->nodeNameResolver->getName($classMethod);
+                $propertyUsageByMethods[$propertyName][] = $classMethodName;
+            }
+        }
+
+        return $propertyUsageByMethods;
+    }
+
+    /**
+     * Covers https://github.com/rectorphp/rector/pull/2558#discussion_r363036110
+     */
+    private function isPropertyChangingInMultipleMethodCalls(ClassMethod $classMethod, string $propertyName): bool
+    {
+        $isPropertyChanging = false;
+        $isPropertyReadInIf = false;
+        $isIfFollowedByAssign = false;
+
+        $this->simpleCallableNodeTraverser->traverseNodesWithCallable(
+            (array) $classMethod->getStmts(),
+            function (Node $node) use (
+            &$isPropertyChanging,
+            $propertyName,
+            &$isPropertyReadInIf,
+            &$isIfFollowedByAssign
+        ): ?int {
+                if ($isPropertyReadInIf) {
+                    if (! $this->propertyFetchAnalyzer->isLocalPropertyOfNames($node, [$propertyName])) {
+                        return null;
+                    }
+
+                    $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
+                    if ($parentNode instanceof Assign && $parentNode->var === $node) {
+                        $isIfFollowedByAssign = true;
+                    }
+                }
+
+                if (! $this->isScopeChangingNode($node)) {
+                    return null;
+                }
+
+                if ($node instanceof If_) {
+                    $isPropertyReadInIf = $this->refactorIf($node, $propertyName);
+                }
+
+                $isPropertyChanging = $this->isPropertyChanging($node, $propertyName);
+                if (! $isPropertyChanging) {
+                    return null;
+                }
+
+                return NodeTraverser::STOP_TRAVERSAL;
+            }
+        );
+
+        return $isPropertyChanging || $isIfFollowedByAssign || $isPropertyReadInIf;
+    }
+
+    private function isScopeChangingNode(Node $node): bool
+    {
+        foreach (self::SCOPE_CHANGING_NODE_TYPES as $type) {
+            if (is_a($node, $type, true)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    private function refactorIf(If_ $if, string $privatePropertyName): ?bool
+    {
+        $this->simpleCallableNodeTraverser->traverseNodesWithCallable($if->cond, function (Node $node) use (
+            $privatePropertyName,
+            &$isPropertyReadInIf
+        ): ?int {
+            if (! $this->propertyFetchAnalyzer->isLocalPropertyOfNames($node, [$privatePropertyName])) {
+                return null;
+            }
+
+            $isPropertyReadInIf = true;
+
+            return NodeTraverser::STOP_TRAVERSAL;
+        });
+
+        return $isPropertyReadInIf;
+    }
+
+    private function isPropertyChanging(Node $node, string $privatePropertyName): bool
+    {
+        $isPropertyChanging = false;
+        // here cannot be any property assign
+
+        $this->simpleCallableNodeTraverser->traverseNodesWithCallable($node, function (Node $node) use (
+            &$isPropertyChanging,
+            $privatePropertyName
+        ): ?int {
+            if (! $node instanceof Assign) {
+                return null;
+            }
+
+            if (! $node->var instanceof PropertyFetch) {
+                return null;
+            }
+
+            if (! $this->nodeNameResolver->isName($node->var->name, $privatePropertyName)) {
+                return null;
+            }
+
+            $isPropertyChanging = true;
+
+            return NodeTraverser::STOP_TRAVERSAL;
+        });
+
+        return $isPropertyChanging;
+    }
+}