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

TomasVotruba · TomasVotruba · commit 94924c850cca · 2025-09-14T21:26:35.000+02:00
diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/include_nested_variable.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/include_nested_variable.php.inc
@@ -0,0 +1,32 @@
+<?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]
+        ];
+    }
+}
+
+?>
diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_different_variable.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_different_variable.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;
+    }
+}
diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_multiple_returns.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_multiple_returns.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;
+    }
+}
diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/some_object.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/some_object.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];
+    }
+}
+
+?>
diff --git a/rules/CodeQuality/NodeAnalyzer/VariableDimFetchAssignResolver.php b/rules/CodeQuality/NodeAnalyzer/VariableDimFetchAssignResolver.php
@@ -11,27 +11,36 @@
 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;
 
 final readonly class VariableDimFetchAssignResolver
 {
     public function __construct(
-        private NodeComparator $nodeComparator,
-        private BetterNodeFinder $betterNodeFinder
+        private BetterNodeFinder $betterNodeFinder,
+        private NodeNameResolver $nodeNameResolver
     ) {
     }
 
     /**
      * @param Stmt[] $stmts
      * @return KeyAndExpr[]
      */
-    public function resolveFromStmtsAndVariable(array $stmts, Variable $variable): array
+    public function resolveFromStmtsAndVariable(array $stmts, ?Assign $emptyArrayAssign, string $variableName): array
     {
         $keysAndExprs = [];
 
         foreach ($stmts as $stmt) {
+            if ($stmt instanceof Expression && $stmt->expr === $emptyArrayAssign) {
+                continue;
+            }
+
+            if ($stmt instanceof Return_) {
+                continue;
+            }
+
             if (! $stmt instanceof Expression) {
                 return [];
             }
@@ -43,7 +52,7 @@ public function resolveFromStmtsAndVariable(array $stmts, Variable $variable): a
 
             $assign = $stmtExpr;
 
-            $keyExpr = $this->matchKeyOnArrayDimFetchOfVariable($assign, $variable);
+            $keyExpr = $this->matchKeyOnArrayDimFetchOfVariable($assign, $variableName);
             if ($assign->var instanceof ArrayDimFetch && $assign->var->var instanceof ArrayDimFetch) {
                 return [];
             }
@@ -60,20 +69,20 @@ public function resolveFromStmtsAndVariable(array $stmts, Variable $variable): a
         return $keysAndExprs;
     }
 
-    private function matchKeyOnArrayDimFetchOfVariable(Assign $assign, Variable $variable): ?Expr
+    private function matchKeyOnArrayDimFetchOfVariable(Assign $assign, string $variableName): ?Expr
     {
         if (! $assign->var instanceof ArrayDimFetch) {
             return null;
         }
 
         $arrayDimFetch = $assign->var;
-        if (! $this->nodeComparator->areNodesEqual($arrayDimFetch->var, $variable)) {
+        if (! $this->nodeNameResolver->isName($arrayDimFetch->var, $variableName)) {
             return null;
         }
 
         $isFoundInExpr = (bool) $this->betterNodeFinder->findFirst(
             $assign->expr,
-            fn (Node $subNode): bool => $this->nodeComparator->areNodesEqual($subNode, $variable)
+            fn (Node $subNode): bool => $this->nodeNameResolver->isName($subNode, $variableName)
         );
 
         if ($isFoundInExpr) {
diff --git a/rules/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector.php b/rules/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector.php
@@ -18,7 +18,6 @@
 use Rector\CodeQuality\ValueObject\KeyAndExpr;
 use Rector\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\NodeTypeResolver\Node\AttributeKey;
-use Rector\PhpParser\Node\Value\ValueResolver;
 use Rector\Rector\AbstractRector;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -30,7 +29,6 @@ final class InlineArrayReturnAssignRector extends AbstractRector
 {
     public function __construct(
         private readonly VariableDimFetchAssignResolver $variableDimFetchAssignResolver,
-        private readonly ValueResolver $valueResolver
     ) {
     }
 
@@ -81,60 +79,46 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        $firstStmt = array_shift($stmts);
-        $variable = $this->matchVariableAssignOfEmptyArray($firstStmt);
-        if (! $variable instanceof Variable) {
+        $lastStmt = $node->stmts[count($stmts) - 1] ?? null;
+        if (! $lastStmt instanceof Return_) {
             return null;
         }
 
-        if (! $this->areAssignExclusiveToDimFetch($stmts)) {
+        if (! $lastStmt->expr instanceof Variable) {
             return null;
         }
 
-        $lastStmt = array_pop($stmts);
-        if (! $lastStmt instanceof Stmt) {
+        $returnedVariableName = $this->getName($lastStmt->expr);
+        if (! is_string($returnedVariableName)) {
             return null;
         }
 
-        if (! $this->isReturnOfVariable($lastStmt, $variable)) {
-            return null;
-        }
-
-        $keysAndExprs = $this->variableDimFetchAssignResolver->resolveFromStmtsAndVariable($stmts, $variable);
-        if ($keysAndExprs === []) {
-            return null;
-        }
-
-        $array = $this->createArray($keysAndExprs);
-        $node->stmts = [new Return_($array)];
+        $emptyArrayAssign = $this->resolveDefaultEmptyArrayAssign($stmts, $returnedVariableName);
 
-        return $node;
-    }
-
-    private function matchVariableAssignOfEmptyArray(Stmt $stmt): ?Variable
-    {
-        if (! $stmt instanceof Expression) {
+        if (! $this->areAssignExclusiveToDimFetchVariable($stmts, $emptyArrayAssign)) {
             return null;
         }
 
-        if (! $stmt->expr instanceof Assign) {
+        if (! $this->isReturnOfVariable($lastStmt, $returnedVariableName)) {
             return null;
         }
 
-        $assign = $stmt->expr;
-
-        if (! $this->valueResolver->isValue($assign->expr, [])) {
+        $keysAndExprs = $this->variableDimFetchAssignResolver->resolveFromStmtsAndVariable(
+            $stmts,
+            $emptyArrayAssign,
+            $returnedVariableName
+        );
+        if ($keysAndExprs === []) {
             return null;
         }
 
-        if (! $assign->var instanceof Variable) {
-            return null;
-        }
+        $array = $this->createArray($keysAndExprs);
+        $node->stmts = [new Return_($array)];
 
-        return $assign->var;
+        return $node;
     }
 
-    private function isReturnOfVariable(Stmt $stmt, Variable $variable): bool
+    private function isReturnOfVariable(Stmt $stmt, string $variableName): bool
     {
         if (! $stmt instanceof Return_) {
             return false;
@@ -144,7 +128,7 @@ private function isReturnOfVariable(Stmt $stmt, Variable $variable): bool
             return false;
         }
 
-        return $this->nodeComparator->areNodesEqual($stmt->expr, $variable);
+        return $this->isName($stmt->expr, $variableName);
     }
 
     /**
@@ -169,7 +153,7 @@ private function createArray(array $keysAndExprs): Array_
      *
      * @param Stmt[] $stmts
      */
-    private function areAssignExclusiveToDimFetch(array $stmts): bool
+    private function areAssignExclusiveToDimFetchVariable(array $stmts, ?Assign $emptyArrayAssign): bool
     {
         $lastKey = array_key_last($stmts);
 
@@ -189,6 +173,16 @@ private function areAssignExclusiveToDimFetch(array $stmts): bool
 
             $assign = $stmt->expr;
 
+            // skip initial assign
+            if ($assign === $emptyArrayAssign) {
+                continue;
+            }
+
+            // instance of empty array in first line is ok
+            if ($key === 0 && $assign->expr instanceof Array_) {
+                continue;
+            }
+
             // skip new X instance with args to keep complex assign readable
             if ($assign->expr instanceof New_ && ! $assign->expr->isFirstClassCallable() && $assign->expr->getArgs() !== []) {
                 return false;
@@ -197,8 +191,46 @@ private function areAssignExclusiveToDimFetch(array $stmts): bool
             if (! $assign->var instanceof ArrayDimFetch) {
                 return false;
             }
+
+            $arrayDimFetch = $assign->var;
+            if (! $arrayDimFetch->var instanceof Variable) {
+                return false;
+            }
         }
 
         return true;
     }
+
+    /**
+     * @param Expression[] $stmts
+     */
+    private function resolveDefaultEmptyArrayAssign(array $stmts, string $returnedVariableName): ?Assign
+    {
+        foreach ($stmts as $stmt) {
+            if (! $stmt instanceof Expression) {
+                continue;
+            }
+
+            if (! $stmt->expr instanceof Assign) {
+                continue;
+            }
+
+            $assign = $stmt->expr;
+            if (! $assign->var instanceof Variable) {
+                continue;
+            }
+
+            if (! $this->isName($assign->var, $returnedVariableName)) {
+                continue;
+            }
+
+            if (! $assign->expr instanceof Array_) {
+                continue;
+            }
+
+            return $assign;
+        }
+
+        return null;
+    }
 }
diff --git a/rules/Privatization/TypeManipulator/TypeNormalizer.php b/rules/Privatization/TypeManipulator/TypeNormalizer.php
@@ -25,7 +25,7 @@ final class TypeNormalizer
      * @deprecated This method is deprecated and will be removed in the next major release.
      * Use @see generalizeConstantTypes() instead.
      */
-    public function generalizeConstantBoolTypes(\PHPStan\Type\Type $type): Type
+    public function generalizeConstantBoolTypes(Type $type): Type
     {
         return $this->generalizeConstantTypes($type);
     }
diff --git a/rules/TypeDeclarationDocblocks/Rector/Class_/AddReturnDocblockDataProviderRector.php b/rules/TypeDeclarationDocblocks/Rector/Class_/AddReturnDocblockDataProviderRector.php
@@ -154,11 +154,9 @@ public function refactor(Node $node): ?Node
             if ($yields !== []) {
                 $yieldType = $this->yieldTypeResolver->resolveFromYieldNodes($yields, $dataProviderClassMethod);
 
-                if ($yieldType instanceof FullyQualifiedGenericObjectType) {
-                    if ($yieldType->getClassName() === 'Generator') {
-                        // most likely, a static iterator is used in data test fixtures
-                        $yieldType = new FullyQualifiedGenericObjectType('Iterator', $yieldType->getTypes());
-                    }
+                if ($yieldType instanceof FullyQualifiedGenericObjectType && $yieldType->getClassName() === 'Generator') {
+                    // most likely, a static iterator is used in data test fixtures
+                    $yieldType = new FullyQualifiedGenericObjectType('Iterator', $yieldType->getTypes());
                 }
 
                 $this->addGeneratedTypeReturnDocblockType($yieldType, $classMethodPhpDocInfo, $dataProviderClassMethod);

Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ final class TypeNormalizer`
`25`	`25`	`* @deprecated This method is deprecated and will be removed in the next major release.`
`26`	`26`	`* Use @see generalizeConstantTypes() instead.`
`27`	`27`	`*/`
`28`		`- public function generalizeConstantBoolTypes(\PHPStan\Type\Type $type): Type`
	`28`	`+ public function generalizeConstantBoolTypes(Type $type): Type`
`29`	`29`	`{`
`30`	`30`	`return $this->generalizeConstantTypes($type);`
`31`	`31`	`}`