Skip to content

Fix XMLReader::open() return type#456

Merged
ondrejmirtes merged 2 commits intophpstan:masterfrom
jeroennoten:patch-1
Mar 17, 2021
Merged

Fix XMLReader::open() return type#456
ondrejmirtes merged 2 commits intophpstan:masterfrom
jeroennoten:patch-1

Conversation

@jeroennoten
Copy link
Copy Markdown
Contributor

@jeroennoten jeroennoten commented Feb 23, 2021

XMLReader::open() returns an instance of XMLReader when called statically (and according to its ReflectionMethod, XMLReader::open() is actually a static function).

Currently, this method is now unable to use in boths ways without PHPStan reporting errors with strict rules enabled:

Actually, an even better fix would be to make the return type dependent on whether the method is called statically or not, is that possible? It returns XMLReader|false when called statically or bool when called non-statically.

@ondrejmirtes
Copy link
Copy Markdown
Member

Actually, an even better fix would be to make the return type dependent on whether the method is called statically or not, is that possible?

This is possible with dynamic return type extensions: https://phpstan.org/developing-extensions/dynamic-return-type-extensions so feel free to contribute one :) There are many examples to learn from in PHPStan's codebase itself: https://github.com/phpstan/phpstan-src/tree/master/src/Type/Php

@jeroennoten jeroennoten force-pushed the patch-1 branch 4 times, most recently from c928740 to 21e042f Compare March 17, 2021 12:13
@jeroennoten
Copy link
Copy Markdown
Contributor Author

jeroennoten commented Mar 17, 2021

@ondrejmirtes, I found some time to add a dynamic return type extension for XMLReader::open(). Could you please check if this the right way to do it?

@ondrejmirtes ondrejmirtes merged commit 2b30830 into phpstan:master Mar 17, 2021
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you

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.

2 participants