Add Type::chunkArray()
#3408
Add Type::chunkArray()
#3408
Conversation
herndlm
force-pushed
the
chunk-array
branch
5 times, most recently
from
76faece
to
82dcfac
Compare
|
This one should be good. Waiting for it to get merged to open the next one for sliceArray btw 😊 |
| @@ -175,6 +175,11 @@ public function unsetOffset(Type $offsetType): Type | |||
| return $this; | |||
| } | |||
|
|
|||
| public function chunkArray(Type $lengthType, TrinaryLogic $preserveKeys): Type | |||
| { | |||
| return new NonEmptyArrayType(); | |||
There was a problem hiding this comment.
I think this once and the one in HasOffsetValueType is wrong. You should test what happens when you pass string&hasOffset to array_chunk.
A solution might be to return $this, or maybe handle this in IntersectionType::chunkArray (which knows what this is intersected with).
There was a problem hiding this comment.
I see what you mean. It's not wrong IMO, but it could be better. I pushed tests to show the current behaviour.
Within HasOffset*Type we cannot return $this, that would be wrong because the offset ends up being nested after array_chunk. NonEmptyArray() is correct IMO but we could do better. See also https://3v4l.org/dCOoR which is the same as the type in the new tests (non-empty-list<non-empty-list<mixed>> or non-empty-list<non-empty-array>.
Using my new tests to check the current behaviour also shows that this PR would make it less precise, so that's definitely not good :/ https://phpstan.org/r/c4dfc504-4d6f-41fc-bad2-7bb9c90272bc
And yes, you're correct, looks like making it better means adapting IntersectionType most likely :) I'll try to push the fix soon.
There was a problem hiding this comment.
or is there a way to return this nested structure in the accessory type? so far I'm then getting a *NEVER* as result if I do that :/ guess I need to debug the types that it operates on a bit more..
There was a problem hiding this comment.
never mind, I think this is easier than I thought 😊
There was a problem hiding this comment.
sorry for the noise :P
I managed to return whatever type I want, but realized that the current "live" behavior might be wrong already. e.g. how would we type the return type of https://3v4l.org/TfHjW assuming it's not a constant array but there are those 3 known offsets and there could be more we don't know about since its a generic array.
At first I thought it would be non-empty-list<hasOffsetValue('a','foo')&hasOffsetValue('b','bar')&hasOffsetValue('c','baz')&non-empty-array> but that's not correct since we have 3 arrays inside and the generic representation is just non-empty-list<non-empty-array>, right?
Then I thought we could use the offset and value info to make something like
non-empty-list<non-empty-array<'a'|'b'|'c','foo'|'bar'|'baz'>> but that's also wrong since, again, there could be other keys and values.
I seem to be coming back to the current behavior / result or am I missing something? As long as it's not a constant array we don't know in which "chunk" that offset (+ value) might end up in, right?
herndlm
force-pushed
the
chunk-array
branch
from
82dcfac
to
b31eb35
Compare
|
Alright, let's try it like this :) Thank you. |
just cleaning up to simplify things and hopefully be able to cleanup some type checks in ConstantArrayType later..