Support narrowing a constant array to a list with count#3876
Support narrowing a constant array to a list with count#3876ondrejmirtes merged 1 commit intophpstan:2.1.xfrom
Conversation
c737a0d to
2abd69e
Compare
ondrejmirtes
left a comment
There was a problem hiding this comment.
I believe this has nothing to do with lists. If you have array{a?: int, b?: string} and check count >= 2, you can make them required too. Please add more tests.
If I have array{a: int, b: string, c?: int} and check count > 2 or count >= 3, c should become required too.
this sounds like sealed vs. unsealed array shape business |
|
Yes, in the future we will have to modify the condition to only apply to sealed shapes. |
|
You're right, we shouldn't change this now before sealed shapes are implemented. |
|
Thank you. |
|
We have another regression: https://phpstan.org/r/c97add7b-a241-45c9-88f1-b8e7c76daa09 Please, Martin or Markus, look into it. Thank you! |
|
weird, this stuff is supposed to deal only with lists or constant arrays, I'll try to take a look |
|
yeah, looks not related to recent changes at least. if I did not mess up massively right now, then this is I'll see if I can find out why though.. |
|
wait, everything is OK I think 😅 it is comparing a constant int with an array and that will always return false and make the assertion throw an exception. riiight? :) https://3v4l.org/k2LU5 I was already deep-debugging |
|
Oh now I'm confused 😂 I need to look into what was I thinking. |
|
Alright, this is the right code sample: https://phpstan.org/r/032a2b0f-40bc-48e1-a5ee-a71859e78b04 This really changed before and after this PR. Sorry about the previous confusion. |
Implementing what was found/suggested in #3709 (comment) and improves very special cases with int ranges.
PR title is maybe misleading. it's making keys non-optional if it can for list-like cases.