Skip to content

Commit

Permalink
Fixed wrong template types resolution when inheriting phpDocs
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Jan 5, 2020
1 parent 3f20c2d commit 13fb5b1
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 16 deletions.
33 changes: 18 additions & 15 deletions src/Reflection/Php/PhpClassReflectionExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -526,7 +532,6 @@ private function createMethod(
}
}
} else {
$phpDocBlockClassReflection = $declaringClass;
$isPhpDocBlockExplicit = true;
$stubPhpDocString = $resolvedPhpDoc->getPhpDocString();
}
Expand All @@ -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();
Expand Down Expand Up @@ -883,25 +888,23 @@ private function getPhpDocReturnType(ClassReflection $phpDocBlockClassReflection
}

/**
* @param string $declaringClassName
* @param ClassReflection $declaringClass
* @param string $methodName
* @param array<int, string> $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;
Expand All @@ -915,7 +918,7 @@ private function findMethodPhpDocIncludingAncestors(string $declaringClassName,
continue;
}

return $resolved;
return [$resolved, $ancestor];
}

return null;
Expand Down
3 changes: 2 additions & 1 deletion stubs/iterable.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ public function send($value) {}

/**
* @implements Traversable<mixed, mixed>
* @implements ArrayAccess<mixed, mixed>
*/
class SimpleXMLElement implements Traversable
class SimpleXMLElement implements Traversable, ArrayAccess
{

}
Expand Down
6 changes: 6 additions & 0 deletions tests/PHPStan/Rules/Arrays/OffsetAccessAssignmentRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'], []);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

namespace InheritDocTemplateTypeResolution;

class Foo extends \SimpleXMLElement
{

public function removeThis(): void
{
unset($this[0]);

}

}

0 comments on commit 13fb5b1

Please sign in to comment.