[code-quality] Improve InlineArrayReturnAssignRector to include implicit variable init and nested keys

Author: TomasVotruba

phpstan.neon

@@ -8,7 +8,7 @@ rules:
 parameters:
     level: 8
 
-    reportUnmatchedIgnoredErrors: false
+    # reportUnmatchedIgnoredErrors: false
     errorFormat: symplify
 
     # requires exact closure types
@@ -338,3 +338,7 @@ parameters:
         -
             message: '#Offset float\|int\|string might not exist on string#'
             path: rules/Php70/EregToPcreTransformer.php
+
+        -
+            identifier: symplify.noReference
+            message: '#Use explicit return value over magic &reference#'

rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/include_nested_variable.php.inc

@@ -0,0 +1,30 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
+
+final class IncludeNestedVariable
+{
+    public function create()
+    {
+        $items[0]['name'] = 'John';
+        $items[0]['surname'] = 'Doe';
+        $items[0]['age'] = 50;
+        return $items;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
+
+final class IncludeNestedVariable
+{
+    public function create()
+    {
+        return [['name' => 'John', 'surname' => 'Doe', 'age' => 50]];
+    }
+}
+
+?>

rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/keep_comments.php.inc

@@ -0,0 +1,32 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
+
+final class NestedArrayWithMultipleKeys
+{
+    public function run()
+    {
+        $arr = [];
+        $arr['foo']['bar']['baz'] = ['a' => 1, 'b' => 2];
+
+        return $arr;
+    }
+}
+
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
+
+final class NestedArrayWithMultipleKeys
+{
+    public function run()
+    {
+        return ['foo' => ['bar' => ['baz' => ['a' => 1, 'b' => 2]]]];
+    }
+}
+
+
+?>

rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/nested_array_with_multiple_keys.php.inc

@@ -0,0 +1,17 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
+
+final class SkipDifferentVariable
+{
+    public function create()
+    {
+        $item['sub_key'] = 500;
+
+        $items['name'] = 'John';
+        $items['surname'] = 'Doe';
+        $items['age'] = $item;
+
+        return $items;
+    }
+}

rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_different_variable.php.inc

@@ -0,0 +1,19 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
+
+final class SkipMultipleReturns
+{
+    public function create()
+    {
+        $items['name'] = 'John';
+        $items['surname'] = 'Doe';
+        $items['age'] = 50;
+
+        if (mt_rand(1, 2)) {
+            return [];
+        }
+
+        return $items;
+    }
+}

rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_multiple_returns.php.inc

@@ -0,0 +1,30 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
+
+class SomeClass
+{
+    public function create()
+    {
+        $items['name'] = 'John';
+        $items['surname'] = 'Doe';
+        $items['age'] = 50;
+        return $items;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture;
+
+class SomeClass
+{
+    public function create()
+    {
+        return ['name' => 'John', 'surname' => 'Doe', 'age' => 50];
+    }
+}
+
+?>

rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_nested_array.php.inc

@@ -4,34 +4,44 @@
 
 namespace Rector\CodeQuality\NodeAnalyzer;
 
-use PhpParser\Node;
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\ArrayDimFetch;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Expression;
+use PhpParser\Node\Stmt\Return_;
 use Rector\CodeQuality\ValueObject\KeyAndExpr;
-use Rector\PhpParser\Comparing\NodeComparator;
+use Rector\NodeNameResolver\NodeNameResolver;
 use Rector\PhpParser\Node\BetterNodeFinder;
+use Rector\PhpParser\Node\Value\ValueResolver;
 
 final readonly class VariableDimFetchAssignResolver
 {
     public function __construct(
-        private NodeComparator $nodeComparator,
-        private BetterNodeFinder $betterNodeFinder
+        private ValueResolver $valueResolver
     ) {
     }
 
     /**
      * @param Stmt[] $stmts
-     * @return KeyAndExpr[]
+     * @return array<mixed, KeyAndExpr[]>
      */
-    public function resolveFromStmtsAndVariable(array $stmts, Variable $variable): array
+    public function resolveFromStmtsAndVariable(array $stmts, ?Assign $emptyArrayAssign): array
     {
-        $keysAndExprs = [];
+        $exprs = [];
+
+        $key = 0;
 
         foreach ($stmts as $stmt) {
+            if ($stmt instanceof Expression && $stmt->expr === $emptyArrayAssign) {
+                continue;
+            }
+
+            if ($stmt instanceof Return_) {
+                continue;
+            }
+
             if (! $stmt instanceof Expression) {
                 return [];
             }
@@ -43,66 +53,46 @@ public function resolveFromStmtsAndVariable(array $stmts, Variable $variable): a
 
             $assign = $stmtExpr;
 
-            $keyExpr = $this->matchKeyOnArrayDimFetchOfVariable($assign, $variable);
-            if ($assign->var instanceof ArrayDimFetch && $assign->var->var instanceof ArrayDimFetch) {
-                return [];
-            }
-
-            $keysAndExprs[] = new KeyAndExpr($keyExpr, $assign->expr, $stmt->getComments());
-        }
+            $dimValues = [];
 
-        // we can only work with same variable
-        // and exclusively various keys or empty keys
-        if (! $this->hasExclusivelyNullKeyOrFilledKey($keysAndExprs)) {
-            return [];
-        }
+            $arrayDimFetch = $assign->var;
+            while ($arrayDimFetch instanceof ArrayDimFetch) {
+                $dimValues[] = $arrayDimFetch->dim instanceof Expr ? $this->valueResolver->getValue(
+                    $arrayDimFetch->dim
+                ) : $key;
 
-        return $keysAndExprs;
-    }
+                $arrayDimFetch = $arrayDimFetch->var;
+            }
 
-    private function matchKeyOnArrayDimFetchOfVariable(Assign $assign, Variable $variable): ?Expr
-    {
-        if (! $assign->var instanceof ArrayDimFetch) {
-            return null;
-        }
+            ++$key;
 
-        $arrayDimFetch = $assign->var;
-        if (! $this->nodeComparator->areNodesEqual($arrayDimFetch->var, $variable)) {
-            return null;
+            $this->setNestedKeysExpr($exprs, $dimValues, $assign->expr);
         }
 
-        $isFoundInExpr = (bool) $this->betterNodeFinder->findFirst(
-            $assign->expr,
-            fn (Node $subNode): bool => $this->nodeComparator->areNodesEqual($subNode, $variable)
-        );
-
-        if ($isFoundInExpr) {
-            return null;
-        }
+        // we can only work with same variable
+        // and exclusively various keys or empty keys
+        //        if (! $this->hasExclusivelyNullKeyOrFilledKey($exprsByKeys)) {
+        //            return [];
+        //        }
 
-        return $arrayDimFetch->dim;
+        return $exprs;
     }
 
     /**
-     * @param KeyAndExpr[] $keysAndExprs
+     * @param mixed[] $exprsByKeys
+     * @param array<string|int> $keys
      */
-    private function hasExclusivelyNullKeyOrFilledKey(array $keysAndExprs): bool
+    private function setNestedKeysExpr(array &$exprsByKeys, array $keys, Expr $expr): void
     {
-        $alwaysNullKey = true;
-        $alwaysStringKey = true;
-
-        foreach ($keysAndExprs as $keyAndExpr) {
-            if ($keyAndExpr->getKeyExpr() instanceof Expr) {
-                $alwaysNullKey = false;
-            } else {
-                $alwaysStringKey = false;
-            }
-        }
+        $reference = &$exprsByKeys;
+
+        $keys = array_reverse($keys);
 
-        if ($alwaysNullKey) {
-            return true;
+        foreach ($keys as $key) {
+            // create intermediate arrays automatically
+            $reference = &$reference[$key];
         }
 
-        return $alwaysStringKey;
+        $reference = $expr;
     }
 }