Skip to content

Commit 320381f

Browse files
github-actions[bot]phpstan-bot
authored andcommitted
Fix phpstan/phpstan#7317: SimpleXMLElement::current() tentative return type not checked
- The tentative return type check in OverridingMethodRule only checked the deepest prototype (e.g., Iterator::current() with tentative type mixed), missing more specific tentative types from intermediate parent classes (e.g., SimpleXMLElement::current() with tentative type SimpleXMLElement) - Added getParentMethodTentativeReturnType() to also check the direct parent class method's tentative return type and use the more specific one - Updated Bug9615 test expectation to reflect the more accurate parent class reference - New regression test in tests/PHPStan/Rules/Methods/data/bug-7317.php
1 parent c36922b commit 320381f

File tree

4 files changed

+88
-9
lines changed

4 files changed

+88
-9
lines changed

phpstan-baseline.neon

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1940,3 +1940,9 @@ parameters:
19401940
identifier: phpstanApi.varTagAssumption
19411941
count: 1
19421942
path: tests/PHPStan/Type/IterableTypeTest.php
1943+
1944+
-
1945+
rawMessage: 'Unused PHPStan\Reflection\MethodPrototypeReflection::getName'
1946+
identifier: shipmonk.deadMethod
1947+
count: 1
1948+
path: src/Reflection/MethodPrototypeReflection.php

src/Rules/Methods/OverridingMethodRule.php

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,15 @@
1111
use PHPStan\DependencyInjection\ValidatesStubFiles;
1212
use PHPStan\Node\InClassMethodNode;
1313
use PHPStan\Php\PhpVersion;
14+
use PHPStan\Reflection\ClassReflection;
1415
use PHPStan\Reflection\ExtendedFunctionVariant;
1516
use PHPStan\Reflection\MethodPrototypeReflection;
1617
use PHPStan\Rules\IdentifierRuleError;
1718
use PHPStan\Rules\Rule;
1819
use PHPStan\Rules\RuleErrorBuilder;
1920
use PHPStan\Type\MixedType;
21+
use PHPStan\Type\Type;
22+
use PHPStan\Type\TypehintHelper;
2023
use PHPStan\Type\VerbosityLevel;
2124
use function array_merge;
2225
use function count;
@@ -203,22 +206,45 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array
203206

204207
$realPrototype = $method->getPrototype();
205208

209+
$effectiveTentativeReturnType = null;
210+
$tentativeDeclaringClass = null;
211+
$hasTentativeReturnType = false;
212+
213+
if ($realPrototype instanceof MethodPrototypeReflection && $realPrototype->getTentativeReturnType() !== null) {
214+
$effectiveTentativeReturnType = $realPrototype->getTentativeReturnType();
215+
$tentativeDeclaringClass = $realPrototype->getDeclaringClass();
216+
$hasTentativeReturnType = true;
217+
}
218+
219+
// The parent class method may have a more specific tentative return type
220+
// than the deepest prototype (e.g., SimpleXMLElement::current() has tentative
221+
// type SimpleXMLElement, while Iterator::current() has tentative type mixed)
222+
$parentTentativeReturnType = $this->getParentMethodTentativeReturnType($prototypeDeclaringClass, $prototype->getName());
223+
if ($parentTentativeReturnType !== null) {
224+
$hasTentativeReturnType = true;
225+
if ($effectiveTentativeReturnType === null || !$parentTentativeReturnType->isSuperTypeOf($effectiveTentativeReturnType)->yes()) {
226+
$effectiveTentativeReturnType = $parentTentativeReturnType;
227+
$tentativeDeclaringClass = $prototypeDeclaringClass;
228+
}
229+
}
230+
206231
if (
207-
$realPrototype instanceof MethodPrototypeReflection
232+
$hasTentativeReturnType
233+
&& $effectiveTentativeReturnType !== null
234+
&& $tentativeDeclaringClass !== null
208235
&& $this->phpVersion->hasTentativeReturnTypes()
209-
&& $realPrototype->getTentativeReturnType() !== null
210236
&& !$this->hasReturnTypeWillChangeAttribute($node->getOriginalNode())
211237
&& count($prototypeDeclaringClass->getNativeReflection()->getMethod($prototype->getName())->getAttributes('ReturnTypeWillChange')) === 0
212238
) {
213-
if (!$this->methodParameterComparisonHelper->isReturnTypeCompatible($realPrototype->getTentativeReturnType(), $method->getNativeReturnType(), true)) {
239+
if (!$this->methodParameterComparisonHelper->isReturnTypeCompatible($effectiveTentativeReturnType, $method->getNativeReturnType(), true)) {
214240
$messages[] = RuleErrorBuilder::message(sprintf(
215241
'Return type %s of method %s::%s() is not covariant with tentative return type %s of method %s::%s().',
216242
$methodReturnType->describe(VerbosityLevel::typeOnly()),
217243
$method->getDeclaringClass()->getDisplayName(),
218244
$method->getName(),
219-
$realPrototype->getTentativeReturnType()->describe(VerbosityLevel::typeOnly()),
220-
$realPrototype->getDeclaringClass()->getDisplayName(true),
221-
$realPrototype->getName(),
245+
$effectiveTentativeReturnType->describe(VerbosityLevel::typeOnly()),
246+
$tentativeDeclaringClass->getDisplayName(true),
247+
$prototype->getName(),
222248
))
223249
->tip('Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.')
224250
->nonIgnorable()
@@ -236,8 +262,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array
236262
$prototypeReturnType = $prototypeVariant->getNativeReturnType();
237263
$reportReturnType = true;
238264
if ($this->phpVersion->hasTentativeReturnTypes()) {
239-
$reportReturnType = !$realPrototype instanceof MethodPrototypeReflection
240-
|| $realPrototype->getTentativeReturnType() === null
265+
$reportReturnType = !$hasTentativeReturnType
241266
|| (is_bool($prototype->isBuiltin()) ? !$prototype->isBuiltin() : $prototype->isBuiltin()->no());
242267
} else {
243268
if ($realPrototype instanceof MethodPrototypeReflection && $realPrototype->isInternal()) {
@@ -371,4 +396,19 @@ private function hasOverrideAttribute(Node\Stmt\ClassMethod $method): bool
371396
return false;
372397
}
373398

399+
private function getParentMethodTentativeReturnType(ClassReflection $prototypeDeclaringClass, string $methodName): ?Type
400+
{
401+
$prototypeNativeReflection = $prototypeDeclaringClass->getNativeReflection();
402+
if (!$prototypeNativeReflection->hasMethod($methodName)) {
403+
return null;
404+
}
405+
406+
$prototypeReflMethod = $prototypeNativeReflection->getMethod($methodName);
407+
if (!$prototypeReflMethod->hasTentativeReturnType()) {
408+
return null;
409+
}
410+
411+
return TypehintHelper::decideTypeFromReflection($prototypeReflMethod->getTentativeReturnType(), selfClass: $prototypeDeclaringClass);
412+
}
413+
374414
}

tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,7 @@ public function testBug9615(): void
677677
$tipText,
678678
],
679679
[
680-
'Return type mixed of method Bug9615\ExpectComplaintsHere::getChildren() is not covariant with tentative return type RecursiveIterator|null of method RecursiveIterator<mixed,mixed>::getChildren().',
680+
'Return type mixed of method Bug9615\ExpectComplaintsHere::getChildren() is not covariant with tentative return type RecursiveFilterIterator|null of method RecursiveFilterIterator::getChildren().',
681681
20,
682682
$tipText,
683683
],
@@ -839,6 +839,24 @@ public function testSimpleXmlElementChildClass(): void
839839
$this->analyse([__DIR__ . '/data/simple-xml-element-child.php'], []);
840840
}
841841

842+
#[RequiresPhp('>= 8.1')]
843+
public function testBug7317(): void
844+
{
845+
$this->phpVersionId = PHP_VERSION_ID;
846+
$this->analyse([__DIR__ . '/data/bug-7317.php'], [
847+
[
848+
'Return type bool of method Bug7317\MySimpleXMLElement::current() is not covariant with tentative return type static(SimpleXMLElement)|null of method SimpleXMLElement::current().',
849+
8,
850+
'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.',
851+
],
852+
[
853+
'Return type int of method Bug7317\MySimpleXMLElement::valid() is not covariant with tentative return type bool of method Iterator<string,static(SimpleXMLElement)>::valid().',
854+
12,
855+
'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.',
856+
],
857+
]);
858+
}
859+
842860
public function testFixOverride(): void
843861
{
844862
$this->phpVersionId = PHP_VERSION_ID;
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php // lint >= 8.1
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug7317;
6+
7+
class MySimpleXMLElement extends \SimpleXMLElement {
8+
public function current(): bool {
9+
return false;
10+
}
11+
12+
public function valid(): int {
13+
return 1;
14+
}
15+
}

0 commit comments

Comments
 (0)