From aa9588f261324674cda0554c4b09d13a299b2dcd Mon Sep 17 00:00:00 2001 From: TomasVotruba Date: Tue, 24 Mar 2020 21:25:08 +0100 Subject: [PATCH] [PHP 7.4] Improve TypedProeprtyRector for Doctrine collection --- .../src/DoctrineTypeAnalyzer.php | 47 +++++++++++++++++++ .../src/TypeMapper/UnionTypeMapper.php | 43 +++++++++++++---- .../Rector/Property/TypedPropertyRector.php | 25 ++++++++-- .../DoctrineTypedPropertyRectorTest.php | 36 ++++++++++++++ .../doctrine_collection.php.inc | 43 +++++++++++++++++ .../doctrine_intersect_collection.php.inc | 43 +++++++++++++++++ 6 files changed, 223 insertions(+), 14 deletions(-) create mode 100644 packages/phpstan-static-type-mapper/src/DoctrineTypeAnalyzer.php create mode 100644 rules/php-74/tests/Rector/Property/TypedPropertyRector/DoctrineTypedPropertyRectorTest.php create mode 100644 rules/php-74/tests/Rector/Property/TypedPropertyRector/FixtureDoctrine/doctrine_collection.php.inc create mode 100644 rules/php-74/tests/Rector/Property/TypedPropertyRector/FixtureDoctrine/doctrine_intersect_collection.php.inc diff --git a/packages/phpstan-static-type-mapper/src/DoctrineTypeAnalyzer.php b/packages/phpstan-static-type-mapper/src/DoctrineTypeAnalyzer.php new file mode 100644 index 000000000000..a4b3f29debb7 --- /dev/null +++ b/packages/phpstan-static-type-mapper/src/DoctrineTypeAnalyzer.php @@ -0,0 +1,47 @@ +getTypes() as $unionedType) { + if ($this->isCollectionObjectType($unionedType)) { + $hasDoctrineCollectionType = true; + } + + if ($unionedType instanceof ArrayType) { + $arrayType = $unionedType; + } + } + + if (! $hasDoctrineCollectionType) { + return false; + } + + return $arrayType !== null; + } + + private function isCollectionObjectType(Type $type): bool + { + if (! $type instanceof TypeWithClassName) { + return false; + } + + return $type->getClassName() === 'Doctrine\Common\Collections\Collection'; + } +} diff --git a/packages/phpstan-static-type-mapper/src/TypeMapper/UnionTypeMapper.php b/packages/phpstan-static-type-mapper/src/TypeMapper/UnionTypeMapper.php index 6a72dc752d0c..aa0a4f8b3c0d 100644 --- a/packages/phpstan-static-type-mapper/src/TypeMapper/UnionTypeMapper.php +++ b/packages/phpstan-static-type-mapper/src/TypeMapper/UnionTypeMapper.php @@ -21,6 +21,7 @@ use Rector\Core\Php\PhpVersionProvider; use Rector\Core\ValueObject\PhpVersionFeature; use Rector\PHPStanStaticTypeMapper\Contract\TypeMapperInterface; +use Rector\PHPStanStaticTypeMapper\DoctrineTypeAnalyzer; use Rector\PHPStanStaticTypeMapper\PHPStanStaticTypeMapper; use Rector\PHPStanStaticTypeMapper\TypeAnalyzer\UnionTypeAnalyzer; @@ -41,10 +42,19 @@ final class UnionTypeMapper implements TypeMapperInterface */ private $unionTypeAnalyzer; - public function __construct(PhpVersionProvider $phpVersionProvider, UnionTypeAnalyzer $unionTypeAnalyzer) - { + /** + * @var DoctrineTypeAnalyzer + */ + private $doctrineTypeAnalyzer; + + public function __construct( + PhpVersionProvider $phpVersionProvider, + UnionTypeAnalyzer $unionTypeAnalyzer, + DoctrineTypeAnalyzer $doctrineTypeAnalyzer + ) { $this->phpVersionProvider = $phpVersionProvider; $this->unionTypeAnalyzer = $unionTypeAnalyzer; + $this->doctrineTypeAnalyzer = $doctrineTypeAnalyzer; } /** @@ -210,16 +220,18 @@ private function matchPhpParserUnionType(UnionType $unionType): ?PhpParserUnionT private function resolveCompatibleObjectCandidate(UnionType $unionType): ?string { - foreach ($unionType->getTypes() as $unionedType) { - if (! $unionedType instanceof TypeWithClassName) { - return null; - } + if ($this->doctrineTypeAnalyzer->isDoctrineCollectionWithIterableUnionType($unionType)) { + return 'Doctrine\Common\Collections\Collection'; + } - foreach ($unionType->getTypes() as $nestedUnionedType) { - if (! $nestedUnionedType instanceof TypeWithClassName) { - return null; - } + if (! $this->isUnionTypeWithTypeClassNameOnly($unionType)) { + return null; + } + /** @var TypeWithClassName $unionedType */ + foreach ($unionType->getTypes() as $unionedType) { + /** @var TypeWithClassName $nestedUnionedType */ + foreach ($unionType->getTypes() as $nestedUnionedType) { if (! $this->areTypeWithClassNamesRelated($unionedType, $nestedUnionedType)) { continue 2; } @@ -249,4 +261,15 @@ private function shouldSkipIterable(UnionType $unionType): bool return $unionTypeAnalysis->hasIterable() && $unionTypeAnalysis->hasArray(); } + + private function isUnionTypeWithTypeClassNameOnly(UnionType $unionType): bool + { + foreach ($unionType->getTypes() as $unionedType) { + if (! $unionedType instanceof TypeWithClassName) { + return false; + } + } + + return true; + } } diff --git a/rules/php-74/src/Rector/Property/TypedPropertyRector.php b/rules/php-74/src/Rector/Property/TypedPropertyRector.php index f078f310af4f..f5e4e027a62b 100644 --- a/rules/php-74/src/Rector/Property/TypedPropertyRector.php +++ b/rules/php-74/src/Rector/Property/TypedPropertyRector.php @@ -10,11 +10,13 @@ use PHPStan\PhpDocParser\Ast\Type\ArrayTypeNode; use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode; use PHPStan\Type\MixedType; +use PHPStan\Type\Type; use Rector\Core\Rector\AbstractRector; use Rector\Core\RectorDefinition\CodeSample; use Rector\Core\RectorDefinition\RectorDefinition; use Rector\Core\ValueObject\PhpVersionFeature; use Rector\NodeTypeResolver\Node\AttributeKey; +use Rector\PHPStanStaticTypeMapper\DoctrineTypeAnalyzer; use Rector\PHPStanStaticTypeMapper\PHPStanStaticTypeMapper; use Rector\TypeDeclaration\TypeInferer\PropertyTypeInferer; use Rector\VendorLocker\VendorLockResolver; @@ -36,10 +38,19 @@ final class TypedPropertyRector extends AbstractRector */ private $vendorLockResolver; - public function __construct(PropertyTypeInferer $propertyTypeInferer, VendorLockResolver $vendorLockResolver) - { + /** + * @var DoctrineTypeAnalyzer + */ + private $doctrineTypeAnalyzer; + + public function __construct( + PropertyTypeInferer $propertyTypeInferer, + VendorLockResolver $vendorLockResolver, + DoctrineTypeAnalyzer $doctrineTypeAnalyzer + ) { $this->propertyTypeInferer = $propertyTypeInferer; $this->vendorLockResolver = $vendorLockResolver; + $this->doctrineTypeAnalyzer = $doctrineTypeAnalyzer; } public function getDefinition(): RectorDefinition @@ -100,6 +111,7 @@ public function refactor(Node $node): ?Node $varType, PHPStanStaticTypeMapper::KIND_PROPERTY ); + if ($propertyTypeNode === null) { return null; } @@ -108,15 +120,20 @@ public function refactor(Node $node): ?Node return null; } - $this->removeVarPhpTagValueNodeIfNotComment($node); + $this->removeVarPhpTagValueNodeIfNotComment($node, $varType); $node->type = $propertyTypeNode; return $node; } - private function removeVarPhpTagValueNodeIfNotComment(Property $property): void + private function removeVarPhpTagValueNodeIfNotComment(Property $property, Type $type): void { + // keep doctrine collection narrow type + if ($this->doctrineTypeAnalyzer->isDoctrineCollectionWithIterableUnionType($type)) { + return; + } + $propertyPhpDocInfo = $property->getAttribute(AttributeKey::PHP_DOC_INFO); // nothing to remove if ($propertyPhpDocInfo === null) { diff --git a/rules/php-74/tests/Rector/Property/TypedPropertyRector/DoctrineTypedPropertyRectorTest.php b/rules/php-74/tests/Rector/Property/TypedPropertyRector/DoctrineTypedPropertyRectorTest.php new file mode 100644 index 000000000000..ea2d36aa7569 --- /dev/null +++ b/rules/php-74/tests/Rector/Property/TypedPropertyRector/DoctrineTypedPropertyRectorTest.php @@ -0,0 +1,36 @@ +doTestFile($file); + } + + public function provideData(): Iterator + { + return $this->yieldFilesFromDirectory(__DIR__ . '/FixtureDoctrine'); + } + + protected function getRectorClass(): string + { + return TypedPropertyRector::class; + } + + protected function getPhpVersion(): string + { + // before union type + return '7.4'; + } +} diff --git a/rules/php-74/tests/Rector/Property/TypedPropertyRector/FixtureDoctrine/doctrine_collection.php.inc b/rules/php-74/tests/Rector/Property/TypedPropertyRector/FixtureDoctrine/doctrine_collection.php.inc new file mode 100644 index 000000000000..93e37ad53c6d --- /dev/null +++ b/rules/php-74/tests/Rector/Property/TypedPropertyRector/FixtureDoctrine/doctrine_collection.php.inc @@ -0,0 +1,43 @@ + +----- + diff --git a/rules/php-74/tests/Rector/Property/TypedPropertyRector/FixtureDoctrine/doctrine_intersect_collection.php.inc b/rules/php-74/tests/Rector/Property/TypedPropertyRector/FixtureDoctrine/doctrine_intersect_collection.php.inc new file mode 100644 index 000000000000..03b42711201f --- /dev/null +++ b/rules/php-74/tests/Rector/Property/TypedPropertyRector/FixtureDoctrine/doctrine_intersect_collection.php.inc @@ -0,0 +1,43 @@ + +----- +