Skip to content

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Oct 2, 2025

Fatal error: Uncaught ValueError: array_rand(): Argument #1 ($array) must not be empty in /in/sEB2j:3
Stack trace:
#0 /in/sEB2j(3): array_rand(Array)
#1 {main}
  thrown in /in/sEB2j on line 3

see https://3v4l.org/sEB2j


we did not error before this PR: https://phpstan.org/r/2f650341-3dc9-4849-9cac-2bd267a7fb41

@staabm
Copy link
Contributor Author

staabm commented Oct 2, 2025

it even requires a positve $num arg as of php8, see https://3v4l.org/b53OZ

'iterator_count' => ['0|positive-int', 'iterator'=>'iterable'],
'iterator_to_array' => ['array', 'iterator'=>'iterable', 'use_keys='=>'bool'],
'str_split' => ['list<string>', 'str'=>'string', 'split_length='=>'positive-int'],
'Random\Randomizer::pickArrayKeys' => ['non-empty-array<int|string>', 'array'=>'non-empty-array', 'num'=>'positive-int'],
Copy link
Contributor Author

@staabm staabm Oct 2, 2025

Choose a reason for hiding this comment

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

not sure whether/how this signature should be behind bleeding edge but beeing php 8.2+ at the same time.

as the api is pretty recent and throws runtime errors when used with empty array or non-positive-int its acceptable to enforce this more strict signature even in non-bleeding edge?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, in this case it's okay 👍

@staabm staabm marked this pull request as ready for review October 2, 2025 08:13
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@ondrejmirtes ondrejmirtes merged commit 2548dbb into phpstan:2.1.x Oct 6, 2025
538 of 547 checks passed
@ondrejmirtes
Copy link
Member

Thank you!

@staabm staabm deleted the rand-empty branch October 6, 2025 10:14
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.

3 participants