From 93402494ee92dd2e7264f112105151586203e0ec Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 15 May 2024 08:16:53 +0200 Subject: [PATCH] Fix resolving `self` and `static` in `@phpstan-closure-this` from trait stub file --- src/Analyser/NameScope.php | 14 +++ src/Analyser/NodeScopeResolver.php | 106 +++++++++--------- src/PhpDoc/PhpDocInheritanceResolver.php | 2 +- src/PhpDoc/ResolvedPhpDocBlock.php | 18 ++- src/PhpDoc/StubPhpDocProvider.php | 13 ++- .../Php/PhpClassReflectionExtension.php | 16 ++- tests/PHPStan/Analyser/Bug11009Test.php | 36 ++++++ tests/PHPStan/Analyser/bug-11009.neon | 3 + tests/PHPStan/Analyser/data/bug-11009.php | 45 ++++++++ tests/PHPStan/Analyser/data/bug-11009.stub | 21 ++++ 10 files changed, 211 insertions(+), 63 deletions(-) create mode 100644 tests/PHPStan/Analyser/Bug11009Test.php create mode 100644 tests/PHPStan/Analyser/bug-11009.neon create mode 100644 tests/PHPStan/Analyser/data/bug-11009.php create mode 100644 tests/PHPStan/Analyser/data/bug-11009.stub diff --git a/src/Analyser/NameScope.php b/src/Analyser/NameScope.php index af3688e107..24370dfe0b 100644 --- a/src/Analyser/NameScope.php +++ b/src/Analyser/NameScope.php @@ -171,6 +171,20 @@ public function withTemplateTypeMap(TemplateTypeMap $map): self ); } + public function withClassName(string $className): self + { + return new self( + $this->namespace, + $this->uses, + $className, + $this->functionName, + $this->templateTypeMap, + $this->typeAliasesMap, + $this->bypassTypeAliases, + $this->constUses, + ); + } + public function unsetTemplateType(string $name): self { $map = $this->templateTypeMap; diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 9abf5a6520..6ce976569f 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -2636,67 +2636,63 @@ static function (): void { $impurePoints = array_merge($impurePoints, $result->getImpurePoints()); $scope = $result->getScope(); } elseif ($expr->class instanceof Name) { - $className = $scope->resolveName($expr->class); - if ($this->reflectionProvider->hasClass($className)) { - $classReflection = $this->reflectionProvider->getClass($className); - $methodName = $expr->name->name; - if ($classReflection->hasMethod($methodName)) { - $methodReflection = $classReflection->getMethod($methodName, $scope); - $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs( - $scope, - $expr->getArgs(), - $methodReflection->getVariants(), - $methodReflection->getNamedArgumentsVariants(), - ); + $classType = $scope->resolveTypeByName($expr->class); + $methodName = $expr->name->name; + if ($classType->hasMethod($methodName)->yes()) { + $methodReflection = $classType->getMethod($methodName, $scope); + $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs( + $scope, + $expr->getArgs(), + $methodReflection->getVariants(), + $methodReflection->getNamedArgumentsVariants(), + ); - $methodThrowPoint = $this->getStaticMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope); - if ($methodThrowPoint !== null) { - $throwPoints[] = $methodThrowPoint; - } - if ( - $classReflection->getName() === 'Closure' - && strtolower($methodName) === 'bind' - ) { - $thisType = null; - $nativeThisType = null; - if (isset($expr->getArgs()[1])) { - $argType = $scope->getType($expr->getArgs()[1]->value); - if ($argType->isNull()->yes()) { - $thisType = null; - } else { - $thisType = $argType; - } + $methodThrowPoint = $this->getStaticMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope); + if ($methodThrowPoint !== null) { + $throwPoints[] = $methodThrowPoint; + } - $nativeArgType = $scope->getNativeType($expr->getArgs()[1]->value); - if ($nativeArgType->isNull()->yes()) { - $nativeThisType = null; - } else { - $nativeThisType = $nativeArgType; - } + $declaringClass = $methodReflection->getDeclaringClass(); + if ( + $declaringClass->getName() === 'Closure' + && strtolower($methodName) === 'bind' + ) { + $thisType = null; + $nativeThisType = null; + if (isset($expr->getArgs()[1])) { + $argType = $scope->getType($expr->getArgs()[1]->value); + if ($argType->isNull()->yes()) { + $thisType = null; + } else { + $thisType = $argType; + } + + $nativeArgType = $scope->getNativeType($expr->getArgs()[1]->value); + if ($nativeArgType->isNull()->yes()) { + $nativeThisType = null; + } else { + $nativeThisType = $nativeArgType; } - $scopeClasses = ['static']; - if (isset($expr->getArgs()[2])) { - $argValue = $expr->getArgs()[2]->value; - $argValueType = $scope->getType($argValue); - - $scopeClasses = []; - $directClassNames = $argValueType->getObjectClassNames(); - if (count($directClassNames) > 0) { - $scopeClasses = $directClassNames; - $thisTypes = []; - foreach ($directClassNames as $directClassName) { - $thisTypes[] = new ObjectType($directClassName); - } - $thisType = TypeCombinator::union(...$thisTypes); - } else { - $thisType = $argValueType->getClassStringObjectType(); - $scopeClasses = $thisType->getObjectClassNames(); + } + $scopeClasses = ['static']; + if (isset($expr->getArgs()[2])) { + $argValue = $expr->getArgs()[2]->value; + $argValueType = $scope->getType($argValue); + + $directClassNames = $argValueType->getObjectClassNames(); + if (count($directClassNames) > 0) { + $scopeClasses = $directClassNames; + $thisTypes = []; + foreach ($directClassNames as $directClassName) { + $thisTypes[] = new ObjectType($directClassName); } + $thisType = TypeCombinator::union(...$thisTypes); + } else { + $thisType = $argValueType->getClassStringObjectType(); + $scopeClasses = $thisType->getObjectClassNames(); } - $closureBindScope = $scope->enterClosureBind($thisType, $nativeThisType, $scopeClasses); } - } else { - $throwPoints[] = ThrowPoint::createImplicit($scope, $expr); + $closureBindScope = $scope->enterClosureBind($thisType, $nativeThisType, $scopeClasses); } } else { $throwPoints[] = ThrowPoint::createImplicit($scope, $expr); diff --git a/src/PhpDoc/PhpDocInheritanceResolver.php b/src/PhpDoc/PhpDocInheritanceResolver.php index a0b34db3e0..0e909dd96a 100644 --- a/src/PhpDoc/PhpDocInheritanceResolver.php +++ b/src/PhpDoc/PhpDocInheritanceResolver.php @@ -119,7 +119,7 @@ private function docBlockToResolvedDocBlock(PhpDocBlock $phpDocBlock, ?string $t $classReflection = $phpDocBlock->getClassReflection(); if ($functionName !== null && $classReflection->getNativeReflection()->hasMethod($functionName)) { $methodReflection = $classReflection->getNativeReflection()->getMethod($functionName); - $stub = $this->stubPhpDocProvider->findMethodPhpDoc($classReflection->getName(), $functionName, array_map(static fn (ReflectionParameter $parameter): string => $parameter->getName(), $methodReflection->getParameters())); + $stub = $this->stubPhpDocProvider->findMethodPhpDoc($classReflection->getName(), $classReflection->getName(), $functionName, array_map(static fn (ReflectionParameter $parameter): string => $parameter->getName(), $methodReflection->getParameters())); if ($stub !== null) { return $stub; } diff --git a/src/PhpDoc/ResolvedPhpDocBlock.php b/src/PhpDoc/ResolvedPhpDocBlock.php index e3931a60ea..4783a8f710 100644 --- a/src/PhpDoc/ResolvedPhpDocBlock.php +++ b/src/PhpDoc/ResolvedPhpDocBlock.php @@ -161,7 +161,7 @@ public static function create( ReflectionProvider $reflectionProvider, ): self { - // new property also needs to be added to createEmpty() and merge() + // new property also needs to be added to withNameScope(), createEmpty() and merge() $self = new self(); $self->phpDocNode = $phpDocNode; $self->phpDocNodes = [$phpDocNode]; @@ -176,6 +176,22 @@ public static function create( return $self; } + public function withNameScope(NameScope $nameScope): self + { + $self = new self(); + $self->phpDocNode = $this->phpDocNode; + $self->phpDocNodes = $this->phpDocNodes; + $self->phpDocString = $this->phpDocString; + $self->filename = $this->filename; + $self->nameScope = $nameScope; + $self->templateTypeMap = $this->templateTypeMap; + $self->templateTags = $this->templateTags; + $self->phpDocNodeResolver = $this->phpDocNodeResolver; + $self->reflectionProvider = $this->reflectionProvider; + + return $self; + } + public static function createEmpty(): self { // new property also needs to be added to merge() diff --git a/src/PhpDoc/StubPhpDocProvider.php b/src/PhpDoc/StubPhpDocProvider.php index b42350184a..d00085e985 100644 --- a/src/PhpDoc/StubPhpDocProvider.php +++ b/src/PhpDoc/StubPhpDocProvider.php @@ -146,7 +146,12 @@ public function findClassConstantPhpDoc(string $className, string $constantName) /** * @param array $positionalParameterNames */ - public function findMethodPhpDoc(string $className, string $methodName, array $positionalParameterNames): ?ResolvedPhpDocBlock + public function findMethodPhpDoc( + string $className, + string $implementingClassName, + string $methodName, + array $positionalParameterNames, + ): ?ResolvedPhpDocBlock { if (!$this->isKnownClass($className)) { return null; @@ -170,6 +175,12 @@ public function findMethodPhpDoc(string $className, string $methodName, array $p throw new ShouldNotHappenException(); } + if ($className !== $implementingClassName && $resolvedPhpDoc->getNullableNameScope() !== null) { + $resolvedPhpDoc = $resolvedPhpDoc->withNameScope( + $resolvedPhpDoc->getNullableNameScope()->withClassName($implementingClassName), + ); + } + $methodParameterNames = $this->knownMethodsParameterNames[$className][$methodName]; $parameterNameMapping = []; foreach ($positionalParameterNames as $i => $parameterName) { diff --git a/src/Reflection/Php/PhpClassReflectionExtension.php b/src/Reflection/Php/PhpClassReflectionExtension.php index 83a9df25eb..f3ff0aa2ab 100644 --- a/src/Reflection/Php/PhpClassReflectionExtension.php +++ b/src/Reflection/Php/PhpClassReflectionExtension.php @@ -502,7 +502,7 @@ private function createMethod( $stubImmediatelyInvokedCallableParameters = []; $stubClosureThisParameters = []; if (count($methodSignatures) === 1) { - $stubPhpDocPair = $this->findMethodPhpDocIncludingAncestors($declaringClass, $methodReflection->getName(), array_map(static fn (ParameterSignature $parameterSignature): string => $parameterSignature->getName(), $methodSignature->getParameters())); + $stubPhpDocPair = $this->findMethodPhpDocIncludingAncestors($declaringClass, $declaringClass, $methodReflection->getName(), array_map(static fn (ParameterSignature $parameterSignature): string => $parameterSignature->getName(), $methodSignature->getParameters())); if ($stubPhpDocPair !== null) { [$stubPhpDoc, $stubDeclaringClass] = $stubPhpDocPair; $templateTypeMap = $stubDeclaringClass->getActiveTemplateTypeMap(); @@ -637,7 +637,7 @@ private function createMethod( public function createUserlandMethodReflection(ClassReflection $fileDeclaringClass, ClassReflection $actualDeclaringClass, BuiltinMethodReflection $methodReflection, ?string $declaringTraitName): PhpMethodReflection { $resolvedPhpDoc = null; - $stubPhpDocPair = $this->findMethodPhpDocIncludingAncestors($fileDeclaringClass, $methodReflection->getName(), array_map(static fn (ReflectionParameter $parameter): string => $parameter->getName(), $methodReflection->getParameters())); + $stubPhpDocPair = $this->findMethodPhpDocIncludingAncestors($fileDeclaringClass, $fileDeclaringClass, $methodReflection->getName(), array_map(static fn (ReflectionParameter $parameter): string => $parameter->getName(), $methodReflection->getParameters())); $phpDocBlockClassReflection = $fileDeclaringClass; if ($methodReflection->getReflection() !== null) { @@ -647,6 +647,7 @@ public function createUserlandMethodReflection(ClassReflection $fileDeclaringCla if (! $methodReflection->getDeclaringClass()->isTrait() || $methodDeclaringClass->getName() !== $methodReflection->getDeclaringClass()->getName()) { $stubPhpDocPair = $this->findMethodPhpDocIncludingAncestors( $this->reflectionProviderProvider->getReflectionProvider()->getClass($methodDeclaringClass->getName()), + $this->reflectionProviderProvider->getReflectionProvider()->getClass($methodReflection->getDeclaringClass()->getName()), $methodReflection->getName(), array_map( static fn (ReflectionParameter $parameter): string => $parameter->getName(), @@ -1126,10 +1127,15 @@ private function getPhpDocReturnType(ClassReflection $phpDocBlockClassReflection * @param array $positionalParameterNames * @return array{ResolvedPhpDocBlock, ClassReflection}|null */ - private function findMethodPhpDocIncludingAncestors(ClassReflection $declaringClass, string $methodName, array $positionalParameterNames): ?array + private function findMethodPhpDocIncludingAncestors( + ClassReflection $declaringClass, + ClassReflection $implementingClass, + string $methodName, + array $positionalParameterNames, + ): ?array { $declaringClassName = $declaringClass->getName(); - $resolved = $this->stubPhpDocProvider->findMethodPhpDoc($declaringClassName, $methodName, $positionalParameterNames); + $resolved = $this->stubPhpDocProvider->findMethodPhpDoc($declaringClassName, $implementingClass->getName(), $methodName, $positionalParameterNames); if ($resolved !== null) { return [$resolved, $declaringClass]; } @@ -1146,7 +1152,7 @@ private function findMethodPhpDocIncludingAncestors(ClassReflection $declaringCl continue; } - $resolved = $this->stubPhpDocProvider->findMethodPhpDoc($ancestor->getName(), $methodName, $positionalParameterNames); + $resolved = $this->stubPhpDocProvider->findMethodPhpDoc($ancestor->getName(), $ancestor->getName(), $methodName, $positionalParameterNames); if ($resolved === null) { continue; } diff --git a/tests/PHPStan/Analyser/Bug11009Test.php b/tests/PHPStan/Analyser/Bug11009Test.php new file mode 100644 index 0000000000..480e170756 --- /dev/null +++ b/tests/PHPStan/Analyser/Bug11009Test.php @@ -0,0 +1,36 @@ +gatherAssertTypes(__DIR__ . '/data/bug-11009.php'); + } + + /** + * @dataProvider dataFileAsserts + * @param mixed ...$args + */ + public function testFileAsserts( + string $assertType, + string $file, + ...$args, + ): void + { + $this->assertFileAsserts($assertType, $file, ...$args); + } + + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/../../../conf/bleedingEdge.neon', + __DIR__ . '/bug-11009.neon', + ]; + } + +} diff --git a/tests/PHPStan/Analyser/bug-11009.neon b/tests/PHPStan/Analyser/bug-11009.neon new file mode 100644 index 0000000000..d9a90c70b4 --- /dev/null +++ b/tests/PHPStan/Analyser/bug-11009.neon @@ -0,0 +1,3 @@ +parameters: + stubFiles: + - data/bug-11009.stub diff --git a/tests/PHPStan/Analyser/data/bug-11009.php b/tests/PHPStan/Analyser/data/bug-11009.php new file mode 100644 index 0000000000..1eea19fe18 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-11009.php @@ -0,0 +1,45 @@ +returnStatic()); + assertType(B::class, $b->returnSelf()); +}; diff --git a/tests/PHPStan/Analyser/data/bug-11009.stub b/tests/PHPStan/Analyser/data/bug-11009.stub new file mode 100644 index 0000000000..dce4347bb4 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-11009.stub @@ -0,0 +1,21 @@ +