From 320381f8aa5240b72fc432670d0ea0ec5cffb0ae Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 31 Mar 2026 04:01:31 +0000 Subject: [PATCH] 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 --- phpstan-baseline.neon | 6 ++ src/Rules/Methods/OverridingMethodRule.php | 56 ++++++++++++++++--- .../Methods/OverridingMethodRuleTest.php | 20 ++++++- tests/PHPStan/Rules/Methods/data/bug-7317.php | 15 +++++ 4 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 tests/PHPStan/Rules/Methods/data/bug-7317.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 6906e22da9d..390e6a23826 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1940,3 +1940,9 @@ parameters: identifier: phpstanApi.varTagAssumption count: 1 path: tests/PHPStan/Type/IterableTypeTest.php + + - + rawMessage: 'Unused PHPStan\Reflection\MethodPrototypeReflection::getName' + identifier: shipmonk.deadMethod + count: 1 + path: src/Reflection/MethodPrototypeReflection.php diff --git a/src/Rules/Methods/OverridingMethodRule.php b/src/Rules/Methods/OverridingMethodRule.php index 2058b64931f..ca2709be78d 100644 --- a/src/Rules/Methods/OverridingMethodRule.php +++ b/src/Rules/Methods/OverridingMethodRule.php @@ -11,12 +11,15 @@ use PHPStan\DependencyInjection\ValidatesStubFiles; use PHPStan\Node\InClassMethodNode; use PHPStan\Php\PhpVersion; +use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ExtendedFunctionVariant; use PHPStan\Reflection\MethodPrototypeReflection; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\MixedType; +use PHPStan\Type\Type; +use PHPStan\Type\TypehintHelper; use PHPStan\Type\VerbosityLevel; use function array_merge; use function count; @@ -203,22 +206,45 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array $realPrototype = $method->getPrototype(); + $effectiveTentativeReturnType = null; + $tentativeDeclaringClass = null; + $hasTentativeReturnType = false; + + if ($realPrototype instanceof MethodPrototypeReflection && $realPrototype->getTentativeReturnType() !== null) { + $effectiveTentativeReturnType = $realPrototype->getTentativeReturnType(); + $tentativeDeclaringClass = $realPrototype->getDeclaringClass(); + $hasTentativeReturnType = true; + } + + // The parent class method may have a more specific tentative return type + // than the deepest prototype (e.g., SimpleXMLElement::current() has tentative + // type SimpleXMLElement, while Iterator::current() has tentative type mixed) + $parentTentativeReturnType = $this->getParentMethodTentativeReturnType($prototypeDeclaringClass, $prototype->getName()); + if ($parentTentativeReturnType !== null) { + $hasTentativeReturnType = true; + if ($effectiveTentativeReturnType === null || !$parentTentativeReturnType->isSuperTypeOf($effectiveTentativeReturnType)->yes()) { + $effectiveTentativeReturnType = $parentTentativeReturnType; + $tentativeDeclaringClass = $prototypeDeclaringClass; + } + } + if ( - $realPrototype instanceof MethodPrototypeReflection + $hasTentativeReturnType + && $effectiveTentativeReturnType !== null + && $tentativeDeclaringClass !== null && $this->phpVersion->hasTentativeReturnTypes() - && $realPrototype->getTentativeReturnType() !== null && !$this->hasReturnTypeWillChangeAttribute($node->getOriginalNode()) && count($prototypeDeclaringClass->getNativeReflection()->getMethod($prototype->getName())->getAttributes('ReturnTypeWillChange')) === 0 ) { - if (!$this->methodParameterComparisonHelper->isReturnTypeCompatible($realPrototype->getTentativeReturnType(), $method->getNativeReturnType(), true)) { + if (!$this->methodParameterComparisonHelper->isReturnTypeCompatible($effectiveTentativeReturnType, $method->getNativeReturnType(), true)) { $messages[] = RuleErrorBuilder::message(sprintf( 'Return type %s of method %s::%s() is not covariant with tentative return type %s of method %s::%s().', $methodReturnType->describe(VerbosityLevel::typeOnly()), $method->getDeclaringClass()->getDisplayName(), $method->getName(), - $realPrototype->getTentativeReturnType()->describe(VerbosityLevel::typeOnly()), - $realPrototype->getDeclaringClass()->getDisplayName(true), - $realPrototype->getName(), + $effectiveTentativeReturnType->describe(VerbosityLevel::typeOnly()), + $tentativeDeclaringClass->getDisplayName(true), + $prototype->getName(), )) ->tip('Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.') ->nonIgnorable() @@ -236,8 +262,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array $prototypeReturnType = $prototypeVariant->getNativeReturnType(); $reportReturnType = true; if ($this->phpVersion->hasTentativeReturnTypes()) { - $reportReturnType = !$realPrototype instanceof MethodPrototypeReflection - || $realPrototype->getTentativeReturnType() === null + $reportReturnType = !$hasTentativeReturnType || (is_bool($prototype->isBuiltin()) ? !$prototype->isBuiltin() : $prototype->isBuiltin()->no()); } else { if ($realPrototype instanceof MethodPrototypeReflection && $realPrototype->isInternal()) { @@ -371,4 +396,19 @@ private function hasOverrideAttribute(Node\Stmt\ClassMethod $method): bool return false; } + private function getParentMethodTentativeReturnType(ClassReflection $prototypeDeclaringClass, string $methodName): ?Type + { + $prototypeNativeReflection = $prototypeDeclaringClass->getNativeReflection(); + if (!$prototypeNativeReflection->hasMethod($methodName)) { + return null; + } + + $prototypeReflMethod = $prototypeNativeReflection->getMethod($methodName); + if (!$prototypeReflMethod->hasTentativeReturnType()) { + return null; + } + + return TypehintHelper::decideTypeFromReflection($prototypeReflMethod->getTentativeReturnType(), selfClass: $prototypeDeclaringClass); + } + } diff --git a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php index 55979eeb77d..38118aad965 100644 --- a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php +++ b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php @@ -677,7 +677,7 @@ public function testBug9615(): void $tipText, ], [ - 'Return type mixed of method Bug9615\ExpectComplaintsHere::getChildren() is not covariant with tentative return type RecursiveIterator|null of method RecursiveIterator::getChildren().', + 'Return type mixed of method Bug9615\ExpectComplaintsHere::getChildren() is not covariant with tentative return type RecursiveFilterIterator|null of method RecursiveFilterIterator::getChildren().', 20, $tipText, ], @@ -839,6 +839,24 @@ public function testSimpleXmlElementChildClass(): void $this->analyse([__DIR__ . '/data/simple-xml-element-child.php'], []); } + #[RequiresPhp('>= 8.1')] + public function testBug7317(): void + { + $this->phpVersionId = PHP_VERSION_ID; + $this->analyse([__DIR__ . '/data/bug-7317.php'], [ + [ + 'Return type bool of method Bug7317\MySimpleXMLElement::current() is not covariant with tentative return type static(SimpleXMLElement)|null of method SimpleXMLElement::current().', + 8, + 'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.', + ], + [ + 'Return type int of method Bug7317\MySimpleXMLElement::valid() is not covariant with tentative return type bool of method Iterator::valid().', + 12, + 'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.', + ], + ]); + } + public function testFixOverride(): void { $this->phpVersionId = PHP_VERSION_ID; diff --git a/tests/PHPStan/Rules/Methods/data/bug-7317.php b/tests/PHPStan/Rules/Methods/data/bug-7317.php new file mode 100644 index 00000000000..144f0fb5ddd --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-7317.php @@ -0,0 +1,15 @@ += 8.1 + +declare(strict_types = 1); + +namespace Bug7317; + +class MySimpleXMLElement extends \SimpleXMLElement { + public function current(): bool { + return false; + } + + public function valid(): int { + return 1; + } +}