[Privatization] Add constant support on ChangeReadOnlyVariableWithDef…

…aultValueToConstantRector (#1890) * [Privatization] Add constant support on ChangeReadOnlyVariableWithDefaultValueToConstantRector * implemented * [ci-review] Rector Rectify * add missing Use ConstFetch * [ci-review] Rector Rectify * rename method * phpstan * [ci-review] Rector Rectify * flip check * dependency circular between ExprAnalyzer and ArrayManipulator for check dynamic value * [ci-review] Rector Rectify * [ci-review] Rector Rectify * clean up * phpstan * [ci-review] Rector Rectify * final touch: clean up * [ci-review] Rector Rectify Co-authored-by: GitHub Action <action@github.com>
rectorphp · Mar 2, 2022 · 926a7e0 · 926a7e0
1 parent 0cac723
commit 926a7e0
Show file tree

Hide file tree

Showing 7 changed files with 110 additions and 91 deletions.
diff --git a/packages/NodeNestingScope/ContextAnalyzer.php b/packages/NodeNestingScope/ContextAnalyzer.php
@@ -77,9 +77,7 @@ public function isInIf(Node $node): bool
 
     public function isHasAssignWithIndirectReturn(Node $node, If_ $if): bool
     {
-        $loopNodes = self::LOOP_NODES;
-
-        foreach ($loopNodes as $loopNode) {
+        foreach (self::LOOP_NODES as $loopNode) {
             $loopObjectType = new ObjectType($loopNode);
             $parentType = $this->nodeTypeResolver->getType($node);
             $superType = $parentType->isSuperTypeOf($loopObjectType);

diff --git a/...ass_/ChangeReadOnlyVariableWithDefaultValueToConstantRector/Fixture/some_constant.php.inc b/...ass_/ChangeReadOnlyVariableWithDefaultValueToConstantRector/Fixture/some_constant.php.inc
@@ -0,0 +1,35 @@
+<?php
+
+namespace Rector\Tests\Privatization\Rector\Class_\ChangeReadOnlyVariableWithDefaultValueToConstantRector\Fixture;
+
+class SomeConstant
+{
+    public function run()
+    {
+        $replacements = \PHPUnit\Framework\TestCase\Notice::class;
+
+        if ($replacements === 'asdf') {
+        }
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Privatization\Rector\Class_\ChangeReadOnlyVariableWithDefaultValueToConstantRector\Fixture;
+
+class SomeConstant
+{
+    /**
+     * @var string
+     */
+    private const REPLACEMENTS = \PHPUnit\Framework\TestCase\Notice::class;
+    public function run()
+    {
+        if (self::REPLACEMENTS === 'asdf') {
+        }
+    }
+}
+
+?>
diff --git a/rules/Php81/NodeAnalyzer/ComplexNewAnalyzer.php b/rules/Php81/NodeAnalyzer/ComplexNewAnalyzer.php
@@ -7,18 +7,15 @@
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Array_;
 use PhpParser\Node\Expr\ArrayItem;
-use PhpParser\Node\Expr\ClassConstFetch;
-use PhpParser\Node\Expr\ConstFetch;
 use PhpParser\Node\Expr\New_;
-use PhpParser\Node\Identifier;
-use PhpParser\Node\Name;
 use PhpParser\Node\Name\FullyQualified;
-use PhpParser\Node\Scalar;
 use Rector\Core\NodeAnalyzer\ExprAnalyzer;
+use Rector\Core\NodeManipulator\ArrayManipulator;
 
 final class ComplexNewAnalyzer
 {
     public function __construct(
+        private readonly ArrayManipulator $arrayManipulator,
         private readonly ExprAnalyzer $exprAnalyzer
     ) {
     }
@@ -38,15 +35,12 @@ public function isDynamic(New_ $new): bool
                 continue;
             }
 
+            // new inside array is allowed for New in initializer
             if ($value instanceof Array_ && $this->isAllowedArray($value)) {
                 continue;
             }
 
-            if ($value instanceof Scalar) {
-                continue;
-            }
-
-            if ($this->isAllowedConstFetchOrClassConstFeth($value)) {
+            if (! $this->exprAnalyzer->isDynamicValue($value)) {
                 continue;
             }
 
@@ -56,19 +50,6 @@ public function isDynamic(New_ $new): bool
         return false;
     }
 
-    private function isAllowedConstFetchOrClassConstFeth(Expr $expr): bool
-    {
-        if (! in_array($expr::class, [ConstFetch::class, ClassConstFetch::class], true)) {
-            return false;
-        }
-
-        if ($expr instanceof ClassConstFetch) {
-            return $expr->class instanceof Name && $expr->name instanceof Identifier;
-        }
-
-        return true;
-    }
-
     private function isAllowedNew(Expr $expr): bool
     {
         if ($expr instanceof New_) {
@@ -80,7 +61,7 @@ private function isAllowedNew(Expr $expr): bool
 
     private function isAllowedArray(Array_ $array): bool
     {
-        if (! $this->exprAnalyzer->isDynamicArray($array)) {
+        if (! $this->arrayManipulator->isDynamicArray($array)) {
             return true;
         }
 

diff --git a/src/Console/ConsoleApplication.php b/src/Console/ConsoleApplication.php
@@ -25,13 +25,17 @@ final class ConsoleApplication extends Application
      */
     private const NAME = 'Rector';
 
+    /**
+     * @var string
+     */
+    private const VERSION = VersionResolver::PACKAGE_VERSION;
+
     /**
      * @param Command[] $commands
      */
     public function __construct(CommandNaming $commandNaming, array $commands = [])
     {
-        $version = VersionResolver::PACKAGE_VERSION;
-        parent::__construct(self::NAME, $version);
+        parent::__construct(self::NAME, self::VERSION);
 
         foreach ($commands as $command) {
             $commandName = $commandNaming->resolveFromCommand($command);

diff --git a/src/NodeAnalyzer/ExprAnalyzer.php b/src/NodeAnalyzer/ExprAnalyzer.php
@@ -6,14 +6,16 @@
 
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Array_;
-use PhpParser\Node\Expr\ArrayItem;
+use PhpParser\Node\Expr\ClassConstFetch;
+use PhpParser\Node\Expr\ConstFetch;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\FunctionLike;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Name;
 use PhpParser\Node\Scalar;
-use PhpParser\Node\Scalar\LNumber;
-use PhpParser\Node\Scalar\String_;
 use PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode;
 use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
+use Rector\Core\NodeManipulator\ArrayManipulator;
 use Rector\Core\PhpParser\Comparing\NodeComparator;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\NodeNameResolver\NodeNameResolver;
@@ -24,7 +26,8 @@ public function __construct(
         private readonly NodeComparator $nodeComparator,
         private readonly BetterNodeFinder $betterNodeFinder,
         private readonly PhpDocInfoFactory $phpDocInfoFactory,
-        private readonly NodeNameResolver $nodeNameResolver
+        private readonly NodeNameResolver $nodeNameResolver,
+        private readonly ArrayManipulator $arrayManipulator
     ) {
     }
 
@@ -61,52 +64,29 @@ public function isNonTypedFromParam(Expr $expr): bool
         return false;
     }
 
-    public function isDynamicArray(Array_ $array): bool
+    public function isDynamicValue(Expr $expr): bool
     {
-        foreach ($array->items as $item) {
-            if (! $item instanceof ArrayItem) {
-                continue;
-            }
-
-            $key = $item->key;
-
-            if (! $this->isAllowedArrayKey($key)) {
-                return true;
+        if (! $expr instanceof Array_) {
+            if ($expr instanceof Scalar) {
+                return false;
             }
 
-            $value = $item->value;
-            if (! $this->isAllowedArrayValue($value)) {
-                return true;
-            }
+            return ! $this->isAllowedConstFetchOrClassConstFeth($expr);
         }
 
-        return false;
+        return $this->arrayManipulator->isDynamicArray($expr);
     }
 
-    private function isAllowedArrayKey(?Expr $expr): bool
+    private function isAllowedConstFetchOrClassConstFeth(Expr $expr): bool
     {
-        if (! $expr instanceof Expr) {
-            return true;
-        }
-
-        return in_array($expr::class, [String_::class, LNumber::class], true);
-    }
-
-    private function isAllowedArrayValue(Expr $expr): bool
-    {
-        if ($expr instanceof Array_) {
-            return true;
+        if (! in_array($expr::class, [ConstFetch::class, ClassConstFetch::class], true)) {
+            return false;
         }
 
-        return $this->isAllowedArrayOrScalar($expr);
-    }
-
-    private function isAllowedArrayOrScalar(Expr $expr): bool
-    {
-        if (! $expr instanceof Array_) {
-            return $expr instanceof Scalar;
+        if ($expr instanceof ClassConstFetch) {
+            return $expr->class instanceof Name && $expr->name instanceof Identifier;
         }
 
-        return ! $this->isDynamicArray($expr);
+        return true;
     }
 }
diff --git a/src/NodeManipulator/ArrayManipulator.php b/src/NodeManipulator/ArrayManipulator.php
@@ -4,42 +4,50 @@
 
 namespace Rector\Core\NodeManipulator;
 
+use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Array_;
 use PhpParser\Node\Expr\ArrayItem;
-use PhpParser\Node\Scalar;
+use PhpParser\Node\Scalar\LNumber;
 use PhpParser\Node\Scalar\String_;
 use Rector\ChangesReporting\Collector\RectorChangeCollector;
+use Rector\Core\NodeAnalyzer\ExprAnalyzer;
+use Symfony\Contracts\Service\Attribute\Required;
 
 final class ArrayManipulator
 {
+    private ExprAnalyzer $exprAnalyzer;
+
     public function __construct(
         private readonly RectorChangeCollector $rectorChangeCollector
     ) {
     }
 
-    public function isArrayOnlyScalarValues(Array_ $array): bool
+    #[Required]
+    public function autowire(ExprAnalyzer $exprAnalyzer): void
     {
-        foreach ($array->items as $arrayItem) {
-            if (! $arrayItem instanceof ArrayItem) {
+        $this->exprAnalyzer = $exprAnalyzer;
+    }
+
+    public function isDynamicArray(Array_ $array): bool
+    {
+        foreach ($array->items as $item) {
+            if (! $item instanceof ArrayItem) {
                 continue;
             }
 
-            if ($arrayItem->value instanceof Array_) {
-                if (! $this->isArrayOnlyScalarValues($arrayItem->value)) {
-                    return false;
-                }
+            $key = $item->key;
 
-                continue;
+            if (! $this->isAllowedArrayKey($key)) {
+                return true;
             }
 
-            if ($arrayItem->value instanceof Scalar) {
-                continue;
+            $value = $item->value;
+            if (! $this->isAllowedArrayValue($value)) {
+                return true;
             }
-
-            return false;
         }
 
-        return true;
+        return false;
     }
 
     public function addItemToArrayUnderKey(Array_ $array, ArrayItem $newArrayItem, string $key): void
@@ -97,4 +105,22 @@ public function hasKeyName(ArrayItem $arrayItem, string $name): bool
 
         return $arrayItem->key->value === $name;
     }
+
+    private function isAllowedArrayKey(?Expr $expr): bool
+    {
+        if (! $expr instanceof Expr) {
+            return true;
+        }
+
+        return in_array($expr::class, [String_::class, LNumber::class], true);
+    }
+
+    private function isAllowedArrayValue(Expr $expr): bool
+    {
+        if ($expr instanceof Array_) {
+            return ! $this->isDynamicArray($expr);
+        }
+
+        return ! $this->exprAnalyzer->isDynamicValue($expr);
+    }
 }
diff --git a/src/NodeManipulator/VariableManipulator.php b/src/NodeManipulator/VariableManipulator.php
@@ -7,14 +7,13 @@
 use PhpParser\Node;
 use PhpParser\Node\Arg;
 use PhpParser\Node\Expr;
-use PhpParser\Node\Expr\Array_;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\Variable;
-use PhpParser\Node\Scalar;
 use PhpParser\Node\Scalar\Encapsed;
 use PhpParser\Node\Scalar\EncapsedStringPart;
 use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\ClassMethod;
+use Rector\Core\NodeAnalyzer\ExprAnalyzer;
 use Rector\Core\PhpParser\Comparing\NodeComparator;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\NodeNameResolver\NodeNameResolver;
@@ -25,13 +24,13 @@
 final class VariableManipulator
 {
     public function __construct(
-        private readonly ArrayManipulator $arrayManipulator,
         private readonly AssignManipulator $assignManipulator,
         private readonly BetterNodeFinder $betterNodeFinder,
         private readonly SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
         private readonly NodeNameResolver $nodeNameResolver,
         private readonly VariableToConstantGuard $variableToConstantGuard,
-        private readonly NodeComparator $nodeComparator
+        private readonly NodeComparator $nodeComparator,
+        private readonly ExprAnalyzer $exprAnalyzer
     ) {
     }
 
@@ -53,18 +52,14 @@ function (Node $node) use (&$assignsOfArrayToVariable) {
                     return null;
                 }
 
-                if (! $node->expr instanceof Array_ && ! $node->expr instanceof Scalar) {
+                if ($this->exprAnalyzer->isDynamicValue($node->expr)) {
                     return null;
                 }
 
                 if ($this->hasEncapsedStringPart($node->expr)) {
                     return null;
                 }
 
-                if ($node->expr instanceof Array_ && ! $this->arrayManipulator->isArrayOnlyScalarValues($node->expr)) {
-                    return null;
-                }
-
                 if ($this->isTestCaseExpectedVariable($node->var)) {
                     return null;
                 }