-
Notifications
You must be signed in to change notification settings - Fork 545
Implement Type->looseCompare(type) #2216
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
Conversation
for now I have implemented the looseCompare methods for all types.
now I need to have a look at the failling tests and afterwards type narrowing
1337c88 to
8db02be
Compare
Type narrowing is a separate concern, I wouldn't do that here.
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 don't see the table of == comparison described anywhere in this PR https://www.php.net/manual/en/types.comparisons.php.
All types need to take into account being compared to any other type. For example this implementation in NullType is too naive:
return $type->isNull()->toBooleanType();
Because all of the following examples are true:
null == [] null == false null == 0 null == ""
Also an idea for a preliminary or a future PR - NanType. See the last comment here: https://www.php.net/manual/en/function.is-nan.php
0cff8e4 to
16fb924
Compare
I have implemented a few basic types and added tests for them in loose-compare.php. please have a look whether this is done how you imagined it.
btw: this PR will get really big if we implement all combinations for all types here.
do you think we could split this, e.g. doing basic and simple object type in one PR, and all more specialized types in separate PRs?
thank you
3fe288d to
9aed5d1
Compare
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 like StringType::looseCompare but otherwise it's still gonna be a lot of work 😅 And we're not even diving into type narrowing now. I think it's obvious why I was avoiding this topic until now 😂
20be7b9 to
018d83e
Compare
closes phpstan/phpstan#8224