Solve in_array() once and for all

[ondrejmirtes]

Bug report

/cc @herndlm this is something up your alley, could you please spare a few days to occupy your mind with this to see how it could be solved? :)

in_array is a very problematic function - the type-specifying extension (https://github.com/phpstan/phpstan-src/blob/master/src/Type/Php/InArrayFunctionTypeSpecifyingExtension.php) currently serves to narrow down the type of the first argument.

The problems come when the error "Call to function in_array() will always evaluate to true/false" is reported for cases where it's not correct.

Here are some open issues about it: https://github.com/phpstan/phpstan/issues?q=is%3Aissue+is%3Aopen+in_array

Please note that this should be solved entirely with conditions in the type-specifying extension and the code related to in_array should be removed from https://github.com/phpstan/phpstan-src/blob/bedcc2db8616d195f4cf698d3731cc69d634c2b7/src/Rules/Comparison/ImpossibleCheckTypeHelper.php#L81-L130 entirely. This is so that issues like this phpstan/phpstan-webmozart-assert#142 when in_array is handled indirectly are also solved.

Thank you.

Code snippet that reproduces the problem

https://phpstan.org/r/179a9da2-e9e7-4d4b-b4af-b0e01c35a9db and many others

[ondrejmirtes]

I did something: phpstan/phpstan-src@4321374

But it's still a mess worthy of some deep research and cleanup.

[phpstan-bot]

@ondrejmirtes After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
-15: Call to function in_array() with arguments 0, array<int, 0|1> and true will always evaluate to true.
+No errors

[herndlm]

There's no way to describe that e.g. array<int> has an offset value type of e.g. 0, right? meaning that we have an int array but one of them is 0 for sure. This is normalized away everywhere, isn't it? This seems to be one of the many problematic things with in_array..
UPDATE: I guess this can't be described cleanly, in that case I guess we have to ignore it

[ondrejmirtes]

@herndlm HasOffsetType could be enriched to contain the value type as well. After that we could stop relying on the fact that the ArrayDimFetch expr like $data[$key] is sometimes specified in TypeSpecifier to carry this value information.

[herndlm]

I had a similar idea that I was playing around with yesterday. If that works, it might solve many of the in_array problems. I'll keep looking a bit in that direction then, thx

[herndlm]

I'm getting closer. I found a thing that confuses me though: given the type non-empty-array<int, 'a'>, shouldn't ->hasOffsetValueType(new IntegerType()) answer that with yes instead of maybe because of the intersection of both? if so, were would this be cleanly implemented? :/
UPDATE: bad idea.. xD

[herndlm]

I seem to be down to one weird extra conditional needed only. and it is a bit related to the previous comment I guess. maybe I'll open a PR later to get some feedback from you

[ondrejmirtes]

OMG, I just realized what we need to solve in_array once and for all and without any hacks.

We have HasOffsetType (we know the key) and HasOffsetValueType (we know the key and value combination).

For in_array() and maybe other uses we need HasConstantValueType accessory type that tells us about value existence in an unknown key.

Paired with Type::hasConstantValueType() this should stop these headaches 😊

[herndlm]

Yeah exactly, that would fix most of the problems, if not all, here. That's why I was so looking forward to HasOffsetValueType. Until I realised it needs a constant key :) or a key at all :D

[herndlm]

I was thinking about this one again, regarding your idea about a HasConstantValueType - was there something specific why it has to be "constant"? wouldn't it be possible still to work with any value?

[ondrejmirtes]

It breaks apart if you do 1|2 or int - you can't tell anything for sure with these.

[herndlm]

ok, an initial implementation looks promising, I'll try to bring it in a reviewable state soon. maybe before that - looks like I have questions already now :)

• I see already that e.g. appending ConstantType values to generic arrays will add that new type as intersection infinite times. like if $foo is array<int> and I do $foo[] = 17; $foo[] = 19; I basically end up with non-empty-array<int>&hasConstantValue(17)&hasConstantValue(19), which is exactly what helps with e.g. in_array(). But I was wondering if this might add up number-wise and lead to performance problems, there are no limitations right now for e.g. HasOffsetType, are there?
• if I encounter 2 arrays with that new type like non-empty-array<int>&hasConstantValue(17) and non-empty-array<int>&hasConstantValue(19) and want to union them, I have to drop that type, right?

[ondrejmirtes]

appending ConstantType values to generic arrays will add that new type as intersection infinite times

Yes. Same problem we have with HasOffsetValueType. Remedied here: https://github.com/phpstan/phpstan-src/blob/506a4d9df43ca815b75e95a5807b44d983a1fdab/src/Type/TypeCombinator.php#L898-L902

if I encounter 2 arrays with that new type

That's already taken care of somehow by this piece of code: https://github.com/phpstan/phpstan-src/blob/506a4d9df43ca815b75e95a5807b44d983a1fdab/src/Type/TypeCombinator.php#L532-L560