Make ReturnTypeFromStrictNewArrayRector handle only single level arra…

…ys (#4575)
rectorphp · Jul 21, 2023 · 4a56168 · 4a56168
1 parent 3ff83e5
commit 4a56168
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 154 deletions.
diff --git a/...ixture/include_simple_array_shape.php.inc → .../Fixture/array_shape_simple_array.php.inc b/...ixture/include_simple_array_shape.php.inc → .../Fixture/array_shape_simple_array.php.inc
@@ -4,15 +4,14 @@ declare(strict_types=1);
 
 namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictNewArrayRector\Fixture;
 
-final class IncludeSimpleArrayShape
+final class ArrayShapeSimpleArray
 {
     public function run()
     {
         $items = [];
 
         $items['one'] = 1;
         $items['two'] = 1;
-        $items['three'] = 1;
 
         return $items;
     }
@@ -26,18 +25,14 @@ declare(strict_types=1);
 
 namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictNewArrayRector\Fixture;
 
-final class IncludeSimpleArrayShape
+final class ArrayShapeSimpleArray
 {
-    /**
-     * @return array{one: int, two: int, three: int}
-     */
     public function run(): array
     {
         $items = [];
 
         $items['one'] = 1;
         $items['two'] = 1;
-        $items['three'] = 1;
 
         return $items;
     }

diff --git a/...ethod/ReturnTypeFromStrictNewArrayRector/Fixture/make_complex_array_shape_simpler.php.inc b/...ethod/ReturnTypeFromStrictNewArrayRector/Fixture/make_complex_array_shape_simpler.php.inc
diff --git a/...eturnTypeFromStrictNewArrayRector/Fixture/make_nested_complex_array_shape_simpler.php.inc b/...eturnTypeFromStrictNewArrayRector/Fixture/make_nested_complex_array_shape_simpler.php.inc
diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector.php
@@ -19,15 +19,12 @@
 use PHPStan\Analyser\Scope;
 use PHPStan\Type\ArrayType;
 use PHPStan\Type\Constant\ConstantArrayType;
+use PHPStan\Type\IntegerType;
 use PHPStan\Type\MixedType;
 use PHPStan\Type\NeverType;
-use PHPStan\Type\ObjectType;
-use PHPStan\Type\Type;
-use PHPStan\Type\VerbosityLevel;
 use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger;
 use Rector\Core\Rector\AbstractScopeAwareRector;
 use Rector\Core\ValueObject\PhpVersion;
-use Rector\NodeTypeResolver\TypeComparator\TypeComparator;
 use Rector\VendorLocker\NodeVendorLocker\ClassMethodReturnTypeOverrideGuard;
 use Rector\VersionBonding\Contract\MinPhpVersionInterface;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -40,7 +37,6 @@ final class ReturnTypeFromStrictNewArrayRector extends AbstractScopeAwareRector
 {
     public function __construct(
         private readonly PhpDocTypeChanger $phpDocTypeChanger,
-        private readonly TypeComparator $typeComparator,
         private readonly ClassMethodReturnTypeOverrideGuard $classMethodReturnTypeOverrideGuard
     ) {
     }
@@ -126,8 +122,7 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
         }
 
         $returnType = $this->nodeTypeResolver->getType($onlyReturn->expr);
-
-        if (! $returnType->isArray()->yes()) {
+        if (! $returnType instanceof ArrayType) {
             return null;
         }
 
@@ -138,11 +133,9 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
         // 3. always returns array
         $node->returnType = new Identifier('array');
 
-        // 4. add more precise type if suitable
-        $exprType = $this->getType($onlyReturn->expr);
-
-        if ($this->shouldAddReturnArrayDocType($exprType)) {
-            $this->changeReturnType($node, $exprType);
+        // 4. add more precise array type if suitable
+        if ($this->shouldAddReturnArrayDocType($returnType)) {
+            $this->changeReturnType($node, $returnType);
         }
 
         return $node;
@@ -165,15 +158,21 @@ private function shouldSkip(ClassMethod|Function_|Closure $node, Scope $scope):
         );
     }
 
-    private function changeReturnType(ClassMethod|Function_|Closure $node, Type $exprType): void
+    private function changeReturnType(ClassMethod|Function_|Closure $node, ArrayType $arrayType): void
     {
         $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
 
-        $exprType = $this->narrowConstantArrayType($exprType);
+        // skip already filled type, on purpose
+        if (! $phpDocInfo->getReturnType() instanceof MixedType) {
+            return;
+        }
 
-        if (! $this->typeComparator->isSubtype($phpDocInfo->getReturnType(), $exprType)) {
-            $this->phpDocTypeChanger->changeReturnType($node, $phpDocInfo, $exprType);
+        // can handle only exactly 1-type array
+        if ($arrayType instanceof ConstantArrayType && count($arrayType->getValueTypes()) !== 1) {
+            return;
         }
+
+        $this->phpDocTypeChanger->changeReturnType($node, $phpDocInfo, $arrayType);
     }
 
     private function isVariableOverriddenWithNonArray(
@@ -229,35 +228,19 @@ private function matchArrayAssignedVariable(array $stmts): Variable|null
         return null;
     }
 
-    private function shouldAddReturnArrayDocType(Type $exprType): bool
+    private function shouldAddReturnArrayDocType(ArrayType $arrayType): bool
     {
-        if ($exprType instanceof ConstantArrayType) {
-            // sign of empty array, keep empty
-            return ! $exprType->getItemType() instanceof NeverType;
-        }
-
-        return $exprType->isArray()
-            ->yes();
-    }
-
-    private function narrowConstantArrayType(Type $type): Type
-    {
-        if (! $type instanceof ConstantArrayType) {
-            return $type;
-        }
-
-        if (count($type->getValueTypes()) === 1) {
-            $singleValueType = $type->getValueTypes()[0];
-            if ($singleValueType instanceof ObjectType) {
-                return $type;
+        if ($arrayType instanceof ConstantArrayType) {
+            if ($arrayType->getItemType() instanceof NeverType) {
+                return false;
             }
-        }
 
-        $printedDescription = $type->describe(VerbosityLevel::precise());
-        if (strlen($printedDescription) > 50) {
-            return new ArrayType(new MixedType(), new MixedType());
+            // handle only simple arrays
+            if (! $arrayType->getKeyType() instanceof IntegerType) {
+                return false;
+            }
         }
 
-        return $type;
+        return true;
     }
 }