[cleanup] cleanup ArrayDimFetchToMethodCallRector to use direct nodes (#7713)

Author: TomasVotruba

phpstan.neon

@@ -385,7 +385,9 @@ parameters:
 
         # handles full file
         -
-            path: rules/TypeDeclaration/Rector/StmtsAwareInterface/IncreaseDeclareStrictTypesRector.php
+            paths:
+                - rules/TypeDeclaration/Rector/StmtsAwareInterface/IncreaseDeclareStrictTypesRector.php
+                - rules/TypeDeclaration/Rector/StmtsAwareInterface/DeclareStrictTypesRector.php
             identifier: rector.noOnlyNullReturnInRefactor
 
         -

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 = [];
@@ -123,7 +132,8 @@ private function handleIsset(Isset_ $isset): Expr|int|null
         }
 
         if ($exprs === []) {
-            return NodeVisitor::DONT_TRAVERSE_CHILDREN;
+            // nothing to handle
+            return null;
         }
 
         if ($issets !== []) {
@@ -141,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 = [];
@@ -161,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 !== []) {
@@ -174,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) {
@@ -190,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) {

rules/TypeDeclaration/Rector/StmtsAwareInterface/DeclareStrictTypesRector.php

@@ -7,7 +7,6 @@
 use PhpParser\Node;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Nop;
-use PhpParser\NodeVisitor;
 use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
 use Rector\Contract\Rector\HTMLAverseRectorInterface;
 use Rector\PhpParser\Enum\NodeGroup;
@@ -103,11 +102,11 @@ public function getNodeTypes(): array
     /**
      * @param StmtsAware $node
      */
-    public function refactor(Node $node): int
+    public function refactor(Node $node): null
     {
         // workaround, as Rector now only hooks to specific nodes, not arrays
         // avoid traversing, as we already handled in beforeTraverse()
-        return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
+        return null;
     }
 
     public function provideMinPhpVersion(): int

src/NodeTypeResolver/Node/AttributeKey.php

@@ -202,6 +202,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

@@ -11,6 +11,7 @@
 use PhpParser\Node\Expr\ArrayDimFetch;
 use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
 use PhpParser\Node\Expr\Closure;
+use PhpParser\Node\Expr\Isset_;
 use PhpParser\Node\Expr\PostDec;
 use PhpParser\Node\Expr\PostInc;
 use PhpParser\Node\Expr\PreDec;
@@ -65,6 +66,14 @@ public function enterNode(Node $node): ?Node
             return null;
         }
 
+        if ($node instanceof Isset_) {
+            foreach ($node->vars as $var) {
+                $var->setAttribute(AttributeKey::IS_ISSET_VAR, true);
+            }
+
+            return null;
+        }
+
         if ($node instanceof Attribute) {
             $this->processContextInAttribute($node);
             return null;