[NodeAnalyzer] Check property fetch for read (#4431)

* [NodeAnalyzer] Check property fetch for read

* [ReadWrite] Decopule new package for read/write/only analysis

* add property fetch

composer.json

@@ -90,6 +90,7 @@
             "Rector\\DowngradePhp71\\": "rules/downgrade-php71/src",
             "Rector\\DowngradePhp72\\": "rules/downgrade-php72/src",
             "Rector\\DowngradePhp73\\": "rules/downgrade-php73/src",
+            "Rector\\ReadWrite\\": "packages/read-write/src",
             "Rector\\DowngradePhp74\\": "rules/downgrade-php74/src",
             "Rector\\DowngradePhp80\\": "rules/downgrade-php80/src",
             "Rector\\DynamicTypeAnalysis\\": "packages/dynamic-type-analysis/src",

packages/node-collector/src/NodeCollector/NodeRepository.php

@@ -33,6 +33,7 @@ use Rector\NodeCollector\ValueObject\ArrayCallable;
 use Rector\NodeNameResolver\NodeNameResolver;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\NodeTypeResolver\NodeTypeResolver;
+use Rector\PHPStan\Type\ShortenedObjectType;
 use Rector\PHPStanStaticTypeMapper\Utils\TypeUnwrapper;
 
 /**
@@ -297,6 +298,28 @@ final class NodeRepository
         return $this->parsedPropertyFetchNodeCollector->findPropertyFetchesByTypeAndName($className, $propertyName);
     }
 
+    /**
+     * @return PropertyFetch[]
+     */
+    public function findPropertyFetchesByPropertyFetch(PropertyFetch $propertyFetch): array
+    {
+        $propertyFetcheeType = $this->nodeTypeResolver->getStaticType($propertyFetch->var);
+        if (! $propertyFetcheeType instanceof TypeWithClassName) {
+            return [];
+        }
+
+        if ($propertyFetcheeType instanceof ShortenedObjectType) {
+            $className = $propertyFetcheeType->getFullyQualifiedName();
+        } else {
+            $className = $propertyFetcheeType->getClassName();
+        }
+
+        /** @var string $propertyName */
+        $propertyName = $this->nodeNameResolver->getName($propertyFetch);
+
+        return $this->parsedPropertyFetchNodeCollector->findPropertyFetchesByTypeAndName($className, $propertyName);
+    }
+
     /**
      * @return MethodCall[]|StaticCall[]|ArrayCallable[]
      */

packages/read-write/config/config.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+
+return static function (ContainerConfigurator $containerConfigurator): void {
+    $services = $containerConfigurator->services();
+
+    $services->defaults()
+        ->public()
+        ->autowire()
+        ->autoconfigure();
+
+    $services->load('Rector\ReadWrite\\', __DIR__ . '/../src');
+};

packages/read-write/src/Contract/ReadNodeAnalyzerInterface.php

@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\ReadWrite\Contract;
+
+use PhpParser\Node;
+
+interface ReadNodeAnalyzerInterface
+{
+    public function supports(Node $node): bool;
+
+    public function isRead(Node $node): bool;
+}

packages/read-write/src/NodeAnalyzer/ReadExprAnalyzer.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\ReadWrite\NodeAnalyzer;
+
+use PhpParser\Node\Expr;
+use Rector\Core\Exception\NotImplementedYetException;
+use Rector\ReadWrite\Contract\ReadNodeAnalyzerInterface;
+
+final class ReadExprAnalyzer
+{
+    /**
+     * @var ReadNodeAnalyzerInterface[]
+     */
+    private $readNodeAnalyzers = [];
+
+    /**
+     * @param ReadNodeAnalyzerInterface[] $readNodeAnalyzers
+     */
+    public function __construct(array $readNodeAnalyzers)
+    {
+        $this->readNodeAnalyzers = $readNodeAnalyzers;
+    }
+
+    /**
+     * Is the value read or used for read purpose (at least, not only)
+     */
+    public function isExprRead(Expr $expr): bool
+    {
+        foreach ($this->readNodeAnalyzers as $readNodeAnalyzer) {
+            if (! $readNodeAnalyzer->supports($expr)) {
+                continue;
+            }
+
+            return $readNodeAnalyzer->isRead($expr);
+        }
+
+        throw new NotImplementedYetException(get_class($expr));
+    }
+}

packages/read-write/src/NodeAnalyzer/ReadWritePropertyAnalyzer.php

@@ -2,7 +2,7 @@
 
 declare(strict_types=1);
 
-namespace Rector\Core\NodeAnalyzer;
+namespace Rector\ReadWrite\NodeAnalyzer;
 
 use PhpParser\Node;
 use PhpParser\Node\Arg;

packages/read-write/src/ReadNodeAnalyzer/AbstractReadNodeAnalyzer.php

@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\ReadWrite\ReadNodeAnalyzer;
+
+use PhpParser\Node\Expr;
+use PhpParser\Node\Stmt\Return_;
+use Rector\Core\Exception\NotImplementedYetException;
+use Rector\Core\NodeFinder\NodeUsageFinder;
+use Rector\NodeNestingScope\ParentScopeFinder;
+use Rector\NodeTypeResolver\Node\AttributeKey;
+
+abstract class AbstractReadNodeAnalyzer
+{
+    /**
+     * @var ParentScopeFinder
+     */
+    protected $parentScopeFinder;
+
+    /**
+     * @var NodeUsageFinder
+     */
+    protected $nodeUsageFinder;
+
+    /**
+     * @required
+     */
+    public function autowireAbstractReadNodeAnalyzer(
+        ParentScopeFinder $parentScopeFinder,
+        NodeUsageFinder $nodeUsageFinder
+    ): void {
+        $this->parentScopeFinder = $parentScopeFinder;
+        $this->nodeUsageFinder = $nodeUsageFinder;
+    }
+
+    protected function isCurrentContextRead(Expr $expr): bool
+    {
+        $parent = $expr->getAttribute(AttributeKey::PARENT_NODE);
+        if ($parent instanceof Return_) {
+            return true;
+        }
+
+        throw new NotImplementedYetException();
+    }
+}

packages/read-write/src/ReadNodeAnalyzer/PropertyFetchReadNodeAnalyzer.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\ReadWrite\ReadNodeAnalyzer;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\PropertyFetch;
+use Rector\ReadWrite\Contract\ReadNodeAnalyzerInterface;
+
+final class PropertyFetchReadNodeAnalyzer extends AbstractReadNodeAnalyzer implements ReadNodeAnalyzerInterface
+{
+    public function supports(Node $node): bool
+    {
+        return $node instanceof PropertyFetch;
+    }
+
+    /**
+     * @param PropertyFetch $node
+     */
+    public function isRead(Node $node): bool
+    {
+        $propertyFetchUsages = $this->nodeUsageFinder->findPropertyFetchUsages($node);
+        foreach ($propertyFetchUsages as $propertyFetchUsage) {
+            if ($this->isCurrentContextRead($propertyFetchUsage)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+}

packages/read-write/src/ReadNodeAnalyzer/VariableReadNodeAnalyzer.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\ReadWrite\ReadNodeAnalyzer;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\Variable;
+use Rector\ReadWrite\Contract\ReadNodeAnalyzerInterface;
+
+final class VariableReadNodeAnalyzer extends AbstractReadNodeAnalyzer implements ReadNodeAnalyzerInterface
+{
+    public function supports(Node $node): bool
+    {
+        return $node instanceof Variable;
+    }
+
+    /**
+     * @param Variable $node
+     */
+    public function isRead(Node $node): bool
+    {
+        $parentScope = $this->parentScopeFinder->find($node);
+        if ($parentScope === null) {
+            return false;
+        }
+
+        $variableUsages = $this->nodeUsageFinder->findVariableUsages((array) $parentScope->stmts, $node);
+        foreach ($variableUsages as $variableUsage) {
+            if ($this->isCurrentContextRead($variableUsage)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+}

rules/dead-code/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_dim_fetch_use_property_fetch.php.inc

@@ -0,0 +1,25 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\Property\RemoveUnusedPrivatePropertyRector\Fixture;
+
+final class SkipDimFetchUsePropertyFetch
+{
+    /**
+     * @var string
+     */
+    private $key;
+
+    protected $writeOnly;
+
+    public function __construct(string $key)
+    {
+        $this->key = $key;
+    }
+
+    public function buildData(): array
+    {
+        $this->writeOnly[$this->key] = 10000;
+
+        return $this->writeOnly;
+    }
+}

rules/dead-code/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/write_only_dim_fetch_use_variable.php.inc

@@ -0,0 +1,44 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\Property\RemoveUnusedPrivatePropertyRector\Fixture;
+
+final class WriteOnlyDimFetchUseVariable
+{
+    /**
+     * @var string
+     */
+    private $key;
+
+    protected $writeOnly;
+
+    public function __construct(string $key)
+    {
+        $this->key = $key;
+    }
+
+    public function buildData(): array
+    {
+        $this->writeOnly[$this->key] = 10000;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\Property\RemoveUnusedPrivatePropertyRector\Fixture;
+
+final class WriteOnlyDimFetchUseVariable
+{
+    protected $writeOnly;
+
+    public function __construct()
+    {
+    }
+
+    public function buildData(): array
+    {
+    }
+}
+
+?>

src/NodeAnalyzer/ReadExprAnalyzer.php

@@ -1,66 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Rector\Core\NodeAnalyzer;
-
-use PhpParser\Node\Expr;
-use PhpParser\Node\Expr\Variable;
-use PhpParser\Node\Stmt\Return_;
-use Rector\Core\Exception\NotImplementedYetException;
-use Rector\Core\NodeFinder\NodeUsageFinder;
-use Rector\NodeNestingScope\ParentScopeFinder;
-use Rector\NodeTypeResolver\Node\AttributeKey;
-
-final class ReadExprAnalyzer
-{
-    /**
-     * @var ParentScopeFinder
-     */
-    private $parentScopeFinder;
-
-    /**
-     * @var NodeUsageFinder
-     */
-    private $nodeUsageFinder;
-
-    public function __construct(ParentScopeFinder $parentScopeFinder, NodeUsageFinder $nodeUsageFinder)
-    {
-        $this->parentScopeFinder = $parentScopeFinder;
-        $this->nodeUsageFinder = $nodeUsageFinder;
-    }
-
-    /**
-     * Is the value read or used for read purpose (at least, not only)
-     */
-    public function isExprRead(Expr $expr): bool
-    {
-        if ($expr instanceof Variable) {
-            $parentScope = $this->parentScopeFinder->find($expr);
-            if ($parentScope === null) {
-                return false;
-            }
-
-            $variableUsages = $this->nodeUsageFinder->findVariableUsages((array) $parentScope->stmts, $expr);
-            foreach ($variableUsages as $variableUsage) {
-                if ($this->isCurrentContextRead($variableUsage)) {
-                    return true;
-                }
-            }
-
-            return false;
-        }
-
-        throw new NotImplementedYetException(get_class($expr));
-    }
-
-    private function isCurrentContextRead(Expr $expr): bool
-    {
-        $parent = $expr->getAttribute(AttributeKey::PARENT_NODE);
-        if ($parent instanceof Return_) {
-            return true;
-        }
-
-        throw new NotImplementedYetException();
-    }
-}

src/NodeFinder/NodeUsageFinder.php

@@ -5,8 +5,10 @@ declare(strict_types=1);
 namespace Rector\Core\NodeFinder;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\PropertyFetch;
 use PhpParser\Node\Expr\Variable;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
+use Rector\NodeCollector\NodeCollector\NodeRepository;
 use Rector\NodeNameResolver\NodeNameResolver;
 
 final class NodeUsageFinder
@@ -21,10 +23,19 @@ final class NodeUsageFinder
      */
     private $betterNodeFinder;
 
-    public function __construct(NodeNameResolver $nodeNameResolver, BetterNodeFinder $betterNodeFinder)
-    {
+    /**
+     * @var NodeRepository
+     */
+    private $nodeRepository;
+
+    public function __construct(
+        NodeNameResolver $nodeNameResolver,
+        BetterNodeFinder $betterNodeFinder,
+        NodeRepository $nodeRepository
+    ) {
         $this->nodeNameResolver = $nodeNameResolver;
         $this->betterNodeFinder = $betterNodeFinder;
+        $this->nodeRepository = $nodeRepository;
     }
 
     /**
@@ -47,4 +58,23 @@ final class NodeUsageFinder
             return $this->nodeNameResolver->isName($node, $variableName);
         });
     }
+
+    /**
+     * @return PropertyFetch[]
+     */
+    public function findPropertyFetchUsages(PropertyFetch $desiredPropertyFetch): array
+    {
+        $propertyFetches = $this->nodeRepository->findPropertyFetchesByPropertyFetch($desiredPropertyFetch);
+
+        $propertyFetchesWithoutPropertyFetch = [];
+        foreach ($propertyFetches as $propertyFetch) {
+            if ($propertyFetch === $desiredPropertyFetch) {
+                continue;
+            }
+
+            $propertyFetchesWithoutPropertyFetch[] = $propertyFetch;
+        }
+
+        return $propertyFetchesWithoutPropertyFetch;
+    }
 }

src/PhpParser/Node/Manipulator/PropertyManipulator.php

@@ -18,13 +18,13 @@ use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\Property;
 use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
 use Rector\BetterPhpDocParser\PhpDocNode\JMS\SerializerTypeTagValueNode;
-use Rector\Core\NodeAnalyzer\ReadWritePropertyAnalyzer;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\Core\PhpParser\Printer\BetterStandardPrinter;
 use Rector\Doctrine\AbstractRector\DoctrineTrait;
 use Rector\NodeCollector\NodeCollector\NodeRepository;
 use Rector\NodeNameResolver\NodeNameResolver;
 use Rector\NodeTypeResolver\Node\AttributeKey;
+use Rector\ReadWrite\NodeAnalyzer\ReadWritePropertyAnalyzer;
 use Rector\SOLID\Guard\VariableToConstantGuard;
 
 /**