[CodeQuality] extract CompactFuncCallAnalyzer (#5425)

rules/code-quality/src/NodeAnalyzer/ArrayCompacter.php

@@ -0,0 +1,34 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\CodeQuality\NodeAnalyzer;
+
+use PhpParser\Node\Expr\Array_;
+use PhpParser\Node\Expr\ArrayItem;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Scalar\String_;
+
+final class ArrayCompacter
+{
+    public function compactStringToVariableArray(Array_ $array): void
+    {
+        foreach ($array->items as $arrayItem) {
+            if (! $arrayItem instanceof ArrayItem) {
+                continue;
+            }
+
+            if ($arrayItem->key !== null) {
+                continue;
+            }
+
+            if (! $arrayItem->value instanceof String_) {
+                continue;
+            }
+
+            $variableName = $arrayItem->value->value;
+            $arrayItem->key = new String_($variableName);
+            $arrayItem->value = new Variable($variableName);
+        }
+    }
+}

rules/code-quality/src/NodeAnalyzer/CompactFuncCallAnalyzer.php

@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\CodeQuality\NodeAnalyzer;
+
+use PhpParser\Node\Expr\Array_;
+use PhpParser\Node\Expr\ArrayItem;
+use PhpParser\Node\Scalar\String_;
+use PHPStan\Analyser\Scope;
+
+final class CompactFuncCallAnalyzer
+{
+    public function hasArrayDefinedVariableNames(Array_ $array, Scope $scope): bool
+    {
+        foreach ($array->items as $arrayItem) {
+            if (! $arrayItem instanceof ArrayItem) {
+                continue;
+            }
+
+            if (! $arrayItem->value instanceof String_) {
+                continue;
+            }
+
+            $variableName = $arrayItem->value->value;
+
+            // the variable must not be defined here
+            if ($scope->hasVariableType($variableName)->no()) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+}

rules/code-quality/src/Rector/FuncCall/CompactToVariablesRector.php

@@ -7,14 +7,15 @@ namespace Rector\CodeQuality\Rector\FuncCall;
 use PhpParser\Node;
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Array_;
-use PhpParser\Node\Expr\ArrayItem;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\FuncCall;
-use PhpParser\Node\Expr\Variable;
-use PhpParser\Node\Scalar\String_;
+use PHPStan\Analyser\Scope;
 use PHPStan\Type\Constant\ConstantArrayType;
 use Rector\CodeQuality\CompactConverter;
+use Rector\CodeQuality\NodeAnalyzer\ArrayCompacter;
+use Rector\CodeQuality\NodeAnalyzer\CompactFuncCallAnalyzer;
 use Rector\Core\Rector\AbstractRector;
+use Rector\NodeTypeResolver\Node\AttributeKey;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -30,9 +31,24 @@ final class CompactToVariablesRector extends AbstractRector
      */
     private $compactConverter;
 
-    public function __construct(CompactConverter $compactConverter)
-    {
+    /**
+     * @var CompactFuncCallAnalyzer
+     */
+    private $compactFuncCallAnalyzer;
+
+    /**
+     * @var ArrayCompacter
+     */
+    private $arrayCompacter;
+
+    public function __construct(
+        CompactConverter $compactConverter,
+        CompactFuncCallAnalyzer $compactFuncCallAnalyzer,
+        ArrayCompacter $arrayCompacter
+    ) {
         $this->compactConverter = $compactConverter;
+        $this->compactFuncCallAnalyzer = $compactFuncCallAnalyzer;
+        $this->arrayCompacter = $arrayCompacter;
     }
 
     public function getRuleDefinition(): RuleDefinition
@@ -93,44 +109,49 @@ CODE_SAMPLE
         $firstValueStaticType = $this->getStaticType($firstValue);
 
         if ($firstValueStaticType instanceof ConstantArrayType) {
-            return $this->refactorAssignArray($firstValue);
+            return $this->refactorAssignArray($firstValue, $node);
         }
 
         return null;
     }
 
-    private function refactorAssignedArray(Array_ $array): void
+    private function refactorAssignedArray(Assign $assign, FuncCall $funcCall): void
     {
-        foreach ($array->items as $arrayItem) {
-            if (! $arrayItem instanceof ArrayItem) {
-                continue;
-            }
-
-            if ($arrayItem->key !== null) {
-                continue;
-            }
-
-            if (! $arrayItem->value instanceof String_) {
-                continue;
-            }
-
-            $arrayItem->key = $arrayItem->value;
-            $arrayItem->value = new Variable($arrayItem->value->value);
+        if (! $assign->expr instanceof Array_) {
+            return;
         }
+
+        $array = $assign->expr;
+
+        $assignScope = $assign->getAttribute(AttributeKey::SCOPE);
+        if (! $assignScope instanceof Scope) {
+            return;
+        }
+
+        if ($this->compactFuncCallAnalyzer->hasArrayDefinedVariableNames($array, $assignScope)) {
+            $this->arrayCompacter->compactStringToVariableArray($array);
+            return;
+        }
+
+        $this->removeNode($assign);
+
+        $this->arrayCompacter->compactStringToVariableArray($array);
+
+        $assignVariable = $funcCall->args[0]->value;
+        $preAssign = new Assign($assignVariable, $array);
+
+        $currentStatement = $funcCall->getAttribute(AttributeKey::CURRENT_STATEMENT);
+        $this->addNodeBeforeNode($preAssign, $currentStatement);
     }
 
-    private function refactorAssignArray(Expr $expr): ?Expr
+    private function refactorAssignArray(Expr $expr, FuncCall $funcCall): ?Expr
     {
         $previousAssign = $this->betterNodeFinder->findPreviousAssignToExpr($expr);
         if (! $previousAssign instanceof Assign) {
             return null;
         }
 
-        if (! $previousAssign->expr instanceof Array_) {
-            return null;
-        }
-
-        $this->refactorAssignedArray($previousAssign->expr);
+        $this->refactorAssignedArray($previousAssign, $funcCall);
 
         return $expr;
     }

rules/code-quality/tests/Rector/FuncCall/CompactToVariablesRector/Fixture/compact_with_extract.php.inc

@@ -0,0 +1,38 @@
+<?php
+
+namespace Rector\CodeQuality\Tests\Rector\FuncCall\CompactToVariablesRector\Fixture;
+
+use Rector\Testing\Contract\RunnableInterface;
+
+final class CompactWithExtract implements RunnableInterface
+{
+    public function run()
+    {
+        $values = ['result'];
+
+        $result = 1000;
+
+        return compact($values);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\CodeQuality\Tests\Rector\FuncCall\CompactToVariablesRector\Fixture;
+
+use Rector\Testing\Contract\RunnableInterface;
+
+final class CompactWithExtract implements RunnableInterface
+{
+    public function run()
+    {
+        $result = 1000;
+        $values = ['result' => $result];
+
+        return $values;
+    }
+}
+
+?>

src/PhpParser/Node/BetterNodeFinder.php

@@ -341,6 +341,26 @@ final class BetterNodeFinder
         });
     }
 
+    public function findFirstNext(Node $node, callable $filter): ?Node
+    {
+        $next = $node->getAttribute(AttributeKey::NEXT_NODE);
+        if ($next instanceof Node) {
+            $found = $this->findFirst($next, $filter);
+            if ($found instanceof Node) {
+                return $found;
+            }
+
+            return $this->findFirstNext($next, $filter);
+        }
+
+        $parent = $node->getAttribute(AttributeKey::PARENT_NODE);
+        if ($parent instanceof Node) {
+            return $this->findFirstNext($parent, $filter);
+        }
+
+        return null;
+    }
+
     /**
      * @param Node|Node[] $nodes
      * @param class-string<T> $type
@@ -361,24 +381,4 @@ final class BetterNodeFinder
 
         return null;
     }
-
-    public function findFirstNext(Node $node, callable $filter): ?Node
-    {
-        $next = $node->getAttribute(AttributeKey::NEXT_NODE);
-        if ($next instanceof Node) {
-            $found = $this->findFirst($next, $filter);
-            if ($found instanceof Node) {
-                return $found;
-            }
-
-            return $this->findFirstNext($next, $filter);
-        }
-
-        $parent = $node->getAttribute(AttributeKey::PARENT_NODE);
-        if ($parent instanceof Node) {
-            return $this->findFirstNext($parent, $filter);
-        }
-
-        return null;
-    }
 }