[Privatization] Skip ChangeGlobalVariablesToPropertiesRector on read …

…only global variable (#442) * [Privatization] Skip ChangeGlobalVariablesToPropertiesRector on read only global variable * Fixed 🎉 * clean up * clean up
rectorphp · Jul 15, 2021 · a0f2ad9 · a0f2ad9
1 parent b2c0a05
commit a0f2ad9
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 5 deletions.
diff --git a/...ClassMethod/ChangeGlobalVariablesToPropertiesRector/Fixture/skip_read_only_global.php.inc b/...ClassMethod/ChangeGlobalVariablesToPropertiesRector/Fixture/skip_read_only_global.php.inc
@@ -0,0 +1,17 @@
+<?php
+
+namespace Rector\Tests\Privatization\Rector\ClassMethod\ChangeGlobalVariablesToPropertiesRector\Fixture;
+
+$variable = 'value';
+
+class SkipReadOnlyGlobal
+{
+    public function go()
+    {
+        global $variable;
+
+        echo $variable;
+    }
+}
+
+?>
diff --git a/rules/Privatization/Rector/ClassMethod/ChangeGlobalVariablesToPropertiesRector.php b/rules/Privatization/Rector/ClassMethod/ChangeGlobalVariablesToPropertiesRector.php
@@ -5,6 +5,7 @@
 namespace Rector\Privatization\Rector\ClassMethod;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\PropertyFetch;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Stmt\Class_;
@@ -95,7 +96,7 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        $this->collectGlobalVariableNamesAndRefactorToPropertyFetch($node);
+        $this->collectGlobalVariableNamesAndRefactorToPropertyFetch($classLike, $node);
 
         if ($this->globalVariableNames === []) {
             return null;
@@ -108,13 +109,13 @@ public function refactor(Node $node): ?Node
         return $node;
     }
 
-    private function collectGlobalVariableNamesAndRefactorToPropertyFetch(ClassMethod $classMethod): void
+    private function collectGlobalVariableNamesAndRefactorToPropertyFetch(Class_ $class, ClassMethod $classMethod): void
     {
         $this->globalVariableNames = [];
 
-        $this->traverseNodesWithCallable($classMethod, function (Node $node): ?PropertyFetch {
+        $this->traverseNodesWithCallable($classMethod, function (Node $node) use ($class): ?PropertyFetch {
             if ($node instanceof Global_) {
-                $this->refactorGlobal($node);
+                $this->refactorGlobal($class, $node);
                 return null;
             }
 
@@ -126,14 +127,18 @@ private function collectGlobalVariableNamesAndRefactorToPropertyFetch(ClassMetho
         });
     }
 
-    private function refactorGlobal(Global_ $global): void
+    private function refactorGlobal(Class_ $class, Global_ $global): void
     {
         foreach ($global->vars as $var) {
             $varName = $this->getName($var);
             if ($varName === null) {
                 return;
             }
 
+            if ($this->isReadOnly($class, $varName)) {
+                return;
+            }
+
             $this->globalVariableNames[] = $varName;
         }
 
@@ -154,4 +159,32 @@ private function refactorGlobalVariable(Variable $variable): ?PropertyFetch
 
         return $this->nodeFactory->createPropertyFetch('this', $variableName);
     }
+
+    private function isReadOnly(Class_ $class, string $globalVariableName): bool
+    {
+        /** @var ClassMethod[] $classMethods */
+        $classMethods = $this->betterNodeFinder->findInstanceOf($class, ClassMethod::class);
+        foreach ($classMethods as $classMethod) {
+            $isReAssign = (bool) $this->betterNodeFinder->findFirst((array) $classMethod->stmts, function (Node $node) use (
+                $globalVariableName
+            ): bool {
+                if (! $node instanceof Assign) {
+                    return false;
+                }
+                if ($node->var instanceof Variable) {
+                    return $this->nodeNameResolver->isName($node->var, $globalVariableName);
+                }
+                if ($node->var instanceof PropertyFetch) {
+                    return $this->nodeNameResolver->isName($node->var, $globalVariableName);
+                }
+                return false;
+            });
+
+            if ($isReAssign) {
+                return false;
+            }
+        }
+
+        return true;
+    }
 }