From 13fb5b148f683f3dc9c20fb380eca77e66ac5659 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 5 Jan 2020 17:02:11 +0100 Subject: [PATCH] Fixed wrong template types resolution when inheriting phpDocs --- .../Php/PhpClassReflectionExtension.php | 33 ++++++++++--------- stubs/iterable.php | 3 +- .../Arrays/OffsetAccessAssignmentRuleTest.php | 6 ++++ .../inherit-doc-template-type-resolution.php | 14 ++++++++ 4 files changed, 40 insertions(+), 16 deletions(-) create mode 100644 tests/PHPStan/Rules/Arrays/data/inherit-doc-template-type-resolution.php diff --git a/src/Reflection/Php/PhpClassReflectionExtension.php b/src/Reflection/Php/PhpClassReflectionExtension.php index 7f9afdf8a8..2de5c2204a 100644 --- a/src/Reflection/Php/PhpClassReflectionExtension.php +++ b/src/Reflection/Php/PhpClassReflectionExtension.php @@ -432,12 +432,13 @@ private function createMethod( $stubPhpDocParameterTypes = []; $stubPhpDocParameterVariadicity = []; if (count($variantNames) === 1) { - $stubPhpDoc = $this->findMethodPhpDocIncludingAncestors($declaringClassName, $methodReflection->getName(), array_map(static function (ParameterSignature $parameterSignature): string { + $stubPhpDocPair = $this->findMethodPhpDocIncludingAncestors($declaringClass, $methodReflection->getName(), array_map(static function (ParameterSignature $parameterSignature): string { return $parameterSignature->getName(); }, $methodSignature->getParameters())); - if ($stubPhpDoc !== null) { + if ($stubPhpDocPair !== null) { + [$stubPhpDoc, $stubDeclaringClass] = $stubPhpDocPair; $stubPhpDocString = $stubPhpDoc->getPhpDocString(); - $templateTypeMap = $declaringClass->getActiveTemplateTypeMap(); + $templateTypeMap = $stubDeclaringClass->getActiveTemplateTypeMap(); $returnTag = $stubPhpDoc->getReturnTag(); if ($returnTag !== null) { $stubPhpDocReturnType = $returnTag->getType(); @@ -491,9 +492,14 @@ private function createMethod( } $declaringTraitName = $this->findMethodTrait($methodReflection); - $resolvedPhpDoc = $this->findMethodPhpDocIncludingAncestors($declaringClassName, $methodReflection->getName(), array_map(static function (\ReflectionParameter $parameter): string { + $resolvedPhpDoc = null; + $stubPhpDocPair = $this->findMethodPhpDocIncludingAncestors($declaringClass, $methodReflection->getName(), array_map(static function (\ReflectionParameter $parameter): string { return $parameter->getName(); }, $methodReflection->getParameters())); + $phpDocBlockClassReflection = $declaringClass; + if ($stubPhpDocPair !== null) { + [$resolvedPhpDoc, $phpDocBlockClassReflection] = $stubPhpDocPair; + } $stubPhpDocString = null; $phpDocBlock = null; if ($resolvedPhpDoc === null) { @@ -526,7 +532,6 @@ private function createMethod( } } } else { - $phpDocBlockClassReflection = $declaringClass; $isPhpDocBlockExplicit = true; $stubPhpDocString = $resolvedPhpDoc->getPhpDocString(); } @@ -547,7 +552,7 @@ private function createMethod( $isInternal = false; $isFinal = false; if ($resolvedPhpDoc !== null) { - if (!isset($phpDocBlockClassReflection) || !isset($isPhpDocBlockExplicit)) { + if (!isset($isPhpDocBlockExplicit)) { throw new \PHPStan\ShouldNotHappenException(); } $templateTypeMap = $resolvedPhpDoc->getTemplateTypeMap(); @@ -883,25 +888,23 @@ private function getPhpDocReturnType(ClassReflection $phpDocBlockClassReflection } /** - * @param string $declaringClassName + * @param ClassReflection $declaringClass * @param string $methodName * @param array $positionalParameterNames - * @return \PHPStan\PhpDoc\ResolvedPhpDocBlock|null + * @return array{\PHPStan\PhpDoc\ResolvedPhpDocBlock, ClassReflection}|null */ - private function findMethodPhpDocIncludingAncestors(string $declaringClassName, string $methodName, array $positionalParameterNames): ?ResolvedPhpDocBlock + private function findMethodPhpDocIncludingAncestors(ClassReflection $declaringClass, string $methodName, array $positionalParameterNames): ?array { + $declaringClassName = $declaringClass->getName(); $resolved = $this->stubPhpDocProvider->findMethodPhpDoc($declaringClassName, $methodName, $positionalParameterNames); if ($resolved !== null) { - return $resolved; + return [$resolved, $declaringClass]; } if (!$this->stubPhpDocProvider->isKnownClass($declaringClassName)) { return null; } - if (!$this->reflectionProvider->hasClass($declaringClassName)) { - return null; - } - $ancestors = $this->reflectionProvider->getClass($declaringClassName)->getAncestors(); + $ancestors = $declaringClass->getAncestors(); foreach ($ancestors as $ancestor) { if ($ancestor->getName() === $declaringClassName) { continue; @@ -915,7 +918,7 @@ private function findMethodPhpDocIncludingAncestors(string $declaringClassName, continue; } - return $resolved; + return [$resolved, $ancestor]; } return null; diff --git a/stubs/iterable.php b/stubs/iterable.php index ce011cd266..a44d83b192 100644 --- a/stubs/iterable.php +++ b/stubs/iterable.php @@ -71,8 +71,9 @@ public function send($value) {} /** * @implements Traversable + * @implements ArrayAccess */ -class SimpleXMLElement implements Traversable +class SimpleXMLElement implements Traversable, ArrayAccess { } diff --git a/tests/PHPStan/Rules/Arrays/OffsetAccessAssignmentRuleTest.php b/tests/PHPStan/Rules/Arrays/OffsetAccessAssignmentRuleTest.php index cbb900db85..45c86e1f33 100644 --- a/tests/PHPStan/Rules/Arrays/OffsetAccessAssignmentRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/OffsetAccessAssignmentRuleTest.php @@ -167,4 +167,10 @@ public function testOffsetAccessAssignmentToScalarWithoutMaybes(): void ); } + public function testInheritDocTemplateTypeResolution(): void + { + $this->checkUnionTypes = true; + $this->analyse([__DIR__ . '/data/inherit-doc-template-type-resolution.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Arrays/data/inherit-doc-template-type-resolution.php b/tests/PHPStan/Rules/Arrays/data/inherit-doc-template-type-resolution.php new file mode 100644 index 0000000000..fb019ba628 --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/inherit-doc-template-type-resolution.php @@ -0,0 +1,14 @@ +