From 4e8ff948d83a07b1b99d165cdb448c9b52e2f7af Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Thu, 1 Aug 2019 00:28:31 +0200 Subject: [PATCH] add priority to PropertyTypeInfererInterface and put doctrine infering first --- packages/TypeDeclaration/config/config.yaml | 2 +- .../Contract/PropertyTypeInfererInterface.php | 5 + .../ConflictingPriorityException.php | 26 +++ .../AllAssignNodePropertyTypeInferer.php | 31 ++- .../ConstructorPropertyTypeInferer.php | 5 + .../DefaultValuePropertyTypeInferer.php | 5 + .../DoctrineColumnPropertyTypeInferer.php | 5 + .../DoctrineRelationPropertyTypeInferer.php | 5 + .../GetterOrSetterPropertyTypeInferer.php | 5 + ...eMethodAssignedNodePropertyTypeInferer.php | 5 + .../PropertyTypeDeclarationRector.php | 27 ++- .../Fixture/complex.php.inc | 211 ++++++++++++++++++ .../PropertyTypeDeclarationRectorTest.php | 2 + 13 files changed, 331 insertions(+), 3 deletions(-) create mode 100644 packages/TypeDeclaration/src/Exception/ConflictingPriorityException.php create mode 100644 packages/TypeDeclaration/tests/Rector/Property/PropertyTypeDeclarationRector/Fixture/complex.php.inc diff --git a/packages/TypeDeclaration/config/config.yaml b/packages/TypeDeclaration/config/config.yaml index 4f5479642313..a9d209e93ad6 100644 --- a/packages/TypeDeclaration/config/config.yaml +++ b/packages/TypeDeclaration/config/config.yaml @@ -5,4 +5,4 @@ services: Rector\TypeDeclaration\: resource: '../src/' - exclude: '../src/{Rector/**/*Rector.php}' + exclude: '../src/{Rector/**/*Rector.php,Exception}' diff --git a/packages/TypeDeclaration/src/Contract/PropertyTypeInfererInterface.php b/packages/TypeDeclaration/src/Contract/PropertyTypeInfererInterface.php index 6caaaa84ecc6..f5a5ae0be463 100644 --- a/packages/TypeDeclaration/src/Contract/PropertyTypeInfererInterface.php +++ b/packages/TypeDeclaration/src/Contract/PropertyTypeInfererInterface.php @@ -10,4 +10,9 @@ interface PropertyTypeInfererInterface * @return string[] */ public function inferProperty(Property $property): array; + + /** + * Higher priority goes first. + */ + public function getPriority(): int; } diff --git a/packages/TypeDeclaration/src/Exception/ConflictingPriorityException.php b/packages/TypeDeclaration/src/Exception/ConflictingPriorityException.php new file mode 100644 index 000000000000..b696fc0bfb3c --- /dev/null +++ b/packages/TypeDeclaration/src/Exception/ConflictingPriorityException.php @@ -0,0 +1,26 @@ +getPriority(), + PHP_EOL, + get_class($firstPropertyTypeInfererInterface), + PHP_EOL, + get_class($secondPropertyTypeInfererInterface), + PHP_EOL + ); + + parent::__construct($message); + } +} diff --git a/packages/TypeDeclaration/src/PropertyTypeInferer/AllAssignNodePropertyTypeInferer.php b/packages/TypeDeclaration/src/PropertyTypeInferer/AllAssignNodePropertyTypeInferer.php index 20870cfc0969..b070b520b529 100644 --- a/packages/TypeDeclaration/src/PropertyTypeInferer/AllAssignNodePropertyTypeInferer.php +++ b/packages/TypeDeclaration/src/PropertyTypeInferer/AllAssignNodePropertyTypeInferer.php @@ -8,6 +8,7 @@ use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Stmt\ClassLike; use PHPStan\Type\ArrayType; +use PHPStan\Type\ErrorType; use PHPStan\Type\IntersectionType; use PHPStan\Type\MixedType; use PHPStan\Type\Type; @@ -36,6 +37,11 @@ public function inferProperty(Node\Stmt\Property $property): array return $this->staticTypeToStringResolver->resolveObjectType($assignedExprStaticType); } + public function getPriority(): int + { + return 500; + } + /** * @return Type[] */ @@ -61,6 +67,10 @@ private function collectAllPropertyAsignExprStaticTypes(ClassLike $classLike, st return null; } + if ($exprStaticType instanceof ErrorType) { + return null; + } + if ($node->var instanceof ArrayDimFetch) { $exprStaticType = new ArrayType(new MixedType(), $exprStaticType); } @@ -70,7 +80,7 @@ private function collectAllPropertyAsignExprStaticTypes(ClassLike $classLike, st return null; }); - return $assignedExprStaticTypes; + return $this->filterOutDuplicatedTypes($assignedExprStaticTypes); } /** @@ -98,4 +108,23 @@ private function matchPropertyAssignExpr(Assign $assign, string $propertyName): return null; } + + /** + * @param Type[] $types + * @return Type[] + */ + private function filterOutDuplicatedTypes(array $types): array + { + if (count($types) === 1) { + return $types; + } + + $uniqueTypes = []; + foreach ($types as $type) { + $valueObjectHash = md5(serialize($type)); + $uniqueTypes[$valueObjectHash] = $type; + } + + return $uniqueTypes; + } } diff --git a/packages/TypeDeclaration/src/PropertyTypeInferer/ConstructorPropertyTypeInferer.php b/packages/TypeDeclaration/src/PropertyTypeInferer/ConstructorPropertyTypeInferer.php index 8cc69e3dc9d2..5cf2ff0d779d 100644 --- a/packages/TypeDeclaration/src/PropertyTypeInferer/ConstructorPropertyTypeInferer.php +++ b/packages/TypeDeclaration/src/PropertyTypeInferer/ConstructorPropertyTypeInferer.php @@ -62,6 +62,11 @@ public function inferProperty(Node\Stmt\Property $property): array return []; } + public function getPriority(): int + { + return 800; + } + private function getResolveParamStaticTypeAsString(ClassMethod $classMethod, string $propertyName): ?string { $paramStaticType = $this->resolveParamStaticType($classMethod, $propertyName); diff --git a/packages/TypeDeclaration/src/PropertyTypeInferer/DefaultValuePropertyTypeInferer.php b/packages/TypeDeclaration/src/PropertyTypeInferer/DefaultValuePropertyTypeInferer.php index 3e1dfa378424..0981650b9138 100644 --- a/packages/TypeDeclaration/src/PropertyTypeInferer/DefaultValuePropertyTypeInferer.php +++ b/packages/TypeDeclaration/src/PropertyTypeInferer/DefaultValuePropertyTypeInferer.php @@ -24,4 +24,9 @@ public function inferProperty(Property $property): array return $this->staticTypeToStringResolver->resolveObjectType($nodeStaticType); } + + public function getPriority(): int + { + return 700; + } } diff --git a/packages/TypeDeclaration/src/PropertyTypeInferer/DoctrineColumnPropertyTypeInferer.php b/packages/TypeDeclaration/src/PropertyTypeInferer/DoctrineColumnPropertyTypeInferer.php index 35658a72e9a7..0f7b568bf769 100644 --- a/packages/TypeDeclaration/src/PropertyTypeInferer/DoctrineColumnPropertyTypeInferer.php +++ b/packages/TypeDeclaration/src/PropertyTypeInferer/DoctrineColumnPropertyTypeInferer.php @@ -102,6 +102,11 @@ public function inferProperty(Property $property): array return $types; } + public function getPriority(): int + { + return 1000; + } + private function isNullable(string $value): bool { return (bool) Strings::match($value, '#nullable=true#'); diff --git a/packages/TypeDeclaration/src/PropertyTypeInferer/DoctrineRelationPropertyTypeInferer.php b/packages/TypeDeclaration/src/PropertyTypeInferer/DoctrineRelationPropertyTypeInferer.php index cd690528a9af..1f6ddefa9716 100644 --- a/packages/TypeDeclaration/src/PropertyTypeInferer/DoctrineRelationPropertyTypeInferer.php +++ b/packages/TypeDeclaration/src/PropertyTypeInferer/DoctrineRelationPropertyTypeInferer.php @@ -59,6 +59,11 @@ public function inferProperty(Property $property): array return []; } + public function getPriority(): int + { + return 900; + } + private function resolveTargetEntity(GenericTagValueNode $genericTagValueNode): ?string { $match = Strings::match($genericTagValueNode->value, '#targetEntity=\"(?.*?)\"#'); diff --git a/packages/TypeDeclaration/src/PropertyTypeInferer/GetterOrSetterPropertyTypeInferer.php b/packages/TypeDeclaration/src/PropertyTypeInferer/GetterOrSetterPropertyTypeInferer.php index ecaf7de6d2fa..d11075a46eba 100644 --- a/packages/TypeDeclaration/src/PropertyTypeInferer/GetterOrSetterPropertyTypeInferer.php +++ b/packages/TypeDeclaration/src/PropertyTypeInferer/GetterOrSetterPropertyTypeInferer.php @@ -56,4 +56,9 @@ public function inferProperty(Property $property): array return []; } + + public function getPriority(): int + { + return 600; + } } diff --git a/packages/TypeDeclaration/src/PropertyTypeInferer/SingleMethodAssignedNodePropertyTypeInferer.php b/packages/TypeDeclaration/src/PropertyTypeInferer/SingleMethodAssignedNodePropertyTypeInferer.php index d12de3ace83b..e43b9ae643b5 100644 --- a/packages/TypeDeclaration/src/PropertyTypeInferer/SingleMethodAssignedNodePropertyTypeInferer.php +++ b/packages/TypeDeclaration/src/PropertyTypeInferer/SingleMethodAssignedNodePropertyTypeInferer.php @@ -79,6 +79,11 @@ public function inferProperty(Property $property): array return $stringTypes; } + public function getPriority(): int + { + return 750; + } + private function resolveAssignedNodeToProperty(ClassMethod $classMethod, string $propertyName): ?Expr { $assignedNode = null; diff --git a/packages/TypeDeclaration/src/Rector/Property/PropertyTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/Property/PropertyTypeDeclarationRector.php index eddb5967ae23..4482f37e0714 100644 --- a/packages/TypeDeclaration/src/Rector/Property/PropertyTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/Property/PropertyTypeDeclarationRector.php @@ -10,6 +10,7 @@ use Rector\Rector\AbstractRector; use Rector\RectorDefinition\RectorDefinition; use Rector\TypeDeclaration\Contract\PropertyTypeInfererInterface; +use Rector\TypeDeclaration\Exception\ConflictingPriorityException; final class PropertyTypeDeclarationRector extends AbstractRector { @@ -29,7 +30,8 @@ final class PropertyTypeDeclarationRector extends AbstractRector public function __construct(DocBlockManipulator $docBlockManipulator, array $propertyTypeInferers = []) { $this->docBlockManipulator = $docBlockManipulator; - $this->propertyTypeInferers = $propertyTypeInferers; + + $this->sortAndSetPropertyTypeInferers($propertyTypeInferers); } public function getDefinition(): RectorDefinition @@ -80,4 +82,27 @@ private function setNodeVarTypes(Node $node, array $varTypes): Node return $node; } + + /** + * @param PropertyTypeInfererInterface[] $propertyTypeInferers + */ + private function sortAndSetPropertyTypeInferers(array $propertyTypeInferers): void + { + foreach ($propertyTypeInferers as $propertyTypeInferer) { + $this->ensurePriorityIsUnique($propertyTypeInferer); + $this->propertyTypeInferers[$propertyTypeInferer->getPriority()] = $propertyTypeInferer; + } + + krsort($this->propertyTypeInferers); + } + + private function ensurePriorityIsUnique(PropertyTypeInfererInterface $propertyTypeInferer): void + { + if (! isset($this->propertyTypeInferers[$propertyTypeInferer->getPriority()])) { + return; + } + + $alreadySetPropertyTypeInferer = $this->propertyTypeInferers[$propertyTypeInferer->getPriority()]; + throw new ConflictingPriorityException($propertyTypeInferer, $alreadySetPropertyTypeInferer); + } } diff --git a/packages/TypeDeclaration/tests/Rector/Property/PropertyTypeDeclarationRector/Fixture/complex.php.inc b/packages/TypeDeclaration/tests/Rector/Property/PropertyTypeDeclarationRector/Fixture/complex.php.inc new file mode 100644 index 000000000000..a0a27cd2b170 --- /dev/null +++ b/packages/TypeDeclaration/tests/Rector/Property/PropertyTypeDeclarationRector/Fixture/complex.php.inc @@ -0,0 +1,211 @@ +answer instanceof Collection) { + return new LocalizedStringList($this->answer->getValues()); + } + + if ($this->answer instanceof LocalizedStringList) { + return $this->answer; + } + + return new LocalizedStringList($this->answer); + } + + /** + * @param LocalizedStringList|LocalizedString[]|ArrayCollection $answer + */ + public function setAnswer($answer): void + { + if ($answer instanceof LocalizedStringList) { + $this->answer = $answer->getLocalizedStrings(); + + return; + } + + if ($answer instanceof ArrayCollection) { + $this->answer = $answer->getValues(); + + return; + } + + $this->answer = $answer; + } + + public function getVoters(): UserList + { + if ($this->voters instanceof Collection) { + return new UserList($this->voters->getValues()); + } + + if ($this->voters instanceof UserList) { + return $this->voters; + } + + return new UserList($this->voters); + } + + /** + * @param UserList|User[]|ArrayCollection $voters + */ + public function setVoters($voters): void + { + if ($voters instanceof UserList) { + $this->voters = $voters->getUsers(); + + return; + } + + if ($voters instanceof ArrayCollection) { + $this->voters = $voters->getValues(); + + return; + } + + $this->voters = $voters; + } + + public function addVoter(User $user): void + { + $this->voters[] = $user; + } +} + +?> +----- +answer instanceof Collection) { + return new LocalizedStringList($this->answer->getValues()); + } + + if ($this->answer instanceof LocalizedStringList) { + return $this->answer; + } + + return new LocalizedStringList($this->answer); + } + + /** + * @param LocalizedStringList|LocalizedString[]|ArrayCollection $answer + */ + public function setAnswer($answer): void + { + if ($answer instanceof LocalizedStringList) { + $this->answer = $answer->getLocalizedStrings(); + + return; + } + + if ($answer instanceof ArrayCollection) { + $this->answer = $answer->getValues(); + + return; + } + + $this->answer = $answer; + } + + public function getVoters(): UserList + { + if ($this->voters instanceof Collection) { + return new UserList($this->voters->getValues()); + } + + if ($this->voters instanceof UserList) { + return $this->voters; + } + + return new UserList($this->voters); + } + + /** + * @param UserList|User[]|ArrayCollection $voters + */ + public function setVoters($voters): void + { + if ($voters instanceof UserList) { + $this->voters = $voters->getUsers(); + + return; + } + + if ($voters instanceof ArrayCollection) { + $this->voters = $voters->getValues(); + + return; + } + + $this->voters = $voters; + } + + public function addVoter(User $user): void + { + $this->voters[] = $user; + } +} + +?> diff --git a/packages/TypeDeclaration/tests/Rector/Property/PropertyTypeDeclarationRector/PropertyTypeDeclarationRectorTest.php b/packages/TypeDeclaration/tests/Rector/Property/PropertyTypeDeclarationRector/PropertyTypeDeclarationRectorTest.php index 5c386352274b..e7609051de47 100644 --- a/packages/TypeDeclaration/tests/Rector/Property/PropertyTypeDeclarationRector/PropertyTypeDeclarationRectorTest.php +++ b/packages/TypeDeclaration/tests/Rector/Property/PropertyTypeDeclarationRector/PropertyTypeDeclarationRectorTest.php @@ -18,6 +18,8 @@ public function test(): void __DIR__ . '/Fixture/doctrine_column.php.inc', __DIR__ . '/Fixture/doctrine_relation.php.inc', + + __DIR__ . '/Fixture/complex.php.inc', // skip __DIR__ . '/Fixture/skip_multi_vars.php.inc', ]);