Skip to content

Fix array_slice() edge cases #3959

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 28, 2025

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented Apr 21, 2025

Noticed this while finishing #3952

Edge cases fixed here:

  1. Bug fix: we cannot assume that the result of array_slice($array, 0, <positive int>) with a non-empty-array as input will also return a non-empty array because we don't know which of the keys is set. That only can be done for a non-empty-list. The last test change shows this nicely
                        assertType('non-empty-array<int>', $items);
                        $batch = array_splice($items, 0, 2);
                        assertType('array<int>', $items);
-                       assertType('non-empty-array<int>', $batch);
+                       assertType('array<int>', $batch);
  1. Improvement: array_slice with length set to 0 will always return array{}

  2. Bug fix: array_slice with negative offset and negative length of the same value or smaller will always return array{}

assertType('non-empty-list<0|1|2|3|4>', $batch);
assertType('list<0|1|2|3|4>', $batch);
Copy link
Contributor Author

@herndlm herndlm Apr 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unfortunate, but it happens because the $items type is not as good as it can be. This will be improved in #3952 and then it will be non-empty-list again.

So there's unfortunately some kind of coupling here.

@herndlm herndlm force-pushed the array-slice-general-array branch 9 times, most recently from 654b17a to 07e6fe4 Compare April 23, 2025 11:25
@herndlm
Copy link
Contributor Author

herndlm commented Apr 28, 2025

I'm happy with this one and can't do much more I think. Just mentioning it because of what I asked in phpstan/phpstan#12919. I don't have the resources right now to look into that new type methods with all the adaptions needed 😅

@ondrejmirtes ondrejmirtes merged commit 6b6c9c4 into phpstan:2.1.x Apr 28, 2025
417 checks passed
@ondrejmirtes
Copy link
Member

Thank you!

@herndlm herndlm deleted the array-slice-general-array branch April 28, 2025 13:06
@herndlm
Copy link
Contributor Author

herndlm commented Apr 28, 2025

Ok thx, I'll rebase the array_splice stuff right away now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants