New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Properties in SimpleXMLElement may be null #76
Conversation
…mplement behaviour for calls on BenevolentUnionTypes by overriding UnionType::unionResults
I couldn't run phpcbf on my windows PHP 7.4 because of deprecations and fatal errors due to an old version of squizlabs\php_codesniffer. I will check on the CI here to fix potential issues |
*/ | ||
protected function unionResults(callable $getResult): TrinaryLogic | ||
{ | ||
return TrinaryLogic::maxMin(...array_map($getResult, $this->getTypes())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not certain maxMin was the best method to use here, but I wasn't sure what to do with the Maybe case...
@ondrejmirtes I think this is ready to be reviewed. Please tell me if I made something wrong or if I should implement more tests or anything! Thanks! |
0786c63
to
9395d76
Compare
dc2e229
to
1d0d3ad
Compare
f54fd5f
to
eca550a
Compare
d45166a
to
3526237
Compare
efd31ce
to
0471f87
Compare
Merged as: fcfcbd5 Sorry to keep you waiting! |
Wow! I lost hope on this one a looong time ago :D Thanks Ondrej! |
As discussed in phpstan/phpstan/issues/2784 this returns a BenevolentUnionType(SimpleXMLElement|null) on properties for SimpleXMLElement.
It then allows operations on BenevolentUnionType by overriding unionResults (the method implementing the logic for whether an operation is permitted). While UnionType allow an operation if it's allowed on every Type in the Union, BenevolenUnionType allow the operation if it's allowed on at least one Type in the Union.
I added a test on impossible-instanceof. Without the modification in SimpleXML, it returned
and with the modification, it doesn't return anything.
As you said in phpstan/phpstan/issues/2784:
I believe some functions in SimpleXMLElement should return BenevolentUnionType as well, such as SimpleXMLElement::attributes, who currently return SimpleXMLElement|null. Unfortunately, I tried modifying the type to (SimpleXMLElement|null) in FunctionMap but it doesn't seem to be resolved into a BenevolentUnionType. This could be for a future PR.