From ebbd854a2d040aebae033f93359bdbd457f45c54 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 2 Sep 2019 19:50:16 +0200 Subject: [PATCH 1/8] skip overriding existing return types --- .../ReturnTypeDeclarationRector.php | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php index 5654136c1350..98c919cf22c4 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php @@ -3,6 +3,7 @@ namespace Rector\TypeDeclaration\Rector\FunctionLike; use PhpParser\Node; +use PhpParser\Node\FunctionLike; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\ClassMethod; @@ -33,10 +34,16 @@ final class ReturnTypeDeclarationRector extends AbstractTypeDeclarationRector */ private $typeAnalyzer; - public function __construct(ReturnTypeInferer $returnTypeInferer, TypeAnalyzer $typeAnalyzer) + /** + * @var bool + */ + private $overrideExistingReturnTypes = true; + + public function __construct(ReturnTypeInferer $returnTypeInferer, TypeAnalyzer $typeAnalyzer, bool $overrideExistingReturnTypes = true) { $this->returnTypeInferer = $returnTypeInferer; $this->typeAnalyzer = $typeAnalyzer; + $this->overrideExistingReturnTypes = $overrideExistingReturnTypes; } public function getDefinition(): RectorDefinition @@ -90,11 +97,6 @@ public function refactor(Node $node): ?Node return null; } - $hasNewType = false; - if ($node->returnType !== null) { - $hasNewType = $node->returnType->getAttribute(self::HAS_NEW_INHERITED_TYPE, false); - } - $inferedTypes = $this->returnTypeInferer->inferFunctionLikeWithExcludedInferers( $node, [ReturnTypeDeclarationReturnTypeInferer::class] @@ -107,7 +109,7 @@ public function refactor(Node $node): ?Node $returnTypeInfo = new ReturnTypeInfo($inferedTypes, $this->typeAnalyzer, $inferedTypes); // @todo is it violation? - if ($hasNewType) { + if ($this->hasNewTypeFromPreviousIteration($node)) { // should override - is it subtype? $possibleOverrideNewReturnType = $returnTypeInfo->getFqnTypeNode(); if ($possibleOverrideNewReturnType !== null) { @@ -208,6 +210,12 @@ private function shouldSkip(Node $node): bool return false; } + if ($this->overrideExistingReturnTypes === false) { + if ($node->returnType) { + return true; + } + } + return $this->isNames($node, self::EXCLUDED_METHOD_NAMES); } @@ -222,4 +230,16 @@ private function isReturnTypeAlreadyAdded(Node $node, ReturnTypeInfo $returnType return false; } + + /** + * @param ClassMethod|Function_ $functionLike + */ + private function hasNewTypeFromPreviousIteration(FunctionLike $functionLike): bool + { + if ($functionLike->returnType === null) { + return false; + } + + return (bool) $functionLike->returnType->getAttribute(self::HAS_NEW_INHERITED_TYPE, false); + } } From 34f831929266a0635290b0bb7128a63913e85df6 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 2 Sep 2019 19:54:12 +0200 Subject: [PATCH 2/8] use findChildrenOfClass() instead of interface + filter --- .../src/Rector/FunctionLike/ReturnTypeDeclarationRector.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php index 98c919cf22c4..b8a6c0f2544e 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php @@ -154,14 +154,10 @@ private function populateChildren(Node $node, ReturnTypeInfo $returnTypeInfo): v throw new ShouldNotHappenException(__METHOD__ . '() on line ' . __LINE__); } - $childrenClassLikes = $this->parsedNodesByType->findClassesAndInterfacesByType($className); + $childrenClassLikes = $this->parsedNodesByType->findChildrenOfClass($className); // update their methods as well foreach ($childrenClassLikes as $childClassLike) { - if (! $childClassLike instanceof Class_) { - continue; - } - $usedTraits = $this->parsedNodesByType->findUsedTraitsInClass($childClassLike); foreach ($usedTraits as $trait) { $this->addReturnTypeToMethod($trait, $node, $returnTypeInfo); From 72b363f60a248207e53c9c14a633a9e780407bf5 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 2 Sep 2019 20:38:25 +0200 Subject: [PATCH 3/8] improve covariance resolution --- .../AbstractTypeDeclarationRector.php | 7 +- .../ReturnTypeDeclarationRector.php | 30 ++++- .../ReturnedPropertyReturnTypeInferer.php | 118 ------------------ .../CorrectionTest.php | 4 +- .../Fixture/Correction/prefix_fqn.php.inc | 4 +- .../return_interface_to_class.php.inc | 17 +++ .../nikic/inheritance_covariance.php.inc | 2 +- .../Fixture/nikic/object_php72.php.inc | 2 +- .../Php72RectorTest.php | 19 ++- .../ReturnTypeDeclarationRectorTest.php | 4 - 10 files changed, 67 insertions(+), 140 deletions(-) delete mode 100644 packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer/ReturnedPropertyReturnTypeInferer.php create mode 100644 packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/Covariance/return_interface_to_class.php.inc diff --git a/packages/TypeDeclaration/src/Rector/FunctionLike/AbstractTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/FunctionLike/AbstractTypeDeclarationRector.php index fdaa50303471..7362b6c67d63 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/AbstractTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/AbstractTypeDeclarationRector.php @@ -140,11 +140,8 @@ protected function isSubtypeOf(Node $possibleSubtype, Node $type, string $kind): $type = $type->toString(); if ($kind === 'return') { - if ($this->isAtLeastPhpVersion('7.4')) { - // @see https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters - if (is_a($possibleSubtype, $type, true)) { - return true; - } + if (is_a($possibleSubtype, $type, true)) { + return true; } } elseif ($kind === 'param') { if (is_a($possibleSubtype, $type, true)) { diff --git a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php index b8a6c0f2544e..9abb198b80ce 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php @@ -4,7 +4,6 @@ use PhpParser\Node; use PhpParser\Node\FunctionLike; -use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Function_; @@ -17,6 +16,9 @@ use Rector\TypeDeclaration\TypeInferer\ReturnTypeInferer; use Rector\TypeDeclaration\TypeInferer\ReturnTypeInferer\ReturnTypeDeclarationReturnTypeInferer; +/** + * @sponsor Thanks https://spaceflow.io/ for sponsoring this rule - visit them on https://github.com/SpaceFlow-app + */ final class ReturnTypeDeclarationRector extends AbstractTypeDeclarationRector { /** @@ -39,8 +41,11 @@ final class ReturnTypeDeclarationRector extends AbstractTypeDeclarationRector */ private $overrideExistingReturnTypes = true; - public function __construct(ReturnTypeInferer $returnTypeInferer, TypeAnalyzer $typeAnalyzer, bool $overrideExistingReturnTypes = true) - { + public function __construct( + ReturnTypeInferer $returnTypeInferer, + TypeAnalyzer $typeAnalyzer, + bool $overrideExistingReturnTypes = true + ) { $this->returnTypeInferer = $returnTypeInferer; $this->typeAnalyzer = $typeAnalyzer; $this->overrideExistingReturnTypes = $overrideExistingReturnTypes; @@ -114,7 +119,10 @@ public function refactor(Node $node): ?Node $possibleOverrideNewReturnType = $returnTypeInfo->getFqnTypeNode(); if ($possibleOverrideNewReturnType !== null) { if ($node->returnType !== null) { - if ($this->isSubtypeOf($possibleOverrideNewReturnType, $node->returnType, 'return')) { + $isSubtype = $this->isSubtypeOf($possibleOverrideNewReturnType, $node->returnType, 'return'); + + // @see https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters + if ($isSubtype && $this->isAtLeastPhpVersion('7.4')) { // allow override $node->returnType = $returnTypeInfo->getFqnTypeNode(); } @@ -127,7 +135,19 @@ public function refactor(Node $node): ?Node return null; } - $node->returnType = $returnTypeInfo->getFqnTypeNode(); + // should be previosu node overriden? + if ($node->returnType !== null && $returnTypeInfo->getFqnTypeNode() !== null) { + $isSubtype = $this->isSubtypeOf($returnTypeInfo->getFqnTypeNode(), $node->returnType, 'return'); + + // @see https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters + if ($this->isAtLeastPhpVersion('7.4') && $isSubtype) { + $node->returnType = $returnTypeInfo->getFqnTypeNode(); + } elseif ($isSubtype === false) { + $node->returnType = $returnTypeInfo->getFqnTypeNode(); + } + } else { + $node->returnType = $returnTypeInfo->getFqnTypeNode(); + } } $this->populateChildren($node, $returnTypeInfo); diff --git a/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer/ReturnedPropertyReturnTypeInferer.php b/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer/ReturnedPropertyReturnTypeInferer.php deleted file mode 100644 index be78fda84455..000000000000 --- a/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer/ReturnedPropertyReturnTypeInferer.php +++ /dev/null @@ -1,118 +0,0 @@ -variable → and get type $this->variable from @var annotation - */ -final class ReturnedPropertyReturnTypeInferer extends AbstractTypeInferer implements ReturnTypeInfererInterface -{ - /** - * @var PropertyTypeInferer - */ - private $propertyTypeInferer; - - public function __construct(PropertyTypeInferer $propertyTypeInferer) - { - $this->propertyTypeInferer = $propertyTypeInferer; - } - - /** - * @param ClassMethod|Closure|Function_ $functionLike - * @return string[]|IdentifierValueObject[] - */ - public function inferFunctionLike(FunctionLike $functionLike): array - { - if (! $functionLike instanceof ClassMethod) { - return []; - } - - $propertyFetch = $this->matchSingleStmtReturnPropertyFetch($functionLike); - if ($propertyFetch === null) { - return []; - } - - $property = $this->getPropertyByPropertyFetch($propertyFetch); - if ($property === null) { - return []; - } - - return $this->propertyTypeInferer->inferProperty($property); - } - - public function getPriority(): int - { - return 700; - } - - private function matchSingleStmtReturnPropertyFetch(ClassMethod $classMethod): ?PropertyFetch - { - if (count((array) $classMethod->stmts) !== 1) { - return null; - } - - $singleStmt = $classMethod->stmts[0]; - if ($singleStmt instanceof Expression) { - $singleStmt = $singleStmt->expr; - } - - // is it return? - if (! $singleStmt instanceof Return_) { - return null; - } - - if (! $singleStmt->expr instanceof PropertyFetch) { - return null; - } - - $propertyFetch = $singleStmt->expr; - if (! $this->nameResolver->isName($propertyFetch->var, 'this')) { - return null; - } - - return $propertyFetch; - } - - private function getPropertyByPropertyFetch(PropertyFetch $propertyFetch): ?Property - { - /** @var Class_|Trait_|Interface_|null $class */ - $class = $propertyFetch->getAttribute(AttributeKey::CLASS_NODE); - if ($class === null) { - return null; - } - - /** @var string $propertyName */ - $propertyName = $this->nameResolver->getName($propertyFetch->name); - - foreach ($class->stmts as $stmt) { - if (! $stmt instanceof Property) { - continue; - } - - if (! $this->nameResolver->isName($stmt, $propertyName)) { - continue; - } - - return $stmt; - } - - return null; - } -} diff --git a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/CorrectionTest.php b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/CorrectionTest.php index 45b00cb0c2de..8948ff152ec8 100644 --- a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/CorrectionTest.php +++ b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/CorrectionTest.php @@ -10,10 +10,10 @@ final class CorrectionTest extends AbstractRectorTestCase public function test(): void { $this->doTestFiles([ - // __DIR__ . '/Fixture/Correction/constructor_property_assign_over_getter.php.inc', + __DIR__ . '/Fixture/Correction/constructor_property_assign_over_getter.php.inc', __DIR__ . '/Fixture/Correction/prefix_fqn.php.inc', // skip - // __DIR__ . '/Fixture/Correction/skip_override_of_the_same_class.php.inc', + __DIR__ . '/Fixture/Correction/skip_override_of_the_same_class.php.inc', ]); } diff --git a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/Correction/prefix_fqn.php.inc b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/Correction/prefix_fqn.php.inc index 49bf7874a9db..970a97822049 100644 --- a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/Correction/prefix_fqn.php.inc +++ b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/Correction/prefix_fqn.php.inc @@ -5,7 +5,7 @@ namespace Rector\TypeDeclaration\Tests\Rector\ClassMethod\ReturnTypeDeclarationR use Rector\TypeDeclaration\Tests\Rector\FunctionLike\ReturnTypeDeclarationRector\Source\RealReturnedClass; use Rector\TypeDeclaration\Tests\Rector\FunctionLike\ReturnTypeDeclarationRector\Source\ReturnedClass; -class SkipOverrideOfTheSameClass +class PrefixFqn { public function getReturnedClass(): ReturnedClass { @@ -22,7 +22,7 @@ namespace Rector\TypeDeclaration\Tests\Rector\ClassMethod\ReturnTypeDeclarationR use Rector\TypeDeclaration\Tests\Rector\FunctionLike\ReturnTypeDeclarationRector\Source\RealReturnedClass; use Rector\TypeDeclaration\Tests\Rector\FunctionLike\ReturnTypeDeclarationRector\Source\ReturnedClass; -class SkipOverrideOfTheSameClass +class PrefixFqn { public function getReturnedClass(): \Rector\TypeDeclaration\Tests\Rector\FunctionLike\ReturnTypeDeclarationRector\Source\RealReturnedClass { diff --git a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/Covariance/return_interface_to_class.php.inc b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/Covariance/return_interface_to_class.php.inc new file mode 100644 index 000000000000..79ca8637d886 --- /dev/null +++ b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/Covariance/return_interface_to_class.php.inc @@ -0,0 +1,17 @@ +get(ParameterProvider::class); - $parameterProvider->changeParameter(' php_version_features', '7.0'); + $parameterProvider->changeParameter('php_version_features', '7.0'); + } + + /** + * Needed to restore previous version + */ + protected function tearDown(): void + { + parent::tearDown(); + + $parameterProvider = self::$container->get(ParameterProvider::class); + $parameterProvider->changeParameter('php_version_features', '10.0'); } public function test(): void { - $this->doTestFiles([__DIR__ . '/Fixture/nikic/object_php72.php.inc']); + $this->doTestFiles([ + __DIR__ . '/Fixture/nikic/object_php72.php.inc', + __DIR__ . '/Fixture/nikic/inheritance_covariance.php.inc', + __DIR__ . '/Fixture/Covariance/return_interface_to_class.php.inc', + ]); } protected function getRectorClass(): string diff --git a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php index e27cf0f61ab0..5df48dd7c042 100644 --- a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php +++ b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php @@ -5,9 +5,6 @@ use Rector\Testing\PHPUnit\AbstractRectorTestCase; use Rector\TypeDeclaration\Rector\FunctionLike\ReturnTypeDeclarationRector; -/** - * @sponsor Thanks https://spaceflow.io/ for sponsoring this rule - visit them on https://github.com/SpaceFlow-app - */ final class ReturnTypeDeclarationRectorTest extends AbstractRectorTestCase { public function test(): void @@ -67,7 +64,6 @@ public function testInheritance(): void { $files = [ __DIR__ . '/Fixture/nikic/inheritance.php.inc', - __DIR__ . '/Fixture/nikic/inheritance_covariance.php.inc', __DIR__ . '/Fixture/nikic/nullable_inheritance.php.inc', ]; From 70086858736abb57dab58947d2dfe7bd86eafc0b Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 2 Sep 2019 20:46:22 +0200 Subject: [PATCH 4/8] make subtype of independent --- .../FunctionLike/AbstractTypeDeclarationRector.php | 12 +++--------- .../FunctionLike/ParamTypeDeclarationRector.php | 2 +- .../FunctionLike/ReturnTypeDeclarationRector.php | 4 ++-- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/TypeDeclaration/src/Rector/FunctionLike/AbstractTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/FunctionLike/AbstractTypeDeclarationRector.php index 7362b6c67d63..1187a30abeb8 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/AbstractTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/AbstractTypeDeclarationRector.php @@ -128,7 +128,7 @@ protected function isChangeVendorLockedIn(ClassMethod $classMethod, int $paramPo * @param Name|NullableType|Identifier $possibleSubtype * @param Name|NullableType|Identifier $type */ - protected function isSubtypeOf(Node $possibleSubtype, Node $type, string $kind): bool + protected function isSubtypeOf(Node $possibleSubtype, Node $type): bool { $type = $type instanceof NullableType ? $type->type : $type; @@ -139,14 +139,8 @@ protected function isSubtypeOf(Node $possibleSubtype, Node $type, string $kind): $possibleSubtype = $possibleSubtype->toString(); $type = $type->toString(); - if ($kind === 'return') { - if (is_a($possibleSubtype, $type, true)) { - return true; - } - } elseif ($kind === 'param') { - if (is_a($possibleSubtype, $type, true)) { - return true; - } + if (is_a($possibleSubtype, $type, true)) { + return true; } if (in_array($possibleSubtype, ['array', 'Traversable'], true) && $type === 'iterable') { diff --git a/packages/TypeDeclaration/src/Rector/FunctionLike/ParamTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/FunctionLike/ParamTypeDeclarationRector.php index 2599b888b845..740ff01f989a 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/ParamTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/ParamTypeDeclarationRector.php @@ -143,7 +143,7 @@ public function refactor(Node $node): ?Node $possibleOverrideNewReturnType = $paramTypeInfo->getFqnTypeNode(); if ($possibleOverrideNewReturnType !== null) { if ($paramNode->type !== null) { - if ($this->isSubtypeOf($possibleOverrideNewReturnType, $paramNode->type, 'param')) { + if ($this->isSubtypeOf($possibleOverrideNewReturnType, $paramNode->type)) { // allow override $paramNode->type = $paramTypeInfo->getFqnTypeNode(); } diff --git a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php index 9abb198b80ce..32b6558aa91b 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php @@ -119,7 +119,7 @@ public function refactor(Node $node): ?Node $possibleOverrideNewReturnType = $returnTypeInfo->getFqnTypeNode(); if ($possibleOverrideNewReturnType !== null) { if ($node->returnType !== null) { - $isSubtype = $this->isSubtypeOf($possibleOverrideNewReturnType, $node->returnType, 'return'); + $isSubtype = $this->isSubtypeOf($possibleOverrideNewReturnType, $node->returnType); // @see https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters if ($isSubtype && $this->isAtLeastPhpVersion('7.4')) { @@ -137,7 +137,7 @@ public function refactor(Node $node): ?Node // should be previosu node overriden? if ($node->returnType !== null && $returnTypeInfo->getFqnTypeNode() !== null) { - $isSubtype = $this->isSubtypeOf($returnTypeInfo->getFqnTypeNode(), $node->returnType, 'return'); + $isSubtype = $this->isSubtypeOf($returnTypeInfo->getFqnTypeNode(), $node->returnType); // @see https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters if ($this->isAtLeastPhpVersion('7.4') && $isSubtype) { From 1bdbc9dadd2de26c2750625643c77028a2629813 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 2 Sep 2019 20:50:58 +0200 Subject: [PATCH 5/8] cleanup dead code --- .../ReturnTypeDeclarationRector.php | 61 ++++--------------- 1 file changed, 13 insertions(+), 48 deletions(-) diff --git a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php index 32b6558aa91b..cf7fce68ae24 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php @@ -3,7 +3,6 @@ namespace Rector\TypeDeclaration\Rector\FunctionLike; use PhpParser\Node; -use PhpParser\Node\FunctionLike; use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Function_; @@ -113,41 +112,22 @@ public function refactor(Node $node): ?Node $returnTypeInfo = new ReturnTypeInfo($inferedTypes, $this->typeAnalyzer, $inferedTypes); - // @todo is it violation? - if ($this->hasNewTypeFromPreviousIteration($node)) { - // should override - is it subtype? - $possibleOverrideNewReturnType = $returnTypeInfo->getFqnTypeNode(); - if ($possibleOverrideNewReturnType !== null) { - if ($node->returnType !== null) { - $isSubtype = $this->isSubtypeOf($possibleOverrideNewReturnType, $node->returnType); - - // @see https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters - if ($isSubtype && $this->isAtLeastPhpVersion('7.4')) { - // allow override - $node->returnType = $returnTypeInfo->getFqnTypeNode(); - } - } else { - $node->returnType = $returnTypeInfo->getFqnTypeNode(); - } - } - } else { - if ($this->isReturnTypeAlreadyAdded($node, $returnTypeInfo)) { - return null; - } + if ($this->isReturnTypeAlreadyAdded($node, $returnTypeInfo)) { + return null; + } - // should be previosu node overriden? - if ($node->returnType !== null && $returnTypeInfo->getFqnTypeNode() !== null) { - $isSubtype = $this->isSubtypeOf($returnTypeInfo->getFqnTypeNode(), $node->returnType); - - // @see https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters - if ($this->isAtLeastPhpVersion('7.4') && $isSubtype) { - $node->returnType = $returnTypeInfo->getFqnTypeNode(); - } elseif ($isSubtype === false) { - $node->returnType = $returnTypeInfo->getFqnTypeNode(); - } - } else { + // should be previous overridden? + if ($node->returnType !== null && $returnTypeInfo->getFqnTypeNode() !== null) { + $isSubtype = $this->isSubtypeOf($returnTypeInfo->getFqnTypeNode(), $node->returnType); + + // @see https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters + if ($this->isAtLeastPhpVersion('7.4') && $isSubtype) { + $node->returnType = $returnTypeInfo->getFqnTypeNode(); + } elseif ($isSubtype === false) { $node->returnType = $returnTypeInfo->getFqnTypeNode(); } + } else { + $node->returnType = $returnTypeInfo->getFqnTypeNode(); } $this->populateChildren($node, $returnTypeInfo); @@ -211,9 +191,6 @@ private function addReturnTypeToMethod( $currentClassMethod->returnType = $resolvedChildType; - // let the method now it was changed now - $currentClassMethod->returnType->setAttribute(self::HAS_NEW_INHERITED_TYPE, true); - $this->notifyNodeChangeFileInfo($currentClassMethod); } @@ -246,16 +223,4 @@ private function isReturnTypeAlreadyAdded(Node $node, ReturnTypeInfo $returnType return false; } - - /** - * @param ClassMethod|Function_ $functionLike - */ - private function hasNewTypeFromPreviousIteration(FunctionLike $functionLike): bool - { - if ($functionLike->returnType === null) { - return false; - } - - return (bool) $functionLike->returnType->getAttribute(self::HAS_NEW_INHERITED_TYPE, false); - } } From e1bb7a0fcb62bca8e462faecfd240d636af4af5a Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 2 Sep 2019 21:41:21 +0200 Subject: [PATCH 6/8] add covariance test --- .../ReturnTypeDeclarationRector.php | 5 ++- .../CovarianceTest.php | 43 +++++++++++++++++++ ...urn_nullable_with_parent_interface.php.inc | 14 ++++++ .../Php72RectorTest.php | 2 - 4 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/CovarianceTest.php create mode 100644 packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/Covariance/return_nullable_with_parent_interface.php.inc diff --git a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php index cf7fce68ae24..f6994e850e5f 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php @@ -111,7 +111,6 @@ public function refactor(Node $node): ?Node } $returnTypeInfo = new ReturnTypeInfo($inferedTypes, $this->typeAnalyzer, $inferedTypes); - if ($this->isReturnTypeAlreadyAdded($node, $returnTypeInfo)) { return null; } @@ -127,7 +126,9 @@ public function refactor(Node $node): ?Node $node->returnType = $returnTypeInfo->getFqnTypeNode(); } } else { - $node->returnType = $returnTypeInfo->getFqnTypeNode(); + if ($returnTypeInfo->getFqnTypeNode() !== null) { + $node->returnType = $returnTypeInfo->getFqnTypeNode(); + } } $this->populateChildren($node, $returnTypeInfo); diff --git a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/CovarianceTest.php b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/CovarianceTest.php new file mode 100644 index 000000000000..11945d0a6881 --- /dev/null +++ b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/CovarianceTest.php @@ -0,0 +1,43 @@ +get(ParameterProvider::class); + $parameterProvider->changeParameter('php_version_features', '7.0'); + } + + public function test(): void + { + $this->doTestFiles([ + __DIR__ . '/Fixture/nikic/inheritance_covariance.php.inc', + __DIR__ . '/Fixture/Covariance/return_interface_to_class.php.inc', + __DIR__ . '/Fixture/Covariance/return_nullable_with_parent_interface.php.inc', + ]); + } + + protected function getRectorClass(): string + { + return ReturnTypeDeclarationRector::class; + } + + /** + * Needed to restore previous version + */ + protected function tearDown(): void + { + parent::tearDown(); + + $parameterProvider = self::$container->get(ParameterProvider::class); + $parameterProvider->changeParameter('php_version_features', '10.0'); + } +} diff --git a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/Covariance/return_nullable_with_parent_interface.php.inc b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/Covariance/return_nullable_with_parent_interface.php.inc new file mode 100644 index 000000000000..322e0f13ab12 --- /dev/null +++ b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/Covariance/return_nullable_with_parent_interface.php.inc @@ -0,0 +1,14 @@ +doTestFiles([ __DIR__ . '/Fixture/nikic/object_php72.php.inc', - __DIR__ . '/Fixture/nikic/inheritance_covariance.php.inc', - __DIR__ . '/Fixture/Covariance/return_interface_to_class.php.inc', ]); } From cf1514fad65793b1da5e399dd4e7ffcda9eb4fda Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 2 Sep 2019 22:11:41 +0200 Subject: [PATCH 7/8] skip self override --- .../ReturnTypeDeclarationRector.php | 9 ++++++++ .../CovarianceTest.php | 22 +++++++++---------- .../Fixture/skip_self.php.inc | 11 ++++++++++ .../Php72RectorTest.php | 4 +--- .../ReturnTypeDeclarationRectorTest.php | 1 + 5 files changed, 33 insertions(+), 14 deletions(-) create mode 100644 packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/skip_self.php.inc diff --git a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php index f6994e850e5f..05eb3be9a15b 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php @@ -222,6 +222,15 @@ private function isReturnTypeAlreadyAdded(Node $node, ReturnTypeInfo $returnType return true; } + // prevent overriding self with itself + if ($this->print($node->returnType) === 'self') { + $className = $node->getAttribute(AttributeKey::CLASS_NAME); + + if (ltrim($this->print($returnTypeInfo->getFqnTypeNode()), '\\') === $className) { + return true; + } + } + return false; } } diff --git a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/CovarianceTest.php b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/CovarianceTest.php index 11945d0a6881..df338dad52b0 100644 --- a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/CovarianceTest.php +++ b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/CovarianceTest.php @@ -16,6 +16,17 @@ protected function setUp(): void $parameterProvider->changeParameter('php_version_features', '7.0'); } + /** + * Needed to restore previous version + */ + protected function tearDown(): void + { + parent::tearDown(); + + $parameterProvider = self::$container->get(ParameterProvider::class); + $parameterProvider->changeParameter('php_version_features', '10.0'); + } + public function test(): void { $this->doTestFiles([ @@ -29,15 +40,4 @@ protected function getRectorClass(): string { return ReturnTypeDeclarationRector::class; } - - /** - * Needed to restore previous version - */ - protected function tearDown(): void - { - parent::tearDown(); - - $parameterProvider = self::$container->get(ParameterProvider::class); - $parameterProvider->changeParameter('php_version_features', '10.0'); - } } diff --git a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/skip_self.php.inc b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/skip_self.php.inc new file mode 100644 index 000000000000..bab980fca004 --- /dev/null +++ b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/skip_self.php.inc @@ -0,0 +1,11 @@ +doTestFiles([ - __DIR__ . '/Fixture/nikic/object_php72.php.inc', - ]); + $this->doTestFiles([__DIR__ . '/Fixture/nikic/object_php72.php.inc']); } protected function getRectorClass(): string diff --git a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php index 5df48dd7c042..448c875d15cb 100644 --- a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php +++ b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php @@ -10,6 +10,7 @@ final class ReturnTypeDeclarationRectorTest extends AbstractRectorTestCase public function test(): void { $files = [ + __DIR__ . '/Fixture/skip_self.php.inc', // static types __DIR__ . '/Fixture/void_type.php.inc', __DIR__ . '/Fixture/no_void_abstract.php.inc', From 57db278625edf788149d233143e97a3b6e11c7c1 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 3 Sep 2019 08:41:22 +0200 Subject: [PATCH 8/8] add mixed type --- .../src/StaticTypeToStringResolver.php | 5 ++- .../PhpDocParser/ParamPhpDocNodeFactory.php | 14 +++++- .../AddArrayParamDocTypeRector.php | 10 ++--- .../Closure/AddClosureReturnTypeRector.php | 2 +- .../ReturnTypeDeclarationRector.php | 24 +++++----- .../PropertyTypeDeclarationRector.php | 13 +----- .../src/TypeInferer/PropertyTypeInferer.php | 6 +-- .../GetterPropertyTypeInferer.php | 4 +- .../src/TypeInferer/ReturnTypeInferer.php | 45 +++++++++++++------ .../ReturnedNodesReturnTypeInferer.php | 7 ++- .../Fixture/skip_mixed_and_string.php.inc | 25 +++++++++++ .../ReturnTypeDeclarationRectorTest.php | 1 + ...structor_array_param_with_nullable.php.inc | 2 +- 13 files changed, 102 insertions(+), 56 deletions(-) create mode 100644 packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/skip_mixed_and_string.php.inc diff --git a/packages/NodeTypeResolver/src/StaticTypeToStringResolver.php b/packages/NodeTypeResolver/src/StaticTypeToStringResolver.php index 90e791f75137..a0ab69f392e9 100644 --- a/packages/NodeTypeResolver/src/StaticTypeToStringResolver.php +++ b/packages/NodeTypeResolver/src/StaticTypeToStringResolver.php @@ -10,6 +10,7 @@ use PHPStan\Type\FloatType; use PHPStan\Type\IntegerType; use PHPStan\Type\IntersectionType; +use PHPStan\Type\MixedType; use PHPStan\Type\NullType; use PHPStan\Type\ObjectType; use PHPStan\Type\ObjectWithoutClassType; @@ -36,6 +37,7 @@ public function __construct(CallableCollectorPopulator $callableCollectorPopulat BooleanType::class => ['bool'], StringType::class => ['string'], NullType::class => ['null'], + MixedType::class => ['mixed'], // more complex callables function (ArrayType $arrayType): array { @@ -69,8 +71,7 @@ function (IntersectionType $intersectionType): array { return $this->removeGenericArrayTypeIfThereIsSpecificArrayType($types); }, function (ObjectType $objectType): array { - // the must be absolute, since we have no other way to check absolute/local path - return ['\\' . $objectType->getClassName()]; + return [$objectType->getClassName()]; }, ]; diff --git a/packages/TypeDeclaration/src/PhpDocParser/ParamPhpDocNodeFactory.php b/packages/TypeDeclaration/src/PhpDocParser/ParamPhpDocNodeFactory.php index 0d2de5ef4f78..2799bf0047bd 100644 --- a/packages/TypeDeclaration/src/PhpDocParser/ParamPhpDocNodeFactory.php +++ b/packages/TypeDeclaration/src/PhpDocParser/ParamPhpDocNodeFactory.php @@ -31,12 +31,12 @@ public function create(array $types, Param $param): AttributeAwarePhpDocTagNode if (count($types) > 1) { $unionedTypes = []; foreach ($types as $type) { - $unionedTypes[] = new IdentifierTypeNode($type); + $unionedTypes[] = $this->createIdentifierTypeNode($type); } $typeNode = new UnionTypeNode($unionedTypes); } elseif (count($types) === 1) { - $typeNode = new IdentifierTypeNode($types[0]); + $typeNode = $this->createIdentifierTypeNode($types[0]); } else { throw new ShouldNotHappenException(__METHOD__ . '() on line ' . __LINE__); } @@ -53,4 +53,14 @@ public function create(array $types, Param $param): AttributeAwarePhpDocTagNode return new AttributeAwarePhpDocTagNode('@param', $paramTagValueNode); } + + private function createIdentifierTypeNode(string $type): IdentifierTypeNode + { + if (class_exists($type)) { + // FQN class name + $type = '\\' . $type; + } + + return new IdentifierTypeNode($type); + } } diff --git a/packages/TypeDeclaration/src/Rector/ClassMethod/AddArrayParamDocTypeRector.php b/packages/TypeDeclaration/src/Rector/ClassMethod/AddArrayParamDocTypeRector.php index cc085825cd4c..36c47b627810 100644 --- a/packages/TypeDeclaration/src/Rector/ClassMethod/AddArrayParamDocTypeRector.php +++ b/packages/TypeDeclaration/src/Rector/ClassMethod/AddArrayParamDocTypeRector.php @@ -99,14 +99,11 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - /** @var Param[] $params */ - $params = (array) $node->params; - - if (count($params) === 0) { + if (count($node->getParams()) === 0) { return null; } - foreach ($params as $param) { + foreach ($node->getParams() as $param) { if ($this->shouldSkipParam($param)) { return null; } @@ -116,7 +113,8 @@ public function refactor(Node $node): ?Node return null; } - $this->docBlockManipulator->addTag($node, $this->paramPhpDocNodeFactory->create($types, $param)); + $paramTagNode = $this->paramPhpDocNodeFactory->create($types, $param); + $this->docBlockManipulator->addTag($node, $paramTagNode); } return $node; diff --git a/packages/TypeDeclaration/src/Rector/Closure/AddClosureReturnTypeRector.php b/packages/TypeDeclaration/src/Rector/Closure/AddClosureReturnTypeRector.php index 34c7686ffa3e..9cb7c3aec344 100644 --- a/packages/TypeDeclaration/src/Rector/Closure/AddClosureReturnTypeRector.php +++ b/packages/TypeDeclaration/src/Rector/Closure/AddClosureReturnTypeRector.php @@ -90,8 +90,8 @@ public function refactor(Node $node): ?Node } $inferedReturnTypes = $this->returnTypeInferer->inferFunctionLike($node); - $returnTypeInfo = new ReturnTypeInfo($inferedReturnTypes, $this->typeAnalyzer, $inferedReturnTypes); + $returnTypeInfo = new ReturnTypeInfo($inferedReturnTypes, $this->typeAnalyzer, $inferedReturnTypes); $returnTypeNode = $returnTypeInfo->getFqnTypeNode(); if ($returnTypeNode === null) { return null; diff --git a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php index 05eb3be9a15b..cb20952d1231 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php @@ -115,23 +115,27 @@ public function refactor(Node $node): ?Node return null; } + $shouldPopulateChildren = false; // should be previous overridden? if ($node->returnType !== null && $returnTypeInfo->getFqnTypeNode() !== null) { $isSubtype = $this->isSubtypeOf($returnTypeInfo->getFqnTypeNode(), $node->returnType); // @see https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters if ($this->isAtLeastPhpVersion('7.4') && $isSubtype) { + $shouldPopulateChildren = true; $node->returnType = $returnTypeInfo->getFqnTypeNode(); - } elseif ($isSubtype === false) { - $node->returnType = $returnTypeInfo->getFqnTypeNode(); - } - } else { - if ($returnTypeInfo->getFqnTypeNode() !== null) { + } elseif ($isSubtype === false) { // type override + $shouldPopulateChildren = true; $node->returnType = $returnTypeInfo->getFqnTypeNode(); } + } elseif ($returnTypeInfo->getFqnTypeNode() !== null) { + $shouldPopulateChildren = true; + $node->returnType = $returnTypeInfo->getFqnTypeNode(); } - $this->populateChildren($node, $returnTypeInfo); + if ($shouldPopulateChildren) { + $this->populateChildren($node, $returnTypeInfo); + } return $node; } @@ -200,16 +204,16 @@ private function addReturnTypeToMethod( */ private function shouldSkip(Node $node): bool { - if (! $node instanceof ClassMethod) { - return false; - } - if ($this->overrideExistingReturnTypes === false) { if ($node->returnType) { return true; } } + if (! $node instanceof ClassMethod) { + return false; + } + return $this->isNames($node, self::EXCLUDED_METHOD_NAMES); } diff --git a/packages/TypeDeclaration/src/Rector/Property/PropertyTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/Property/PropertyTypeDeclarationRector.php index d185d27bedac..f662317c61b4 100644 --- a/packages/TypeDeclaration/src/Rector/Property/PropertyTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/Property/PropertyTypeDeclarationRector.php @@ -10,7 +10,6 @@ use Rector\Rector\AbstractRector; use Rector\RectorDefinition\RectorDefinition; use Rector\TypeDeclaration\TypeInferer\PropertyTypeInferer; -use Rector\TypeDeclaration\ValueObject\IdentifierValueObject; /** * @sponsor Thanks https://spaceflow.io/ for sponsoring this rule - visit them on https://github.com/SpaceFlow-app @@ -61,20 +60,10 @@ public function refactor(Node $node): ?Node $types = $this->propertyTypeInferer->inferProperty($node); if ($types) { - $this->setNodeVarTypes($node, $types); + $this->docBlockManipulator->changeVarTag($node, $types); return $node; } return null; } - - /** - * @param string[]|IdentifierValueObject[] $varTypes - */ - private function setNodeVarTypes(Node $node, array $varTypes): Node - { - $this->docBlockManipulator->changeVarTag($node, $varTypes); - - return $node; - } } diff --git a/packages/TypeDeclaration/src/TypeInferer/PropertyTypeInferer.php b/packages/TypeDeclaration/src/TypeInferer/PropertyTypeInferer.php index c7f90271b408..6dac5bdee35b 100644 --- a/packages/TypeDeclaration/src/TypeInferer/PropertyTypeInferer.php +++ b/packages/TypeDeclaration/src/TypeInferer/PropertyTypeInferer.php @@ -26,9 +26,9 @@ public function __construct(array $propertyTypeInferers) */ public function inferProperty(Property $property): array { - foreach ($this->propertyTypeInferers as $propertyTypeInferers) { - $types = $propertyTypeInferers->inferProperty($property); - if ($types !== []) { + foreach ($this->propertyTypeInferers as $propertyTypeInferer) { + $types = $propertyTypeInferer->inferProperty($property); + if ($types !== [] && $types !== ['mixed']) { return $types; } } diff --git a/packages/TypeDeclaration/src/TypeInferer/PropertyTypeInferer/GetterPropertyTypeInferer.php b/packages/TypeDeclaration/src/TypeInferer/PropertyTypeInferer/GetterPropertyTypeInferer.php index 86ce53f03185..6aea7f05c2f0 100644 --- a/packages/TypeDeclaration/src/TypeInferer/PropertyTypeInferer/GetterPropertyTypeInferer.php +++ b/packages/TypeDeclaration/src/TypeInferer/PropertyTypeInferer/GetterPropertyTypeInferer.php @@ -58,7 +58,7 @@ public function inferProperty(Property $property): array } $returnTypes = $this->inferClassMethodReturnTypes($classMethod); - if ($returnTypes !== []) { + if ($returnTypes !== [] && $returnTypes !== ['mixed']) { return $returnTypes; } } @@ -107,7 +107,7 @@ private function inferClassMethodReturnTypes(ClassMethod $classMethod): array } $inferedTypes = $this->returnedNodesReturnTypeInferer->inferFunctionLike($classMethod); - if ($inferedTypes) { + if ($inferedTypes !== [] && $inferedTypes !== ['mixed']) { return $inferedTypes; } diff --git a/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer.php b/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer.php index 76dd045d082c..a3edd583328b 100644 --- a/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer.php +++ b/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer.php @@ -3,6 +3,7 @@ namespace Rector\TypeDeclaration\TypeInferer; use PhpParser\Node\FunctionLike; +use Rector\Exception\ShouldNotHappenException; use Rector\TypeDeclaration\Contract\TypeInferer\ReturnTypeInfererInterface; final class ReturnTypeInferer extends AbstractPriorityAwareTypeInferer @@ -25,14 +26,7 @@ public function __construct(array $returnTypeInferers) */ public function inferFunctionLike(FunctionLike $functionLike): array { - foreach ($this->returnTypeInferers as $returnTypeInferer) { - $types = $returnTypeInferer->inferFunctionLike($functionLike); - if ($types !== []) { - return $types; - } - } - - return []; + return $this->inferFunctionLikeWithExcludedInferers($functionLike, []); } /** @@ -42,18 +36,43 @@ public function inferFunctionLike(FunctionLike $functionLike): array public function inferFunctionLikeWithExcludedInferers(FunctionLike $functionLike, array $excludedInferers): array { foreach ($this->returnTypeInferers as $returnTypeInferer) { - foreach ($excludedInferers as $excludedInferer) { - if (is_a($returnTypeInferer, $excludedInferer, true)) { - continue 2; - } + if ($this->shouldSkipExcludedTypeInferer($returnTypeInferer, $excludedInferers)) { + continue; } $types = $returnTypeInferer->inferFunctionLike($functionLike); - if ($types !== []) { + if ($types !== [] && $types !== ['mixed']) { return $types; } } return []; } + + /** + * @param string[] $excludedInferers + */ + private function shouldSkipExcludedTypeInferer( + ReturnTypeInfererInterface $returnTypeInferer, + array $excludedInferers + ): bool { + foreach ($excludedInferers as $excludedInferer) { + $this->ensureIsTypeInferer($excludedInferer); + + if (is_a($returnTypeInferer, $excludedInferer)) { + return true; + } + } + + return false; + } + + private function ensureIsTypeInferer(string $excludedInferer): void + { + if (is_a($excludedInferer, ReturnTypeInfererInterface::class, true)) { + return; + } + + throw new ShouldNotHappenException(); + } } diff --git a/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer/ReturnedNodesReturnTypeInferer.php b/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer/ReturnedNodesReturnTypeInferer.php index d5d154c8980d..6b03d37d057c 100644 --- a/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer/ReturnedNodesReturnTypeInferer.php +++ b/packages/TypeDeclaration/src/TypeInferer/ReturnTypeInferer/ReturnedNodesReturnTypeInferer.php @@ -58,16 +58,15 @@ public function getPriority(): int } /** - * @param ClassMethod|Closure|Function_ $functionLike * @return Return_[] */ private function collectReturns(FunctionLike $functionLike): array { $returns = []; - $this->callableNodeTraverser->traverseNodesWithCallable((array) $functionLike->stmts, function (Node $node) use ( - &$returns - ): ?int { + $this->callableNodeTraverser->traverseNodesWithCallable((array) $functionLike->getStmts(), function ( + Node $node + ) use (&$returns): ?int { if ($node instanceof Function_ || $node instanceof Closure || $node instanceof ArrowFunction) { // skip Return_ nodes in nested functions return NodeTraverser::DONT_TRAVERSE_CHILDREN; diff --git a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/skip_mixed_and_string.php.inc b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/skip_mixed_and_string.php.inc new file mode 100644 index 000000000000..29db67e19f08 --- /dev/null +++ b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/Fixture/skip_mixed_and_string.php.inc @@ -0,0 +1,25 @@ +value instanceof stdClass) { + return $this->getStringValue(); + } + + return $this->value; + } + + public function getStringValue(): string + { + return 'abc'; + } +} diff --git a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php index 448c875d15cb..c5a31a5d7cdd 100644 --- a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php +++ b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php @@ -10,6 +10,7 @@ final class ReturnTypeDeclarationRectorTest extends AbstractRectorTestCase public function test(): void { $files = [ + __DIR__ . '/Fixture/skip_mixed_and_string.php.inc', __DIR__ . '/Fixture/skip_self.php.inc', // static types __DIR__ . '/Fixture/void_type.php.inc', diff --git a/packages/TypeDeclaration/tests/Rector/Property/PropertyTypeDeclarationRector/Fixture/constructor_array_param_with_nullable.php.inc b/packages/TypeDeclaration/tests/Rector/Property/PropertyTypeDeclarationRector/Fixture/constructor_array_param_with_nullable.php.inc index d0a5b73fed94..fbee723d8e59 100644 --- a/packages/TypeDeclaration/tests/Rector/Property/PropertyTypeDeclarationRector/Fixture/constructor_array_param_with_nullable.php.inc +++ b/packages/TypeDeclaration/tests/Rector/Property/PropertyTypeDeclarationRector/Fixture/constructor_array_param_with_nullable.php.inc @@ -21,7 +21,7 @@ namespace Rector\TypeDeclaration\Tests\Rector\FunctionLike\PropertyTypeDeclarati class ConstructorArrayParamWithNullable { /** - * @var array|null + * @var mixed[]|null */ private $data;