From f658b3cd29c24ccf4b6a17c7cd97bd4f4c229ad6 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 28 Apr 2025 14:59:52 +0200 Subject: [PATCH 1/2] Resolve `ExtendedFunctionVariant::getReturnType()` more lazily This will allow the `$this` or `static` in PHPDoc return type to be first replaced with final-overriden ObjectType before being thrown into `TypehintHelper::decideType()`. --- src/Reflection/ExtendedFunctionVariant.php | 20 +++++++- .../Php/PhpClassReflectionExtension.php | 7 ++- .../PhpFunctionFromParserNodeReflection.php | 2 +- src/Reflection/Php/PhpFunctionReflection.php | 2 +- src/Reflection/Php/PhpMethodReflection.php | 49 +++++++++---------- .../NativeFunctionReflectionProvider.php | 2 +- ...ackUnresolvedMethodPrototypeReflection.php | 14 +++--- ...ypeUnresolvedMethodPrototypeReflection.php | 12 ++--- .../Type/IntersectionTypeMethodReflection.php | 3 +- .../nsrt/conditional-return-static-union.php | 45 +++++++++++++++++ 10 files changed, 107 insertions(+), 49 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/conditional-return-static-union.php diff --git a/src/Reflection/ExtendedFunctionVariant.php b/src/Reflection/ExtendedFunctionVariant.php index e45f402bb0..d427a8b025 100644 --- a/src/Reflection/ExtendedFunctionVariant.php +++ b/src/Reflection/ExtendedFunctionVariant.php @@ -5,6 +5,7 @@ use PHPStan\Type\Generic\TemplateTypeMap; use PHPStan\Type\Generic\TemplateTypeVarianceMap; use PHPStan\Type\Type; +use PHPStan\Type\TypehintHelper; /** * @api @@ -21,7 +22,7 @@ public function __construct( ?TemplateTypeMap $resolvedTemplateTypeMap, array $parameters, bool $isVariadic, - Type $returnType, + private ?Type $returnType, private Type $phpDocReturnType, private Type $nativeReturnType, ?TemplateTypeVarianceMap $callSiteVarianceMap = null, @@ -32,7 +33,10 @@ public function __construct( $resolvedTemplateTypeMap, $parameters, $isVariadic, - $returnType, + $returnType ?? TypehintHelper::decideType( + $nativeReturnType, + $phpDocReturnType, + ), $callSiteVarianceMap, ); } @@ -48,6 +52,18 @@ public function getParameters(): array return $parameters; } + public function getReturnType(): Type + { + if ($this->returnType === null) { + return $this->returnType = TypehintHelper::decideType( + $this->nativeReturnType, + $this->phpDocReturnType, + ); + } + + return $this->returnType; + } + public function getPhpDocReturnType(): Type { return $this->phpDocReturnType; diff --git a/src/Reflection/Php/PhpClassReflectionExtension.php b/src/Reflection/Php/PhpClassReflectionExtension.php index fc0dd70dbe..7fb79de620 100644 --- a/src/Reflection/Php/PhpClassReflectionExtension.php +++ b/src/Reflection/Php/PhpClassReflectionExtension.php @@ -954,10 +954,9 @@ private function createNativeMethodVariant( } if ($stubPhpDocReturnType !== null) { - $returnType = $stubPhpDocReturnType; $phpDocReturnType = $stubPhpDocReturnType; } else { - $returnType = TypehintHelper::decideType($methodSignature->getReturnType(), $phpDocReturnType); + $phpDocReturnType = TypehintHelper::decideType($methodSignature->getReturnType(), $phpDocReturnType); } return new ExtendedFunctionVariant( @@ -965,8 +964,8 @@ private function createNativeMethodVariant( null, $parameters, $methodSignature->isVariadic(), - $returnType, - $phpDocReturnType ?? new MixedType(), + null, + $phpDocReturnType, $methodSignature->getNativeReturnType(), ); } diff --git a/src/Reflection/Php/PhpFunctionFromParserNodeReflection.php b/src/Reflection/Php/PhpFunctionFromParserNodeReflection.php index 02a5b642af..8db7ae10d7 100644 --- a/src/Reflection/Php/PhpFunctionFromParserNodeReflection.php +++ b/src/Reflection/Php/PhpFunctionFromParserNodeReflection.php @@ -112,7 +112,7 @@ public function getVariants(): array $this->getResolvedTemplateTypeMap(), $this->getParameters(), $this->isVariadic(), - $this->getReturnType(), + null, $this->getPhpDocReturnType(), $this->getNativeReturnType(), ), diff --git a/src/Reflection/Php/PhpFunctionReflection.php b/src/Reflection/Php/PhpFunctionReflection.php index 368ac3f471..8e0433b30e 100644 --- a/src/Reflection/Php/PhpFunctionReflection.php +++ b/src/Reflection/Php/PhpFunctionReflection.php @@ -94,7 +94,7 @@ public function getVariants(): array null, $this->getParameters(), $this->isVariadic(), - $this->getReturnType(), + null, $this->getPhpDocReturnType(), $this->getNativeReturnType(), ), diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php index 432fd69350..ed22649679 100644 --- a/src/Reflection/Php/PhpMethodReflection.php +++ b/src/Reflection/Php/PhpMethodReflection.php @@ -202,7 +202,7 @@ public function getVariants(): array null, $this->getParameters(), $this->isVariadic(), - $this->getReturnType(), + null, $this->getPhpDocReturnType(), $this->getNativeReturnType(), ), @@ -302,30 +302,9 @@ public function isPublic(): bool private function getReturnType(): Type { if ($this->returnType === null) { - $name = strtolower($this->getName()); - $returnType = $this->reflection->getReturnType(); - if ($returnType === null) { - if (in_array($name, ['__construct', '__destruct', '__unset', '__wakeup', '__clone'], true)) { - return $this->returnType = TypehintHelper::decideType(new VoidType(), $this->phpDocReturnType); - } - if ($name === '__tostring') { - return $this->returnType = TypehintHelper::decideType(new StringType(), $this->phpDocReturnType); - } - if ($name === '__isset') { - return $this->returnType = TypehintHelper::decideType(new BooleanType(), $this->phpDocReturnType); - } - if ($name === '__sleep') { - return $this->returnType = TypehintHelper::decideType(new ArrayType(new IntegerType(), new StringType()), $this->phpDocReturnType); - } - if ($name === '__set_state') { - return $this->returnType = TypehintHelper::decideType(new ObjectWithoutClassType(), $this->phpDocReturnType); - } - } - - $this->returnType = TypehintHelper::decideTypeFromReflection( - $returnType, + $this->returnType = TypehintHelper::decideType( + $this->getNativeReturnType(), $this->phpDocReturnType, - $this->declaringClass, ); } @@ -344,8 +323,28 @@ private function getPhpDocReturnType(): Type private function getNativeReturnType(): Type { if ($this->nativeReturnType === null) { + $returnType = $this->reflection->getReturnType(); + if ($returnType === null) { + $name = strtolower($this->getName()); + if (in_array($this->getName(), ['__construct', '__destruct', '__unset', '__wakeup', '__clone'], true)) { + return $this->nativeReturnType = new VoidType(); + } + if ($name === '__tostring') { + return $this->nativeReturnType = new StringType(); + } + if ($name === '__isset') { + return $this->nativeReturnType = new BooleanType(); + } + if ($name === '__sleep') { + return $this->nativeReturnType = new ArrayType(new IntegerType(), new StringType()); + } + if ($name === '__set_state') { + return $this->nativeReturnType = new ObjectWithoutClassType(); + } + } + $this->nativeReturnType = TypehintHelper::decideTypeFromReflection( - $this->reflection->getReturnType(), + $returnType, null, $this->declaringClass, ); diff --git a/src/Reflection/SignatureMap/NativeFunctionReflectionProvider.php b/src/Reflection/SignatureMap/NativeFunctionReflectionProvider.php index 766e665115..9e11fe44cc 100644 --- a/src/Reflection/SignatureMap/NativeFunctionReflectionProvider.php +++ b/src/Reflection/SignatureMap/NativeFunctionReflectionProvider.php @@ -139,7 +139,7 @@ public function findFunctionReflection(string $functionName): ?NativeFunctionRef ); }, $functionSignature->getParameters()), $functionSignature->isVariadic(), - TypehintHelper::decideType($functionSignature->getReturnType(), $phpDocReturnType), + null, $phpDocReturnType ?? new MixedType(), $functionSignature->getReturnType(), ); diff --git a/src/Reflection/Type/CallbackUnresolvedMethodPrototypeReflection.php b/src/Reflection/Type/CallbackUnresolvedMethodPrototypeReflection.php index 980a3f293f..8c3243cd8e 100644 --- a/src/Reflection/Type/CallbackUnresolvedMethodPrototypeReflection.php +++ b/src/Reflection/Type/CallbackUnresolvedMethodPrototypeReflection.php @@ -86,12 +86,12 @@ private function transformMethodWithStaticType(ClassReflection $declaringClass, { $selfOutType = $method->getSelfOutType() !== null ? $this->transformStaticType($method->getSelfOutType()) : null; $variantFn = function (ExtendedParametersAcceptor $acceptor) use (&$selfOutType): ExtendedParametersAcceptor { - $originalReturnType = $acceptor->getReturnType(); - if ($originalReturnType instanceof ThisType && $selfOutType !== null) { - $returnType = TypeCombinator::intersect($selfOutType, $this->transformStaticType($originalReturnType)); - $selfOutType = $returnType; + $originalPhpDocReturnType = $acceptor->getPhpDocReturnType(); + if ($originalPhpDocReturnType instanceof ThisType && $selfOutType !== null) { + $phpDocReturnType = TypeCombinator::intersect($selfOutType, $this->transformStaticType($originalPhpDocReturnType)); + $selfOutType = $phpDocReturnType; } else { - $returnType = $this->transformStaticType($originalReturnType); + $phpDocReturnType = $this->transformStaticType($originalPhpDocReturnType); } return new ExtendedFunctionVariant( $acceptor->getTemplateTypeMap(), @@ -114,8 +114,8 @@ private function transformMethodWithStaticType(ClassReflection $declaringClass, $acceptor->getParameters(), ), $acceptor->isVariadic(), - $returnType, - $this->transformStaticType($acceptor->getPhpDocReturnType()), + null, + $phpDocReturnType, $this->transformStaticType($acceptor->getNativeReturnType()), $acceptor->getCallSiteVarianceMap(), ); diff --git a/src/Reflection/Type/CalledOnTypeUnresolvedMethodPrototypeReflection.php b/src/Reflection/Type/CalledOnTypeUnresolvedMethodPrototypeReflection.php index c78a435583..578129fb26 100644 --- a/src/Reflection/Type/CalledOnTypeUnresolvedMethodPrototypeReflection.php +++ b/src/Reflection/Type/CalledOnTypeUnresolvedMethodPrototypeReflection.php @@ -82,11 +82,11 @@ private function transformMethodWithStaticType(ClassReflection $declaringClass, { $selfOutType = $method->getSelfOutType() !== null ? $this->transformStaticType($method->getSelfOutType()) : null; $variantFn = function (ExtendedParametersAcceptor $acceptor) use ($selfOutType): ExtendedParametersAcceptor { - $originalReturnType = $acceptor->getReturnType(); - if ($originalReturnType instanceof ThisType && $selfOutType !== null) { - $returnType = $selfOutType; + $originalPhpDocReturnType = $acceptor->getPhpDocReturnType(); + if ($originalPhpDocReturnType instanceof ThisType && $selfOutType !== null) { + $phpDocReturnType = $selfOutType; } else { - $returnType = $this->transformStaticType($originalReturnType); + $phpDocReturnType = $this->transformStaticType($originalPhpDocReturnType); } return new ExtendedFunctionVariant( $acceptor->getTemplateTypeMap(), @@ -109,8 +109,8 @@ private function transformMethodWithStaticType(ClassReflection $declaringClass, $acceptor->getParameters(), ), $acceptor->isVariadic(), - $returnType, - $this->transformStaticType($acceptor->getPhpDocReturnType()), + null, + $phpDocReturnType, $this->transformStaticType($acceptor->getNativeReturnType()), $acceptor->getCallSiteVarianceMap(), ); diff --git a/src/Reflection/Type/IntersectionTypeMethodReflection.php b/src/Reflection/Type/IntersectionTypeMethodReflection.php index eafd314157..a978ad1b6d 100644 --- a/src/Reflection/Type/IntersectionTypeMethodReflection.php +++ b/src/Reflection/Type/IntersectionTypeMethodReflection.php @@ -79,7 +79,6 @@ public function getPrototype(): ClassMemberReflection public function getVariants(): array { - $returnType = TypeCombinator::intersect(...array_map(static fn (MethodReflection $method): Type => TypeCombinator::intersect(...array_map(static fn (ParametersAcceptor $acceptor): Type => $acceptor->getReturnType(), $method->getVariants())), $this->methods)); $phpDocReturnType = TypeCombinator::intersect(...array_map(static fn (MethodReflection $method): Type => TypeCombinator::intersect(...array_map(static fn (ParametersAcceptor $acceptor): Type => $acceptor->getPhpDocReturnType(), $method->getVariants())), $this->methods)); $nativeReturnType = TypeCombinator::intersect(...array_map(static fn (MethodReflection $method): Type => TypeCombinator::intersect(...array_map(static fn (ParametersAcceptor $acceptor): Type => $acceptor->getNativeReturnType(), $method->getVariants())), $this->methods)); @@ -88,7 +87,7 @@ public function getVariants(): array $acceptor->getResolvedTemplateTypeMap(), $acceptor->getParameters(), $acceptor->isVariadic(), - $returnType, + null, $phpDocReturnType, $nativeReturnType, $acceptor->getCallSiteVarianceMap(), diff --git a/tests/PHPStan/Analyser/nsrt/conditional-return-static-union.php b/tests/PHPStan/Analyser/nsrt/conditional-return-static-union.php new file mode 100644 index 0000000000..d706a15936 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/conditional-return-static-union.php @@ -0,0 +1,45 @@ +|Config $value + * @return ($value is array ? Config : $this) + */ + public function invalidReturn(array|Config $value = []): Config|static + { + if (is_array($value)) { + return new Config(); + } + return $this; + } + + /** + * @param array|Config $value + * @return ($value is array ? Config : $this) + */ + public function validReturn(array|Config $value = []): Config|self + { + if (is_array($value)) { + return new Config(); + } + return $this; + } +} + +function (MainConfig $c): void { + assertType(Config::class, (new MainConfig())->invalidReturn()); + assertType(Config::class, (new MainConfig())->validReturn()); + assertType(MainConfig::class, (new MainConfig())->invalidReturn(new Config())); + assertType(MainConfig::class, (new MainConfig())->validReturn(new Config())); + + assertType(Config::class, $c->invalidReturn()); + assertType(Config::class, $c->validReturn()); + assertType(MainConfig::class, $c->invalidReturn(new Config())); + assertType(MainConfig::class, $c->validReturn(new Config())); +}; From c6fda38a2fd1c95017b5304aededd524e5ed66ee Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 28 Apr 2025 21:17:16 +0200 Subject: [PATCH 2/2] Allow Traversable in PHPDoc and Iterator in native type to cooperate --- phpstan-baseline.neon | 6 ++++++ src/Type/TypehintHelper.php | 18 +++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index dc8fcfbcc1..4516e5b2d0 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1854,6 +1854,12 @@ parameters: count: 1 path: src/Type/TypehintHelper.php + - + message: '#^Doing instanceof PHPStan\\Type\\TypeWithClassName is error\-prone and deprecated\. Use Type\:\:getObjectClassNames\(\) or Type\:\:getObjectClassReflections\(\) instead\.$#' + identifier: phpstanApi.instanceofType + count: 1 + path: src/Type/TypehintHelper.php + - message: '#^Doing instanceof PHPStan\\Type\\CallableType is error\-prone and deprecated\. Use Type\:\:isCallable\(\) and Type\:\:getCallableParametersAcceptors\(\) instead\.$#' identifier: phpstanApi.instanceofType diff --git a/src/Type/TypehintHelper.php b/src/Type/TypehintHelper.php index 333d9de4a7..744ae8e695 100644 --- a/src/Type/TypehintHelper.php +++ b/src/Type/TypehintHelper.php @@ -12,6 +12,7 @@ use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\Generic\TemplateTypeHelper; use ReflectionType; +use Traversable; use function array_map; use function count; use function get_class; @@ -118,9 +119,24 @@ public static function decideType( } } + $resolvedPhpDocTypeToBounds = TemplateTypeHelper::resolveToBounds($phpDocType); + $isSuperType = $type->isSuperTypeOf($resolvedPhpDocTypeToBounds); + if ( + !$isSuperType->yes() + && (new ObjectType(Traversable::class))->isSuperTypeOf($phpDocType)->yes() + && $type instanceof TypeWithClassName + ) { + // if native type is Iterator and PHPDoc type is Traversable + // Allow PHPDoc type to win + $traversableAncestor = $type->getAncestorWithClassName(Traversable::class); + if ($traversableAncestor !== null) { + $isSuperType = $traversableAncestor->isSuperTypeOf($resolvedPhpDocTypeToBounds); + } + } + if ( (!$phpDocType instanceof NeverType || ($type instanceof MixedType && !$type->isExplicitMixed())) - && $type->isSuperTypeOf(TemplateTypeHelper::resolveToBounds($phpDocType))->yes() + && $isSuperType->yes() ) { $resultType = $phpDocType; } else {