Skip to content

Fix phpstan/phpstan#7317: Return type of SimpleXMLElement::current() is not tentative#5363

Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-r2mgjo6
Open

Fix phpstan/phpstan#7317: Return type of SimpleXMLElement::current() is not tentative#5363
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-r2mgjo6

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

When a class extends SimpleXMLElement and overrides current() with an incompatible return type, PHPStan failed to report a tentative return type error. It correctly reported the error for valid() but not for current(), even though both methods have tentative return types in PHP 8.1+.

Changes

  • Modified src/Rules/Methods/OverridingMethodRule.php to also check the direct parent class method's tentative return type, not just the deepest prototype's
  • Added getParentMethodTentativeReturnType() helper method to resolve the parent method's tentative return type via BetterReflection
  • Updated $reportReturnType logic to use the computed $hasTentativeReturnType flag
  • Added Type and TypehintHelper imports
  • Added ClassReflection import
  • Updated Bug9615 test expectation to reflect the more accurate parent class reference (RecursiveFilterIterator::getChildren() instead of RecursiveIterator::getChildren())
  • Added baseline entry for MethodPrototypeReflection::getName() dead method (no longer called from the rule but may be used externally)

Root cause

The OverridingMethodRule used $method->getPrototype() to get the tentative return type for comparison. PHP's getPrototype() returns the deepest ancestor (interface method), so for MySimpleXMLElement::current(), it returned Iterator::current() with tentative type mixed. Since bool is covariant with mixed, no error was reported.

However, SimpleXMLElement::current() has its own more specific tentative return type SimpleXMLElement. The fix computes the tentative return type from both the deepest prototype AND the direct parent class method, using the more specific one for the check.

Test

Added tests/PHPStan/Rules/Methods/data/bug-7317.php - a regression test that extends SimpleXMLElement and overrides current() with return type bool and valid() with return type int. Both should produce tentative return type errors on PHP 8.1+.

Fixes phpstan/phpstan#7317

…n 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant