cleanup ArrayDimFetchToMethodCallRector to use direct attributes

Author: TomasVotruba

rules/Transform/Enum/MagicPropertyHandler.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Transform\Enum;
+
+final class MagicPropertyHandler
+{
+    public const GET = 'get';
+
+    public const SET = 'set';
+
+    public const ISSET_ = 'exists';
+
+    public const UNSET = 'unset';
+}

rules/Transform/Rector/ArrayDimFetch/ArrayDimFetchToMethodCallRector.php

@@ -15,11 +15,11 @@
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Unset_;
-use PhpParser\NodeVisitor;
 use PHPStan\Type\ObjectType;
 use Rector\Contract\Rector\ConfigurableRectorInterface;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\Rector\AbstractRector;
+use Rector\Transform\Enum\MagicPropertyHandler;
 use Rector\Transform\ValueObject\ArrayDimFetchToMethodCall;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -65,9 +65,9 @@ public function getNodeTypes(): array
 
     /**
      * @param ArrayDimFetch|Assign|Isset_|Unset_ $node
-     * @return ($node is Unset_ ? Stmt[]|int : ($node is Isset_ ? Expr|int : MethodCall|int|null))
+     * @return ($node is Unset_ ? Stmt[] : ($node is Isset_ ? Expr : MethodCall|null))
      */
-    public function refactor(Node $node): array|Expr|null|int
+    public function refactor(Node $node): array|Expr|null
     {
         if ($node instanceof Unset_) {
             return $this->handleUnset($node);
@@ -82,7 +82,7 @@ public function refactor(Node $node): array|Expr|null|int
                 return null;
             }
 
-            return $this->createExplicitMethodCall($node->var, 'set', $node->expr);
+            return $this->createExplicitMethodCall($node->var, MagicPropertyHandler::SET, $node->expr);
         }
 
         // is part of assign, skip
@@ -94,7 +94,16 @@ public function refactor(Node $node): array|Expr|null|int
             return null;
         }
 
-        return $this->createExplicitMethodCall($node, 'get');
+        // should be skipped as handled above
+        if ($node->getAttribute(AttributeKey::IS_UNSET_VAR)) {
+            return null;
+        }
+
+        if ($node->getAttribute(AttributeKey::IS_ISSET_VAR)) {
+            return null;
+        }
+
+        return $this->createExplicitMethodCall($node, MagicPropertyHandler::GET);
     }
 
     public function configure(array $configuration): void
@@ -104,7 +113,7 @@ public function configure(array $configuration): void
         $this->arrayDimFetchToMethodCalls = $configuration;
     }
 
-    private function handleIsset(Isset_ $isset): Expr|int|null
+    private function handleIsset(Isset_ $isset): Expr|null
     {
         $issets = [];
         $exprs = [];
@@ -122,9 +131,9 @@ private function handleIsset(Isset_ $isset): Expr|int|null
             $issets[] = $var;
         }
 
-        // @todo fix this
         if ($exprs === []) {
-            return NodeVisitor::DONT_TRAVERSE_CHILDREN;
+            // nothing to handle
+            return null;
         }
 
         if ($issets !== []) {
@@ -142,9 +151,9 @@ private function handleIsset(Isset_ $isset): Expr|int|null
     }
 
     /**
-     * @return Stmt[]|int
+     * @return Stmt[]|null
      */
-    private function handleUnset(Unset_ $unset): array|int
+    private function handleUnset(Unset_ $unset): ?array
     {
         $unsets = [];
         $stmts = [];
@@ -162,8 +171,9 @@ private function handleUnset(Unset_ $unset): array|int
             $unsets[] = $var;
         }
 
+        // nothing to change
         if ($stmts === []) {
-            return NodeVisitor::DONT_TRAVERSE_CHILDREN;
+            return null;
         }
 
         if ($unsets !== []) {
@@ -175,11 +185,11 @@ private function handleUnset(Unset_ $unset): array|int
     }
 
     /**
-     * @param 'get'|'set'|'exists'|'unset' $action
+     * @param MagicPropertyHandler::* $magicPropertyHandler
      */
     private function createExplicitMethodCall(
         ArrayDimFetch $arrayDimFetch,
-        string $action,
+        string $magicPropertyHandler,
         ?Expr $expr = null
     ): ?MethodCall {
         if (! $arrayDimFetch->dim instanceof Node) {
@@ -191,11 +201,11 @@ private function createExplicitMethodCall(
                 continue;
             }
 
-            $method = match ($action) {
-                'get' => $arrayDimFetchToMethodCall->getMethod(),
-                'set' => $arrayDimFetchToMethodCall->getSetMethod(),
-                'exists' => $arrayDimFetchToMethodCall->getExistsMethod(),
-                'unset' => $arrayDimFetchToMethodCall->getUnsetMethod(),
+            $method = match ($magicPropertyHandler) {
+                MagicPropertyHandler::GET => $arrayDimFetchToMethodCall->getMethod(),
+                MagicPropertyHandler::SET => $arrayDimFetchToMethodCall->getSetMethod(),
+                MagicPropertyHandler::ISSET_ => $arrayDimFetchToMethodCall->getExistsMethod(),
+                MagicPropertyHandler::UNSET => $arrayDimFetchToMethodCall->getUnsetMethod(),
             };
 
             if ($method === null) {

src/NodeTypeResolver/Node/AttributeKey.php

@@ -196,6 +196,8 @@ final class AttributeKey
      */
     public const IS_UNSET_VAR = 'is_unset_var';
 
+    public const IS_ISSET_VAR = 'is_isset_var';
+
     /**
      * @var string
      */

src/NodeTypeResolver/PHPStan/Scope/NodeVisitor/ContextNodeVisitor.php

@@ -58,7 +58,16 @@ public function enterNode(Node $node): ?Node
         }
 
         if ($node instanceof Unset_) {
-            $this->processContextInUnset($node);
+            foreach ($node->vars as $var) {
+                $var->setAttribute(AttributeKey::IS_UNSET_VAR, true);
+            }
+            return null;
+        }
+
+        if ($node instanceof Node\Expr\Isset_) {
+            foreach ($node->vars as $var) {
+                $var->setAttribute(AttributeKey::IS_ISSET_VAR, true);
+            }
             return null;
         }
 
@@ -130,13 +139,6 @@ static function (Node $subNode): null {
         );
     }
 
-    private function processContextInUnset(Unset_ $unset): void
-    {
-        foreach ($unset->vars as $var) {
-            $var->setAttribute(AttributeKey::IS_UNSET_VAR, true);
-        }
-    }
-
     private function processContextInIf(If_|Else_|ElseIf_ $node): void
     {
         foreach ($node->stmts as $stmt) {

src/Rector/AbstractRector.php

@@ -161,6 +161,7 @@ final public function enterNode(Node $node): int|Node|null
 
             // only remove node is supported
             if ($refactoredNodeOrState !== NodeVisitor::REMOVE_NODE) {
+                // @todo warn about unsupported state in the future
                 return null;
             }