From 1ea9bd1fe60d196ba8d60d96ec0df0f7374d1d5f Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 23 Oct 2019 14:34:20 +0200 Subject: [PATCH] fix union too many types --- .../NodeTypeResolver/src/StaticTypeMapper.php | 15 +- .../AddArrayReturnDocTypeRector.php | 23 ++- .../src/TypeInferer/ReturnTypeInferer.php | 43 +---- .../TypeDeclaration/src/TypeNormalizer.php | 174 ++++++++++++++++++ .../NestedArrayTypeValueObject.php | 36 ++++ .../AddArrayReturnDocTypeRectorTest.php | 1 + .../Fixture/skip_too_many_2.php.inc | 66 +++++++ .../tests/TypeNormalizerTest.php | 62 +++++++ phpstan.neon | 1 + 9 files changed, 373 insertions(+), 48 deletions(-) create mode 100644 packages/TypeDeclaration/src/TypeNormalizer.php create mode 100644 packages/TypeDeclaration/src/ValueObject/NestedArrayTypeValueObject.php create mode 100644 packages/TypeDeclaration/tests/Rector/ClassMethod/AddArrayReturnDocTypeRector/Fixture/skip_too_many_2.php.inc create mode 100644 packages/TypeDeclaration/tests/TypeNormalizerTest.php diff --git a/packages/NodeTypeResolver/src/StaticTypeMapper.php b/packages/NodeTypeResolver/src/StaticTypeMapper.php index 7e438dc14a9a..09380ae55f4e 100644 --- a/packages/NodeTypeResolver/src/StaticTypeMapper.php +++ b/packages/NodeTypeResolver/src/StaticTypeMapper.php @@ -27,6 +27,7 @@ use PHPStan\Type\BooleanType; use PHPStan\Type\CallableType; use PHPStan\Type\ClosureType; +use PHPStan\Type\ConstantType; use PHPStan\Type\FloatType; use PHPStan\Type\IntegerType; use PHPStan\Type\IntersectionType; @@ -480,17 +481,15 @@ public function createTypeHash(Type $type): string return $type->getClassName(); } - return $this->mapPHPStanTypeToDocString($type); - } + if ($type instanceof ConstantType) { + if (method_exists($type, 'getValue')) { + return get_class($type) . $type->getValue(); + } - public function mapStringToPHPStanType(string $newSimpleType): Type - { - $phpParserNode = $this->mapStringToPhpParserNode($newSimpleType); - if ($phpParserNode === null) { - return new MixedType(); + throw new ShouldNotHappenException(); } - return $this->mapPhpParserNodePHPStanType($phpParserNode); + return $this->mapPHPStanTypeToDocString($type); } /** diff --git a/packages/TypeDeclaration/src/Rector/ClassMethod/AddArrayReturnDocTypeRector.php b/packages/TypeDeclaration/src/Rector/ClassMethod/AddArrayReturnDocTypeRector.php index 697b3c2c3374..5c13d06b684e 100644 --- a/packages/TypeDeclaration/src/Rector/ClassMethod/AddArrayReturnDocTypeRector.php +++ b/packages/TypeDeclaration/src/Rector/ClassMethod/AddArrayReturnDocTypeRector.php @@ -127,16 +127,14 @@ private function shouldSkip(ClassMethod $classMethod): bool private function shouldSkipType(Type $newType, ClassMethod $classMethod): bool { - if (! $newType instanceof ArrayType && ! $newType instanceof UnionType) { - return true; - } - if ($newType instanceof ArrayType) { - if ($this->isNewAndCurrentTypeBothCallable($newType, $classMethod)) { + if ($this->shouldSkipArrayType($newType, $classMethod)) { return true; } + } - if ($this->isMixedOfSpecificOverride($newType, $classMethod)) { + if ($newType instanceof UnionType) { + if (count($newType->getTypes()) > self::MAX_NUMBER_OF_TYPES) { return true; } } @@ -191,4 +189,17 @@ private function isMixedOfSpecificOverride(ArrayType $arrayType, ClassMethod $cl return true; } + + private function shouldSkipArrayType(ArrayType $arrayType, ClassMethod $classMethod): bool + { + if ($this->isNewAndCurrentTypeBothCallable($arrayType, $classMethod)) { + return true; + } + + if ($this->isMixedOfSpecificOverride($arrayType, $classMethod)) { + return true; + } + + return false; + } } diff --git a/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer.php b/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer.php index 206681e1409c..5cd9e502885d 100644 --- a/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer.php +++ b/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer.php @@ -5,14 +5,11 @@ namespace Rector\TypeDeclaration\TypeInferer; use PhpParser\Node\FunctionLike; -use PHPStan\Type\ArrayType; use PHPStan\Type\MixedType; -use PHPStan\Type\NeverType; use PHPStan\Type\Type; -use PHPStan\Type\UnionType; use Rector\Exception\ShouldNotHappenException; -use Rector\NodeTypeResolver\PHPStan\Type\TypeFactory; use Rector\TypeDeclaration\Contract\TypeInferer\ReturnTypeInfererInterface; +use Rector\TypeDeclaration\TypeNormalizer; final class ReturnTypeInferer extends AbstractPriorityAwareTypeInferer { @@ -22,17 +19,17 @@ final class ReturnTypeInferer extends AbstractPriorityAwareTypeInferer private $returnTypeInferers = []; /** - * @var TypeFactory + * @var TypeNormalizer */ - private $typeFactory; + private $typeNormalizer; /** * @param ReturnTypeInfererInterface[] $returnTypeInferers */ - public function __construct(TypeFactory $typeFactory, array $returnTypeInferers) + public function __construct(array $returnTypeInferers, TypeNormalizer $typeNormalizer) { $this->returnTypeInferers = $this->sortTypeInferersByPriority($returnTypeInferers); - $this->typeFactory = $typeFactory; + $this->typeNormalizer = $typeNormalizer; } public function inferFunctionLike(FunctionLike $functionLike): Type @@ -51,7 +48,10 @@ public function inferFunctionLikeWithExcludedInferers(FunctionLike $functionLike } $type = $returnTypeInferer->inferFunctionLike($functionLike); - $type = $this->normalizeArrayTypeAndArrayNever($type); + + $type = $this->typeNormalizer->normalizeArrayTypeAndArrayNever($type); + $type = $this->typeNormalizer->uniqueateConstantArrayType($type); + $type = $this->typeNormalizer->normalizeArrayOfUnionToUnionArray($type); if (! $type instanceof MixedType) { return $type; @@ -87,29 +87,4 @@ private function ensureIsTypeInferer(string $excludedInferer): void throw new ShouldNotHappenException(); } - - /** - * From "string[]|mixed[]" based on empty array to to "string[]" - */ - private function normalizeArrayTypeAndArrayNever(Type $type): Type - { - if (! $type instanceof UnionType) { - return $type; - } - - $nonNeverTypes = []; - foreach ($type->getTypes() as $unionedType) { - if (! $unionedType instanceof ArrayType) { - return $type; - } - - if ($unionedType->getItemType() instanceof NeverType) { - continue; - } - - $nonNeverTypes[] = $unionedType; - } - - return $this->typeFactory->createMixedPassedOrUnionType($nonNeverTypes); - } } diff --git a/packages/TypeDeclaration/src/TypeNormalizer.php b/packages/TypeDeclaration/src/TypeNormalizer.php new file mode 100644 index 000000000000..d7b1ec1d3193 --- /dev/null +++ b/packages/TypeDeclaration/src/TypeNormalizer.php @@ -0,0 +1,174 @@ +staticTypeMapper = $staticTypeMapper; + $this->typeFactory = $typeFactory; + } + + /** + * Turn nested array union types to unique ones: + * e.g. int[]|string[][]|bool[][]|string[][] + * ↓ + * int[]|string[][]|bool[][] + */ + public function normalizeArrayOfUnionToUnionArray(Type $type, int $arrayNesting = 1): Type + { + if (! $type instanceof ArrayType) { + return $type; + } + + // first collection of types + if ($arrayNesting === 1) { + $this->collectedNestedArrayTypes = []; + } + + if ($type->getItemType() instanceof ArrayType) { + ++$arrayNesting; + $this->normalizeArrayOfUnionToUnionArray($type->getItemType(), $arrayNesting); + } elseif ($type->getItemType() instanceof UnionType) { + $this->collectNestedArrayTypeFromUnionType($type->getItemType(), $arrayNesting); + } else { + $this->collectedNestedArrayTypes[] = new NestedArrayTypeValueObject( + $type->getItemType(), + $arrayNesting + ); + } + + return $this->createUnionedTypesFromArrayTypes($this->collectedNestedArrayTypes); + } + + public function uniqueateConstantArrayType(Type $type): Type + { + if (! $type instanceof ConstantArrayType) { + return $type; + } + + // nothing to normalize + if ($type->getValueTypes() === []) { + return $type; + } + + $uniqueTypes = []; + $removedKeys = []; + foreach ($type->getValueTypes() as $key => $valueType) { + $typeHash = $this->staticTypeMapper->createTypeHash($valueType); + + $valueType = $this->uniqueateConstantArrayType($valueType); + $valueType = $this->normalizeArrayOfUnionToUnionArray($valueType); + + if (! isset($uniqueTypes[$typeHash])) { + $uniqueTypes[$typeHash] = $valueType; + } else { + $removedKeys[] = $key; + } + } + + // re-index keys + $uniqueTypes = array_values($uniqueTypes); + + $keyTypes = []; + foreach ($type->getKeyTypes() as $key => $keyType) { + if (in_array($key, $removedKeys, true)) { + // remove it + continue; + } + + $keyTypes[$key] = $keyType; + } + + return new ConstantArrayType($keyTypes, $uniqueTypes); + } + + /** + * From "string[]|mixed[]" based on empty array to to "string[]" + */ + public function normalizeArrayTypeAndArrayNever(Type $type): Type + { + if (! $type instanceof UnionType) { + return $type; + } + + $nonNeverTypes = []; + foreach ($type->getTypes() as $unionedType) { + if (! $unionedType instanceof ArrayType) { + return $type; + } + + if ($unionedType->getItemType() instanceof NeverType) { + continue; + } + + $nonNeverTypes[] = $unionedType; + } + + return $this->typeFactory->createMixedPassedOrUnionType($nonNeverTypes); + } + + /** + * @param NestedArrayTypeValueObject[] $collectedNestedArrayTypes + */ + private function createUnionedTypesFromArrayTypes(array $collectedNestedArrayTypes): Type + { + $unionedTypes = []; + foreach ($collectedNestedArrayTypes as $collectedNestedArrayType) { + $arrayType = $collectedNestedArrayType->getType(); + for ($i = 0; $i < $collectedNestedArrayType->getArrayNestingLevel(); ++$i) { + $arrayType = new ArrayType(new MixedType(), $arrayType); + } + + /** @var ArrayType $arrayType */ + $unionedTypes[] = $arrayType; + } + + if (count($unionedTypes) > 1) { + return TypeFactoryStaticHelper::createUnionObjectType($unionedTypes); + } + + return $unionedTypes[0]; + } + + private function collectNestedArrayTypeFromUnionType(UnionType $unionType, int $arrayNesting): void + { + foreach ($unionType->getTypes() as $unionedType) { + if ($unionedType instanceof ArrayType) { + ++$arrayNesting; + $this->normalizeArrayOfUnionToUnionArray($unionedType, $arrayNesting); + } else { + $this->collectedNestedArrayTypes[] = new NestedArrayTypeValueObject($unionedType, $arrayNesting); + } + } + } +} diff --git a/packages/TypeDeclaration/src/ValueObject/NestedArrayTypeValueObject.php b/packages/TypeDeclaration/src/ValueObject/NestedArrayTypeValueObject.php new file mode 100644 index 000000000000..96b989369214 --- /dev/null +++ b/packages/TypeDeclaration/src/ValueObject/NestedArrayTypeValueObject.php @@ -0,0 +1,36 @@ +type = $type; + $this->arrayNestingLevel = $arrayNestingLevel; + } + + public function getType(): Type + { + return $this->type; + } + + public function getArrayNestingLevel(): int + { + return $this->arrayNestingLevel; + } +} diff --git a/packages/TypeDeclaration/tests/Rector/ClassMethod/AddArrayReturnDocTypeRector/AddArrayReturnDocTypeRectorTest.php b/packages/TypeDeclaration/tests/Rector/ClassMethod/AddArrayReturnDocTypeRector/AddArrayReturnDocTypeRectorTest.php index 14cf4ee92bd0..92d3608823a6 100644 --- a/packages/TypeDeclaration/tests/Rector/ClassMethod/AddArrayReturnDocTypeRector/AddArrayReturnDocTypeRectorTest.php +++ b/packages/TypeDeclaration/tests/Rector/ClassMethod/AddArrayReturnDocTypeRector/AddArrayReturnDocTypeRectorTest.php @@ -32,6 +32,7 @@ public function provideDataForTest(): Iterator // skip yield [__DIR__ . '/Fixture/skip_too_many.php.inc']; + yield [__DIR__ . '/Fixture/skip_too_many_2.php.inc']; yield [__DIR__ . '/Fixture/skip_mixed_of_specific_override.php.inc']; yield [__DIR__ . '/Fixture/skip_closure_callable_override.php.inc']; yield [__DIR__ . '/Fixture/skip_shorten_class_name.php.inc']; diff --git a/packages/TypeDeclaration/tests/Rector/ClassMethod/AddArrayReturnDocTypeRector/Fixture/skip_too_many_2.php.inc b/packages/TypeDeclaration/tests/Rector/ClassMethod/AddArrayReturnDocTypeRector/Fixture/skip_too_many_2.php.inc new file mode 100644 index 000000000000..2d5815b38aff --- /dev/null +++ b/packages/TypeDeclaration/tests/Rector/ClassMethod/AddArrayReturnDocTypeRector/Fixture/skip_too_many_2.php.inc @@ -0,0 +1,66 @@ +createPackageKey($packageName); + + $packageDownloads = $this->provideForPackage($packageName); + if ($packageDownloads === []) { + continue; + } + + // complete relative number of downloads + $totalDownloads = array_sum($packageDownloads[1]); + + foreach ($packageDownloads[1] as $version => $absoluteDownloads) { + $relativeRate = 100 * ($absoluteDownloads / $totalDownloads); + + $packageDownloads[1][$version] = [ + 'absolute_downloads' => $absoluteDownloads, + 'relative_downloads' => round($relativeRate, 1), + 'version_publish_date' => $this->provideForPackageAndVersion( + $packageName, + $version + ), + ]; + } + + $packagesData[$packageKey] = [ + 'package_name' => $packageName, + 'package_short_name' => Strings::after($packageName, '/'), + ] + $packageDownloads; + } + + return $packagesData; + } + + private function createPackageKey(string $packageName): string + { + return Strings::replace($packageName, '#(/|-)#', '_'); + } + + /** + * @return int[][] + */ + private function provideForPackage(string $packageName): array + { + return [[1, 3]]; + } + + private function provideForPackageAndVersion(): ?string + { + return '' ? null : 'string'; + } +} diff --git a/packages/TypeDeclaration/tests/TypeNormalizerTest.php b/packages/TypeDeclaration/tests/TypeNormalizerTest.php new file mode 100644 index 000000000000..fe6a93cc8280 --- /dev/null +++ b/packages/TypeDeclaration/tests/TypeNormalizerTest.php @@ -0,0 +1,62 @@ +typeNormalizer = self::$container->get(TypeNormalizer::class); + $this->staticTypeMapper = self::$container->get(StaticTypeMapper::class); + } + + /** + * @dataProvider provideDataForTestNormalizeArrayOfUnionToUnionArray() + */ + public function testNormalizeArrayOfUnionToUnionArray(ArrayType $arrayType, string $expectedDocString): void + { + $arrayDocString = $this->staticTypeMapper->mapPHPStanTypeToDocString($arrayType); + $this->assertSame($expectedDocString, $arrayDocString); + + $unionType = $this->typeNormalizer->normalizeArrayOfUnionToUnionArray($arrayType); + $this->assertInstanceOf(UnionType::class, $unionType); + + $unionDocString = $this->staticTypeMapper->mapPHPStanTypeToDocString($unionType); + $this->assertSame($expectedDocString, $unionDocString); + } + + public function provideDataForTestNormalizeArrayOfUnionToUnionArray(): Iterator + { + $arrayType = new ArrayType(new MixedType(), new UnionType([new StringType(), new IntegerType()])); + yield [$arrayType, 'int[]|string[]']; + + $arrayType = new ArrayType(new MixedType(), new UnionType([new StringType(), new IntegerType()])); + $moreNestedArrayType = new ArrayType(new MixedType(), $arrayType); + yield [$moreNestedArrayType, 'int[][]|string[][]']; + } +} diff --git a/phpstan.neon b/phpstan.neon index 54f39eb175d5..f8d64f438b3c 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -209,3 +209,4 @@ parameters: - '#PHPDoc tag @param for parameter \$symfonyStyle with type array is incompatible with native type Symfony\\Component\\Console\\Style\\SymfonyStyle#' - '#In method "Rector\\BetterPhpDocParser\\AnnotationReader\\NodeAnnotationReader\:\:createPropertyReflectionFromPropertyNode", caught "Throwable" must be rethrown\. Either catch a more specific exception or add a "throw" clause in the "catch" block to propagate the exception\. More info\: http\://bit\.ly/failloud#' + - '#Method Rector\\TypeDeclaration\\TypeNormalizer\:\:createUnionedTypesFromArrayTypes\(\) should return array but returns array#'