From 762fc470a2b5308d4d0469457bac809e6ddc2196 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 5 Nov 2021 13:36:14 +0100 Subject: [PATCH] Support for tentative return types --- src/Php/PhpVersion.php | 5 +++ src/Reflection/MethodPrototypeReflection.php | 14 +++++- .../Native/NativeMethodReflection.php | 9 +++- .../Php/BuiltinMethodReflection.php | 2 + .../Php/FakeBuiltinMethodReflection.php | 5 +++ .../Php/NativeBuiltinMethodReflection.php | 9 ++++ src/Reflection/Php/PhpMethodReflection.php | 8 +++- src/Rules/Methods/OverridingMethodRule.php | 35 ++++++++++++++- .../Methods/OverridingMethodRuleTest.php | 38 ++++++++++++++++ .../Methods/data/tentative-return-types.php | 45 +++++++++++++++++++ 10 files changed, 165 insertions(+), 5 deletions(-) create mode 100644 tests/PHPStan/Rules/Methods/data/tentative-return-types.php diff --git a/src/Php/PhpVersion.php b/src/Php/PhpVersion.php index f085f65192..fb61ba1d01 100644 --- a/src/Php/PhpVersion.php +++ b/src/Php/PhpVersion.php @@ -152,4 +152,9 @@ public function supportsCaseInsensitiveConstantNames(): bool return $this->versionId < 80000; } + public function hasTentativeReturnTypes(): bool + { + return $this->versionId >= 80100; + } + } diff --git a/src/Reflection/MethodPrototypeReflection.php b/src/Reflection/MethodPrototypeReflection.php index cb2ecabc58..e9283fbe75 100644 --- a/src/Reflection/MethodPrototypeReflection.php +++ b/src/Reflection/MethodPrototypeReflection.php @@ -2,6 +2,8 @@ namespace PHPStan\Reflection; +use PHPStan\Type\Type; + class MethodPrototypeReflection implements ClassMemberReflection { @@ -22,6 +24,8 @@ class MethodPrototypeReflection implements ClassMemberReflection /** @var ParametersAcceptor[] */ private array $variants; + private ?Type $tentativeReturnType; + /** * @param string $name * @param ClassReflection $declaringClass @@ -31,6 +35,7 @@ class MethodPrototypeReflection implements ClassMemberReflection * @param bool $isAbstract * @param bool $isFinal * @param ParametersAcceptor[] $variants + * @param ?Type $tentativeReturnType */ public function __construct( string $name, @@ -40,7 +45,8 @@ public function __construct( bool $isPublic, bool $isAbstract, bool $isFinal, - array $variants + array $variants, + ?Type $tentativeReturnType ) { $this->name = $name; @@ -51,6 +57,7 @@ public function __construct( $this->isAbstract = $isAbstract; $this->isFinal = $isFinal; $this->variants = $variants; + $this->tentativeReturnType = $tentativeReturnType; } public function getName(): string @@ -101,4 +108,9 @@ public function getVariants(): array return $this->variants; } + public function getTentativeReturnType(): ?Type + { + return $this->tentativeReturnType; + } + } diff --git a/src/Reflection/Native/NativeMethodReflection.php b/src/Reflection/Native/NativeMethodReflection.php index 1410a99583..b4a55b824a 100644 --- a/src/Reflection/Native/NativeMethodReflection.php +++ b/src/Reflection/Native/NativeMethodReflection.php @@ -10,6 +10,7 @@ use PHPStan\Reflection\ReflectionProvider; use PHPStan\TrinaryLogic; use PHPStan\Type\Type; +use PHPStan\Type\TypehintHelper; use PHPStan\Type\VoidType; class NativeMethodReflection implements MethodReflection @@ -89,6 +90,11 @@ public function getPrototype(): ClassMemberReflection $prototypeMethod = $this->reflection->getPrototype(); $prototypeDeclaringClass = $this->reflectionProvider->getClass($prototypeMethod->getDeclaringClass()->getName()); + $tentativeReturnType = null; + if ($prototypeMethod->getTentativeReturnType() !== null) { + $tentativeReturnType = TypehintHelper::decideTypeFromReflection($prototypeMethod->getTentativeReturnType()); + } + return new MethodPrototypeReflection( $prototypeMethod->getName(), $prototypeDeclaringClass, @@ -97,7 +103,8 @@ public function getPrototype(): ClassMemberReflection $prototypeMethod->isPublic(), $prototypeMethod->isAbstract(), $prototypeMethod->isFinal(), - $prototypeDeclaringClass->getNativeMethod($prototypeMethod->getName())->getVariants() + $prototypeDeclaringClass->getNativeMethod($prototypeMethod->getName())->getVariants(), + $tentativeReturnType ); } catch (\ReflectionException $e) { return $this; diff --git a/src/Reflection/Php/BuiltinMethodReflection.php b/src/Reflection/Php/BuiltinMethodReflection.php index 266d002075..83153873b0 100644 --- a/src/Reflection/Php/BuiltinMethodReflection.php +++ b/src/Reflection/Php/BuiltinMethodReflection.php @@ -35,6 +35,8 @@ public function isVariadic(): bool; public function getReturnType(): ?\ReflectionType; + public function getTentativeReturnType(): ?\ReflectionType; + /** * @return \ReflectionParameter[] */ diff --git a/src/Reflection/Php/FakeBuiltinMethodReflection.php b/src/Reflection/Php/FakeBuiltinMethodReflection.php index cfb2d1b831..e6bd53c354 100644 --- a/src/Reflection/Php/FakeBuiltinMethodReflection.php +++ b/src/Reflection/Php/FakeBuiltinMethodReflection.php @@ -105,6 +105,11 @@ public function getReturnType(): ?\ReflectionType return null; } + public function getTentativeReturnType(): ?\ReflectionType + { + return null; + } + /** * @return \ReflectionParameter[] */ diff --git a/src/Reflection/Php/NativeBuiltinMethodReflection.php b/src/Reflection/Php/NativeBuiltinMethodReflection.php index 692c06a010..681a7c4429 100644 --- a/src/Reflection/Php/NativeBuiltinMethodReflection.php +++ b/src/Reflection/Php/NativeBuiltinMethodReflection.php @@ -124,6 +124,15 @@ public function getReturnType(): ?\ReflectionType return $this->reflection->getReturnType(); } + public function getTentativeReturnType(): ?\ReflectionType + { + if (method_exists($this->reflection, 'getTentativeReturnType')) { + return $this->reflection->getTentativeReturnType(); + } + + return null; + } + /** * @return \ReflectionParameter[] */ diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php index 94ce869502..b39acc065e 100644 --- a/src/Reflection/Php/PhpMethodReflection.php +++ b/src/Reflection/Php/PhpMethodReflection.php @@ -162,6 +162,11 @@ public function getPrototype(): ClassMemberReflection $prototypeMethod = $this->reflection->getPrototype(); $prototypeDeclaringClass = $this->reflectionProvider->getClass($prototypeMethod->getDeclaringClass()->getName()); + $tentativeReturnType = null; + if ($prototypeMethod->getTentativeReturnType() !== null) { + $tentativeReturnType = TypehintHelper::decideTypeFromReflection($prototypeMethod->getTentativeReturnType()); + } + return new MethodPrototypeReflection( $prototypeMethod->getName(), $prototypeDeclaringClass, @@ -170,7 +175,8 @@ public function getPrototype(): ClassMemberReflection $prototypeMethod->isPublic(), $prototypeMethod->isAbstract(), $prototypeMethod->isFinal(), - $prototypeDeclaringClass->getNativeMethod($prototypeMethod->getName())->getVariants() + $prototypeDeclaringClass->getNativeMethod($prototypeMethod->getName())->getVariants(), + $tentativeReturnType ); } catch (\ReflectionException $e) { return $this; diff --git a/src/Rules/Methods/OverridingMethodRule.php b/src/Rules/Methods/OverridingMethodRule.php index 56cdd11762..e19ef957b0 100644 --- a/src/Rules/Methods/OverridingMethodRule.php +++ b/src/Rules/Methods/OverridingMethodRule.php @@ -145,8 +145,28 @@ public function processNode(Node $node, Scope $scope): array $prototypeVariant = $prototypeVariants[0]; $methodVariant = ParametersAcceptorSelector::selectSingle($method->getVariants()); + $methodReturnType = $methodVariant->getNativeReturnType(); $methodParameters = $methodVariant->getParameters(); + if ( + $this->phpVersion->hasTentativeReturnTypes() + && $prototype->getTentativeReturnType() !== null + && !$this->hasReturnTypeWillChangeAttribute($node->getOriginalNode()) + ) { + + if (!$this->isTypeCompatible($prototype->getTentativeReturnType(), $methodVariant->getNativeReturnType(), true)) { + $messages[] = RuleErrorBuilder::message(sprintf( + 'Return type %s of method %s::%s() is not covariant with tentative return type %s of method %s::%s().', + $methodReturnType->describe(VerbosityLevel::typeOnly()), + $method->getDeclaringClass()->getDisplayName(), + $method->getName(), + $prototype->getTentativeReturnType()->describe(VerbosityLevel::typeOnly()), + $prototype->getDeclaringClass()->getDisplayName(), + $prototype->getName() + ))->tip('Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.')->nonIgnorable()->build(); + } + } + $prototypeAfterVariadic = false; foreach ($prototypeVariant->getParameters() as $i => $prototypeParameter) { if (!array_key_exists($i, $methodParameters)) { @@ -380,8 +400,6 @@ public function processNode(Node $node, Scope $scope): array } } - $methodReturnType = $methodVariant->getNativeReturnType(); - if (!$prototypeVariant instanceof FunctionVariantWithPhpDocs) { return $this->addErrors($messages, $node, $scope); } @@ -466,4 +484,17 @@ private function addErrors( return $this->methodSignatureRule->processNode($classMethod, $scope); } + private function hasReturnTypeWillChangeAttribute(Node\Stmt\ClassMethod $method): bool + { + foreach ($method->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + if ($attr->name->toLowerString() === 'returntypewillchange') { + return true; + } + } + } + + return false; + } + } diff --git a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php index 939caa2cd4..d9f25a83dc 100644 --- a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php +++ b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php @@ -516,4 +516,42 @@ public function testBug4516(): void $this->analyse([__DIR__ . '/data/bug-4516.php'], []); } + public function dataTentativeReturnTypes(): array + { + return [ + [70400, []], + [80000, []], + [ + 80100, + [ + [ + 'Return type mixed of method TentativeReturnTypes\Foo::getIterator() is not covariant with tentative return type Traversable of method IteratorAggregate::getIterator().', + 8, + 'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.', + ], + [ + 'Return type string of method TentativeReturnTypes\Lorem::getIterator() is not covariant with tentative return type Traversable of method IteratorAggregate::getIterator().', + 40, + 'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.', + ], + ], + ], + ]; + } + + /** + * @dataProvider dataTentativeReturnTypes + * @param int $phpVersionId + * @param mixed[] $errors + */ + public function testTentativeReturnTypes(int $phpVersionId, array $errors): void + { + if (!self::$useStaticReflectionProvider) { + $this->markTestSkipped('Test requires static reflection.'); + } + + $this->phpVersionId = $phpVersionId; + $this->analyse([__DIR__ . '/data/tentative-return-types.php'], $errors); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/tentative-return-types.php b/tests/PHPStan/Rules/Methods/data/tentative-return-types.php new file mode 100644 index 0000000000..d42f319ae0 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/tentative-return-types.php @@ -0,0 +1,45 @@ +