Add Type::searchArray()

[herndlm]

Split out of the extension, but with a couple of small differences

• instead of returning null (happened on PHP < 8 with a warning, see https://3v4l.org/lNrqs) it returns *ERROR*. not sure if that's OK?
• support for HasOffsetValueType which wasn't understood before, see https://phpstan.org/r/e20727f4-7809-42fc-825e-85df0f5ef635
• support for optional array keys, this was incorrect before, see https://phpstan.org/r/96b559ec-55a2-4feb-b3a9-a1e4dff2a84b

not sure if $strictType should be moved into the method signature as well or not 🤔

[ondrejmirtes]

The null behaviour should be kept - if we're aware of these regressions we should fix them :) Actually it should be null before PHP 8 and NeverType on PHP 8+, right?

[herndlm]

The null behaviour should be kept - if we're aware of these regressions we should fix them :) Actually it should be null before PHP 8 and NeverType on PHP 8+, right?

yeah, adding the php-version check makes sense. regarding NeverType - I'm not sure if it should be NeverType or ErrorType, this would be relevant for more array functions most likely :/ e.g. array_flip(): https://3v4l.org/XnrUq which also shows the null behaviour again hmm
UDPATE: ah and never/error obviously is relevant when passing something like false|arary to array_flip() for example. feels like this needs some discussion / cleaning up again

[ondrejmirtes]

The ErrorType doesn't do anything useful, it's basically a MixedType.

There's additional downside to ErrorType with arrays - because of some logic in NodeScopeResolver, if you add a new offset to ErrorType, a new array shape (ConstantArrayType) is created, leading to most likely wrong analysis...

NeverType at least signals that no code after this point is executed because it crashes/throws an exception.

[herndlm]

I wonder if it makes sense to keep the error types in the traits/types and let only the extensions return null/never depending on php version in case is array()->no(). It still feels inconsistent, but maybe I just need to play around with it..

[herndlm]

well, I think this is looking better, but it feels more correct to let the NonArrayTypeTrait return NEVER too, but that would cancel it out in a union I guess. on the other hand I'm not sure if ERROR is the right type there either, see e.g LegacyNodeScopeResolverTest diff. also: I can't check for the PHP version in types, can I? 🤔

[phpstan-bot]

This pull request has been marked as ready for review.

[herndlm]

Another idea: the nonarray trait could return a nulltype, which is what the array functions would do without enforcing the array arg type. The extension checks php version and isArray() and either returns nulltype/nevertype directly, does something different with maybe or passes the type through for a yes(). That could improve the type over all php versions. Not sure yet what should happen with e.g. array|false on php 8+ though. Maybe errortype

[ondrejmirtes]

I wouldn't pass PHP array functions weirdnesses into the Type system layer. Most likely it would lead to weird results in union/intersection types, and also it'd be misinformative for other uses...

[herndlm]

What do you think of the current state then or any other ideas?

I'm not a 100% happy with it, unsure about unions mostly. But in order to keep the type for pre php 8 I'd need to modify the maybearraytypetrait somehow I guess. Which would mess with the type on php 8+. Except we change that and handle it maybe differently in the extension

[herndlm]

Nevermind, I think I know how to improve it in the extension with a bit of extra code for old php. That shouldn't mess with the type system either. I'll try later :)

[herndlm]

I give up, my super smart plan was to handle the isArray()->maybe() for pre PHP 8 and return the iterable key type union null, but since iterable key type is already *ERROR* the outcome is the same 🙃 I think *ERROR* is just fine than. running out of ideas :)
it correctly deals with *NEVER* vs null for non-arrays, maybe that's as good as it can get. I'd do the same for the other array functions as well afterwards

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

Really nice, thank you 😊