Skip to content

Commit 5be9fff

Browse files
github-actions[bot]phpstan-bot
authored andcommitted
Fix tentative return type check for overridden methods with intermediate prototypes
- OverridingMethodRule now also checks the parent class method's own tentative return type, not just the deepest prototype's - This fixes the case where SimpleXMLElement::current() has tentative return type ?static but the check only saw Iterator::current() with tentative type mixed - Updated $reportReturnType logic to suppress regular return type errors when the parent class method has a tentative return type - Added regression test for phpstan/phpstan#7317 Closes phpstan/phpstan#7317
1 parent 2681e50 commit 5be9fff

File tree

3 files changed

+73
-2
lines changed

3 files changed

+73
-2
lines changed

src/Rules/Methods/OverridingMethodRule.php

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use PHPStan\Rules\Rule;
1717
use PHPStan\Rules\RuleErrorBuilder;
1818
use PHPStan\Type\MixedType;
19+
use PHPStan\Type\TypehintHelper;
1920
use PHPStan\Type\VerbosityLevel;
2021
use function array_merge;
2122
use function count;
@@ -201,6 +202,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array
201202

202203
$realPrototype = $method->getPrototype();
203204

205+
$tentativeReturnTypeErrorReported = false;
204206
if (
205207
$realPrototype instanceof MethodPrototypeReflection
206208
&& $this->phpVersion->hasTentativeReturnTypes()
@@ -209,6 +211,7 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array
209211
&& count($prototypeDeclaringClass->getNativeReflection()->getMethod($prototype->getName())->getAttributes('ReturnTypeWillChange')) === 0
210212
) {
211213
if (!$this->methodParameterComparisonHelper->isReturnTypeCompatible($realPrototype->getTentativeReturnType(), $method->getNativeReturnType(), true)) {
214+
$tentativeReturnTypeErrorReported = true;
212215
$messages[] = RuleErrorBuilder::message(sprintf(
213216
'Return type %s of method %s::%s() is not covariant with tentative return type %s of method %s::%s().',
214217
$methodReturnType->describe(VerbosityLevel::typeOnly()),
@@ -225,6 +228,39 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array
225228
}
226229
}
227230

231+
if (
232+
!$tentativeReturnTypeErrorReported
233+
&& $this->phpVersion->hasTentativeReturnTypes()
234+
&& !$this->hasReturnTypeWillChangeAttribute($node->getOriginalNode())
235+
) {
236+
$parentNativeMethod = $prototypeDeclaringClass->getNativeReflection()->getMethod($prototype->getName());
237+
if (
238+
$parentNativeMethod->hasTentativeReturnType()
239+
&& count($parentNativeMethod->getAttributes('ReturnTypeWillChange')) === 0
240+
) {
241+
$parentTentativeReturnType = TypehintHelper::decideTypeFromReflection(
242+
$parentNativeMethod->getTentativeReturnType(),
243+
selfClass: $prototypeDeclaringClass,
244+
);
245+
if (!$this->methodParameterComparisonHelper->isReturnTypeCompatible($parentTentativeReturnType, $method->getNativeReturnType(), true)) {
246+
$tentativeReturnTypeErrorReported = true;
247+
$messages[] = RuleErrorBuilder::message(sprintf(
248+
'Return type %s of method %s::%s() is not covariant with tentative return type %s of method %s::%s().',
249+
$methodReturnType->describe(VerbosityLevel::typeOnly()),
250+
$method->getDeclaringClass()->getDisplayName(),
251+
$method->getName(),
252+
$parentTentativeReturnType->describe(VerbosityLevel::typeOnly()),
253+
$prototypeDeclaringClass->getDisplayName(true),
254+
$prototype->getName(),
255+
))
256+
->tip('Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.')
257+
->nonIgnorable()
258+
->identifier('method.tentativeReturnType')
259+
->build();
260+
}
261+
}
262+
}
263+
228264
$messages = array_merge($messages, $this->methodParameterComparisonHelper->compare($prototype, $prototypeDeclaringClass, $method, false));
229265

230266
if (!$prototypeVariant instanceof ExtendedFunctionVariant) {
@@ -234,8 +270,9 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array
234270
$prototypeReturnType = $prototypeVariant->getNativeReturnType();
235271
$reportReturnType = true;
236272
if ($this->phpVersion->hasTentativeReturnTypes()) {
237-
$reportReturnType = !$realPrototype instanceof MethodPrototypeReflection
238-
|| $realPrototype->getTentativeReturnType() === null
273+
$hasTentativeReturnType = ($realPrototype instanceof MethodPrototypeReflection && $realPrototype->getTentativeReturnType() !== null)
274+
|| $prototypeDeclaringClass->getNativeReflection()->getMethod($prototype->getName())->hasTentativeReturnType();
275+
$reportReturnType = !$hasTentativeReturnType
239276
|| (is_bool($prototype->isBuiltin()) ? !$prototype->isBuiltin() : $prototype->isBuiltin()->no());
240277
} else {
241278
if ($realPrototype instanceof MethodPrototypeReflection && $realPrototype->isInternal()) {

tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,25 @@ public function testBug9524(): void
832832
$this->analyse([__DIR__ . '/data/bug-9524.php'], []);
833833
}
834834

835+
#[RequiresPhp('>= 8.1')]
836+
public function testBug7317(): void
837+
{
838+
$this->phpVersionId = PHP_VERSION_ID;
839+
$tipText = 'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.';
840+
$this->analyse([__DIR__ . '/data/bug-7317.php'], [
841+
[
842+
'Return type bool of method Bug7317\MySimpleXMLElement::current() is not covariant with tentative return type static(SimpleXMLElement)|null of method SimpleXMLElement::current().',
843+
8,
844+
$tipText,
845+
],
846+
[
847+
'Return type int of method Bug7317\MySimpleXMLElement::valid() is not covariant with tentative return type bool of method Iterator<string,static(SimpleXMLElement)>::valid().',
848+
12,
849+
$tipText,
850+
],
851+
]);
852+
}
853+
835854
#[RequiresPhp('>= 8.0')]
836855
public function testSimpleXmlElementChildClass(): void
837856
{
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)