[Core] Handle parent return/assign from FuncCall with No Scope (#2525)

rectorphp · Jun 19, 2022 · 372e030 · 372e030
1 parent 3e6c9cf
commit 372e030
Show file tree

Hide file tree

Showing 23 changed files with 295 additions and 201 deletions.
diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml
@@ -33,9 +33,6 @@ jobs:
                     - 'e2e/use-rector-configurator'
                     - 'e2e/applied-rule-removed-node'
                     - 'e2e/parallel with space'
-                    - 'e2e/parent-arg-from-func-call'
-                    - 'e2e/parent-binaryop-from-func-call'
-                    - 'e2e/parent-assign-from-func-call'
 
         name: End to end test - ${{ matrix.directory }}
 

diff --git a/e2e/parent-arg-from-func-call/.gitignore b/e2e/parent-arg-from-func-call/.gitignore
diff --git a/e2e/parent-arg-from-func-call/composer.json b/e2e/parent-arg-from-func-call/composer.json
diff --git a/e2e/parent-arg-from-func-call/rector.php b/e2e/parent-arg-from-func-call/rector.php
diff --git a/e2e/parent-assign-from-func-call/.gitignore b/e2e/parent-assign-from-func-call/.gitignore
diff --git a/e2e/parent-assign-from-func-call/composer.json b/e2e/parent-assign-from-func-call/composer.json
diff --git a/e2e/parent-assign-from-func-call/expected-output.diff b/e2e/parent-assign-from-func-call/expected-output.diff
diff --git a/e2e/parent-assign-from-func-call/rector.php b/e2e/parent-assign-from-func-call/rector.php
diff --git a/e2e/parent-assign-from-func-call/src/Loop.php b/e2e/parent-assign-from-func-call/src/Loop.php
diff --git a/e2e/parent-binaryop-from-func-call/.gitignore b/e2e/parent-binaryop-from-func-call/.gitignore
diff --git a/e2e/parent-binaryop-from-func-call/composer.json b/e2e/parent-binaryop-from-func-call/composer.json
diff --git a/e2e/parent-binaryop-from-func-call/rector.php b/e2e/parent-binaryop-from-func-call/rector.php
diff --git a/full_build.sh b/full_build.sh
@@ -20,6 +20,13 @@ rm -rf vendor
 composer clear-cache
 
 composer install --ansi
+
+# ensure clear cache
+bin/rector --version --clear-cache
+
+# ensure remove cache directory
+php -r 'shell_exec("rm -rf " . sys_get_temp_dir() . "/rector_cached_files");';
+
 composer install --no-dev --ansi
 
 # early downgrade individual functions

diff --git a/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php b/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php
@@ -8,6 +8,9 @@
 use PhpParser\Node\Arg;
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\AssignOp;
+use PhpParser\Node\Expr\BinaryOp;
+use PhpParser\Node\Expr\Ternary;
 use PhpParser\Node\Name;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Class_;
@@ -101,6 +104,18 @@ public function processNodes(
             $isScopeRefreshing,
             $smartFileInfo
         ): void {
+            if ($node instanceof Ternary) {
+                $this->processTernary($node, $mutatingScope);
+            }
+
+            if ($node instanceof AssignOp) {
+                $node->expr->setAttribute(AttributeKey::SCOPE, $mutatingScope);
+            }
+
+            if ($node instanceof BinaryOp) {
+                $this->processBinaryOp($node, $mutatingScope);
+            }
+
             if ($node instanceof Arg) {
                 $node->value->setAttribute(AttributeKey::SCOPE, $mutatingScope);
             }
@@ -111,13 +126,7 @@ public function processNodes(
             }
 
             if ($node instanceof Property) {
-                foreach ($node->props as $propertyProperty) {
-                    $propertyProperty->setAttribute(AttributeKey::SCOPE, $mutatingScope);
-
-                    if ($propertyProperty->default instanceof Expr) {
-                        $propertyProperty->default->setAttribute(AttributeKey::SCOPE, $mutatingScope);
-                    }
-                }
+                $this->processProperty($node, $mutatingScope);
             }
 
             if ($node instanceof Switch_) {
@@ -127,8 +136,14 @@ public function processNodes(
                 }
             }
 
-            if ($node instanceof TryCatch && $node->finally instanceof Finally_) {
-                $node->finally->setAttribute(AttributeKey::SCOPE, $mutatingScope);
+            if ($node instanceof TryCatch) {
+                foreach ($node->catches as $catch) {
+                    $this->processNodes($catch->stmts, $smartFileInfo, $mutatingScope);
+                }
+
+                if ($node->finally instanceof Finally_) {
+                    $node->finally->setAttribute(AttributeKey::SCOPE, $mutatingScope);
+                }
             }
 
             if ($node instanceof Assign) {
@@ -142,16 +157,6 @@ public function processNodes(
                 $node->expr->setAttribute(AttributeKey::SCOPE, $mutatingScope);
             }
 
-            // scope is missing on attributes
-            // @todo decorate parent nodes too
-            if ($node instanceof Property) {
-                foreach ($node->attrGroups as $attrGroup) {
-                    foreach ($attrGroup->attrs as $attribute) {
-                        $attribute->setAttribute(AttributeKey::SCOPE, $mutatingScope);
-                    }
-                }
-            }
-
             if ($node instanceof Trait_) {
                 $traitName = $this->resolveClassName($node);
 
@@ -196,11 +201,7 @@ public function processNodes(
 
             // special case for unreachable nodes
             if ($node instanceof UnreachableStatementNode) {
-                $originalStmt = $node->getOriginalStatement();
-                $originalStmt->setAttribute(AttributeKey::IS_UNREACHABLE, true);
-                $originalStmt->setAttribute(AttributeKey::SCOPE, $mutatingScope);
-
-                $this->processNodes([$originalStmt], $smartFileInfo, $mutatingScope);
+                $this->processUnreachableStatementNode($node, $smartFileInfo, $mutatingScope);
             } else {
                 $node->setAttribute(AttributeKey::SCOPE, $mutatingScope);
             }
@@ -211,6 +212,58 @@ public function processNodes(
         return $this->processNodesWithDependentFiles($smartFileInfo, $stmts, $scope, $nodeCallback);
     }
 
+    private function processUnreachableStatementNode(
+        UnreachableStatementNode $unreachableStatementNode,
+        SmartFileInfo $smartFileInfo,
+        MutatingScope $mutatingScope
+    ): void {
+        $originalStmt = $unreachableStatementNode->getOriginalStatement();
+        $originalStmt->setAttribute(AttributeKey::IS_UNREACHABLE, true);
+        $originalStmt->setAttribute(AttributeKey::SCOPE, $mutatingScope);
+
+        $this->processNodes([$originalStmt], $smartFileInfo, $mutatingScope);
+
+        $next = $originalStmt->getAttribute(AttributeKey::NEXT_NODE);
+        while ($next instanceof Stmt) {
+            $next->setAttribute(AttributeKey::IS_UNREACHABLE, true);
+
+            $this->processNodes([$next], $smartFileInfo, $mutatingScope);
+            $next = $next->getAttribute(AttributeKey::NEXT_NODE);
+        }
+    }
+
+    private function processProperty(Property $property, MutatingScope $mutatingScope): void
+    {
+        foreach ($property->props as $propertyProperty) {
+            $propertyProperty->setAttribute(AttributeKey::SCOPE, $mutatingScope);
+
+            if ($propertyProperty->default instanceof Expr) {
+                $propertyProperty->default->setAttribute(AttributeKey::SCOPE, $mutatingScope);
+            }
+        }
+
+        foreach ($property->attrGroups as $attrGroup) {
+            foreach ($attrGroup->attrs as $attribute) {
+                $attribute->setAttribute(AttributeKey::SCOPE, $mutatingScope);
+            }
+        }
+    }
+
+    private function processBinaryOp(BinaryOp $binaryOp, MutatingScope $mutatingScope): void
+    {
+        $binaryOp->left->setAttribute(AttributeKey::SCOPE, $mutatingScope);
+        $binaryOp->right->setAttribute(AttributeKey::SCOPE, $mutatingScope);
+    }
+
+    private function processTernary(Ternary $ternary, MutatingScope $mutatingScope): void
+    {
+        if ($ternary->if instanceof Expr) {
+            $ternary->if->setAttribute(AttributeKey::SCOPE, $mutatingScope);
+        }
+
+        $ternary->else->setAttribute(AttributeKey::SCOPE, $mutatingScope);
+    }
+
     /**
      * @param Stmt[] $stmts
      * @param callable(Node $node, MutatingScope $scope): void $nodeCallback

diff --git a/...FuncCall/DowngradeArrayIsListRector/Fixture/no_scope_parent_assign_from_func_call.php.inc b/...FuncCall/DowngradeArrayIsListRector/Fixture/no_scope_parent_assign_from_func_call.php.inc
@@ -0,0 +1,65 @@
+<?php
+
+namespace Rector\Tests\DowngradePhp81\Rector\FuncCall\DowngradeArrayIsListRector\Fixture;
+
+final class Loop
+{
+    /**
+     * @var LoopInterface
+     */
+    private static $instance;
+
+    public static function get()
+    {
+        if (self::$instance instanceof LoopInterface) {
+            return self::$instance;
+        }
+
+        register_shutdown_function(function () {
+            $test = array_is_list([]);
+        });
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\DowngradePhp81\Rector\FuncCall\DowngradeArrayIsListRector\Fixture;
+
+final class Loop
+{
+    /**
+     * @var LoopInterface
+     */
+    private static $instance;
+
+    public static function get()
+    {
+        if (self::$instance instanceof LoopInterface) {
+            return self::$instance;
+        }
+
+        register_shutdown_function(function () {
+            $arrayIsList = function (array $array) : bool {
+                if (function_exists('array_is_list')) {
+                    return array_is_list($array);
+                }
+                if ($array === []) {
+                    return true;
+                }
+                $current_key = 0;
+                foreach ($array as $key => $noop) {
+                    if ($key !== $current_key) {
+                        return false;
+                    }
+                    ++$current_key;
+                }
+                return true;
+            };
+            $test = $arrayIsList([]);
+        });
+    }
+}
+
+?>