Narrow types based on non-strict equality with constant types#2889
Narrow types based on non-strict equality with constant types#2889staabm wants to merge 1 commit into
Conversation
|
You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x. |
| if ($mixed == '1') { | ||
| assertType("mixed", $mixed); | ||
| } | ||
| if ($mixed == '0') { | ||
| assertType("mixed", $mixed); | ||
| } | ||
| if ($mixed == 1) { | ||
| assertType("mixed", $mixed); | ||
| } | ||
| if ($mixed == 0) { | ||
| assertType("mixed", $mixed); | ||
| } |
There was a problem hiding this comment.
these 4 could be more precise
| if ($mixed == '1') { | |
| assertType("mixed", $mixed); | |
| } | |
| if ($mixed == '0') { | |
| assertType("mixed", $mixed); | |
| } | |
| if ($mixed == 1) { | |
| assertType("mixed", $mixed); | |
| } | |
| if ($mixed == 0) { | |
| assertType("mixed", $mixed); | |
| } | |
| if ($mixed == '1') { | |
| assertType("mixed~0|0.0|''|'0'|array{}|false|null", $mixed); | |
| } | |
| if ($mixed == '0') { | |
| assertType("0|0.0|''|'0'|array{}|false|null", $mixed); | |
| } | |
| if ($mixed == 1) { | |
| assertType("mixed~0|0.0|''|'0'|array{}|false|null", $mixed); | |
| } | |
| if ($mixed == 0) { | |
| assertType("0|0.0|''|'0'|array{}|false|null", $mixed); | |
| } |
but I think its a unrelated problem for a followup?
| } | ||
|
|
||
| if ($mixed == []) { | ||
| assertType("mixed", $mixed); |
There was a problem hiding this comment.
same here, could be
| assertType("mixed", $mixed); | |
| assertType("0|0.0|''|'0'|array{}|false|null", $mixed); |
|
Hi, I'd like you to know that I welcome this effort, but I'm unlikely to merge it, because this area is more complex than this PR admits or implements. For example You've already tried a similar problem once (#2216) but didn't finish it. |
|
Also the behaviour is PHP version-dependent: https://3v4l.org/rij8A |
|
thanks for the input. what do you think about turning the assertions into (for all truethy-constant comparisons) ? if ($mixed == 'backend') {
assertType("mixed~0|0.0|''|'0'|array{}|false|null", $mixed);
}I feel we can't really get much better then do some blacklisting in this conditions for now? |
|
I'm for doing as precise types as possible, but it'd be a substantial effort. |
|
and would you be open to do it in small PRs, like e.g. doing "mixed vs. constant-type equal compare narrowing" in a single PR? |
|
We need to design a new method on Type for this. For example if you compare The information from this method can be also used to figure out "is this always true or always false" comparison, similarly to what ImpossibleCheckTypeHelper figures out from the TypeSpecifier output. Once we get this design right and have a proof of concept for a small number of Types, we can merge that and then implement the method in other types in smaller chunks, making the analysis more precise gradually. |
|
my thinking is example: -> take all candidate values out of the comparison table for the involved types
-> based on the comparison table .. the results for these candidates are php7:
php8+:
-> this means for the IF branch php7: php7: stays -> this means for the ELSE branch php7: php7: stays I think this means we need a type-method like |
|
Yeah, something like that, the method also needs to have PhpVersion passed in. |
|
Looking longer at the comparison table, I think it would be helpful to add a new |
|
Ahh just found phpstan/phpstan#10239 (comment) |
closes phpstan/phpstan#10481