[Php81] Skip used as ArrayDimFetch on Arg on side effect FuncCall on …

…ReadOnlyPropertyRector (#1821)

* [Php81] Skip used in CallLike arg on ReadOnlyPropertyRector

* rename fixture

* use VisibilityManipulator->hasVisibility for Private detection

* different method for refactor property

* more fixture for Property

* [ci-review] Rector Rectify

* Fixed 🎉

* [ci-review] Rector Rectify

* [ci-review] Rector Rectify

Co-authored-by: GitHub Action <action@github.com>

packages/ReadWrite/NodeAnalyzer/ReadWritePropertyAnalyzer.php

@@ -7,6 +7,7 @@
 use PhpParser\Node;
 use PhpParser\Node\Arg;
 use PhpParser\Node\Expr\ArrayDimFetch;
+use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\Isset_;
 use PhpParser\Node\Expr\PostDec;
 use PhpParser\Node\Expr\PostInc;
@@ -18,6 +19,7 @@
 use Rector\Core\Exception\ShouldNotHappenException;
 use Rector\Core\NodeManipulator\AssignManipulator;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
+use Rector\DeadCode\SideEffect\PureFunctionDetector;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\ReadWrite\Guard\VariableToConstantGuard;
 
@@ -28,6 +30,7 @@ public function __construct(
         private readonly AssignManipulator $assignManipulator,
         private readonly ReadExprAnalyzer $readExprAnalyzer,
         private readonly BetterNodeFinder $betterNodeFinder,
+        private readonly PureFunctionDetector $pureFunctionDetector
     ) {
     }
 
@@ -51,7 +54,32 @@ public function isRead(PropertyFetch | StaticPropertyFetch $node): bool
             return $this->isArrayDimFetchRead($parent);
         }
 
-        return ! $this->assignManipulator->isLeftPartOfAssign($node);
+        if (! $parent instanceof ArrayDimFetch) {
+            return ! $this->assignManipulator->isLeftPartOfAssign($node);
+        }
+
+        if (! $this->isArrayDimFetchInImpureFunction($parent, $node)) {
+            return ! $this->assignManipulator->isLeftPartOfAssign($node);
+        }
+
+        return false;
+    }
+
+    private function isArrayDimFetchInImpureFunction(ArrayDimFetch $arrayDimFetch, Node $node): bool
+    {
+        if ($arrayDimFetch->var === $node) {
+            $arg = $this->betterNodeFinder->findParentType($arrayDimFetch, Arg::class);
+            if ($arg instanceof Arg) {
+                $parentArg = $arg->getAttribute(AttributeKey::PARENT_NODE);
+                if (! $parentArg instanceof FuncCall) {
+                    return false;
+                }
+
+                return ! $this->pureFunctionDetector->detect($parentArg);
+            }
+        }
+
+        return false;
     }
 
     private function unwrapPostPreIncDec(Node $node): Node

rules-tests/Php81/Rector/Property/ReadOnlyPropertyRector/Fixture/skip_used_as_array_dim_fetch_in_side_effect_call_arg.php.inc

@@ -0,0 +1,19 @@
+<?php
+
+namespace Rector\Tests\Php81\Rector\Property\ReadOnlyPropertyRector\Fixture;
+
+final class SkipUsedAsArrayDimFetchInSideEffectCallArg
+{
+    /**
+     * @param array<string, array<string>> $items
+     */
+    public function __construct(private array $items)
+    {}
+
+    public function popFooItem(): void
+    {
+        if (!empty($this->items) && array_key_exists('foo', $this->items)) {
+            array_pop($this->items['foo']);
+        }
+    }
+}

rules-tests/Php81/Rector/Property/ReadOnlyPropertyRector/Fixture/skip_used_as_array_dim_fetch_in_side_effect_call_arg2.php.inc

@@ -0,0 +1,23 @@
+<?php
+
+namespace Rector\Tests\Php81\Rector\Property\ReadOnlyPropertyRector\Fixture;
+
+final class SkipUsedAsArrayDimFetchInSideEffectCallArg2
+{
+    private array $items;
+
+    /**
+     * @param array<string, array<string>> $items
+     */
+    public function __construct(array $items)
+    {
+        $this->items = $items;
+    }
+
+    public function popFooItem(): void
+    {
+        if (!empty($this->items) && array_key_exists('foo', $this->items)) {
+            array_pop($this->items['foo']);
+        }
+    }
+}

rules/Php81/Rector/Property/ReadOnlyPropertyRector.php

@@ -85,39 +85,44 @@ public function refactor(Node $node): ?Node
             return $this->refactorParam($node);
         }
 
+        return $this->refactorProperty($node);
+    }
+
+    public function provideMinPhpVersion(): int
+    {
+        return PhpVersionFeature::READONLY_PROPERTY;
+    }
+
+    private function refactorProperty(Property $property): ?Property
+    {
         // 1. is property read-only?
-        if ($this->propertyManipulator->isPropertyChangeableExceptConstructor($node)) {
+        if ($this->propertyManipulator->isPropertyChangeableExceptConstructor($property)) {
             return null;
         }
 
-        if ($node->isReadonly()) {
+        if ($property->isReadonly()) {
             return null;
         }
 
-        if ($node->props[0]->default instanceof Expr) {
+        if ($property->props[0]->default instanceof Expr) {
             return null;
         }
 
-        if ($node->type === null) {
+        if ($property->type === null) {
             return null;
         }
 
-        if ($node->flags !== Visibility::PRIVATE) {
+        if (! $this->visibilityManipulator->hasVisibility($property, Visibility::PRIVATE)) {
             return null;
         }
 
-        $this->visibilityManipulator->makeReadonly($node);
-        return $node;
-    }
-
-    public function provideMinPhpVersion(): int
-    {
-        return PhpVersionFeature::READONLY_PROPERTY;
+        $this->visibilityManipulator->makeReadonly($property);
+        return $property;
     }
 
     private function refactorParam(Param $param): Param | null
     {
-        if ($param->flags !== Visibility::PRIVATE) {
+        if (! $this->visibilityManipulator->hasVisibility($param, Visibility::PRIVATE)) {
             return null;
         }
 

src/NodeManipulator/PropertyManipulator.php

@@ -9,6 +9,7 @@
 use PhpParser\Node;
 use PhpParser\Node\Arg;
 use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\ArrayDimFetch;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\PostDec;
 use PhpParser\Node\Expr\PostInc;
@@ -212,6 +213,10 @@ private function isChangeableContext(PropertyFetch | StaticPropertyFetch $proper
             }
         }
 
+        if ($parent instanceof ArrayDimFetch) {
+            return ! $this->readWritePropertyAnalyzer->isRead($propertyFetch);
+        }
+
         return false;
     }