From 72b363f60a248207e53c9c14a633a9e780407bf5 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 2 Sep 2019 20:38:25 +0200 Subject: [PATCH] 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 fdaa5030347..7362b6c67d6 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/AbstractTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/AbstractTypeDeclarationRector.php @@ -140,11 +140,8 @@ abstract class AbstractTypeDeclarationRector extends AbstractRector $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 b8a6c0f2544..9abb198b80c 100644 --- a/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php @@ -4,7 +4,6 @@ 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; use PhpParser\Node\Stmt\Function_; @@ -17,6 +16,9 @@ use Rector\RectorDefinition\RectorDefinition; 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 @@ CODE_SAMPLE $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 @@ CODE_SAMPLE 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 be78fda8445..00000000000 --- 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 45b00cb0c2d..8948ff152ec 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 49bf7874a9d..970a9782204 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 00000000000..79ca8637d88 --- /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 e27cf0f61ab..5df48dd7c04 100644 --- a/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php +++ b/packages/TypeDeclaration/tests/Rector/FunctionLike/ReturnTypeDeclarationRector/ReturnTypeDeclarationRectorTest.php @@ -5,9 +5,6 @@ namespace Rector\TypeDeclaration\Tests\Rector\FunctionLike\ReturnTypeDeclaration 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 @@ final class ReturnTypeDeclarationRectorTest extends AbstractRectorTestCase { $files = [ __DIR__ . '/Fixture/nikic/inheritance.php.inc', - __DIR__ . '/Fixture/nikic/inheritance_covariance.php.inc', __DIR__ . '/Fixture/nikic/nullable_inheritance.php.inc', ];