-
Notifications
You must be signed in to change notification settings - Fork 457
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
array_fill_keys
expects array of array-key
#3110
Conversation
@@ -261,7 +261,7 @@ | |||
'array_change_key_case' => ['array', 'input'=>'array', 'case='=>'int'], | |||
'array_chunk' => ['list<array>', 'input'=>'array', 'size'=>'positive-int', 'preserve_keys='=>'bool'], | |||
'array_column' => ['array', 'array'=>'array', 'column_key'=>'mixed', 'index_key='=>'mixed'], | |||
'array_combine' => ['array|false', 'keys'=>'array', 'values'=>'array'], | |||
'array_combine' => ['array|false', 'keys'=>'array<array-key>', 'values'=>'array'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while here I used the same fix here - I think array-key
better conveys the idea of the type beeing required
this change is here just for consistency with array_fill_keys
It's not okay to change the functionMap for this requirement. This needs a custom rule. Some types outside of int|string can be cast to array key. |
As for:
I believe PHPStan already does all of that but if it does not, open a feature request. |
https://phpstan.org/r/a2c1cff4-0464-4021-bb32-04680554d9bf you are right, it works already |
would it make sense to create a new "virtual type", like we have with phpstan-src/src/PhpDoc/TypeNodeResolver.php Lines 431 to 432 in 04b8403
|
I'd say creating a new custom rule is easier than modifying the type system. |
refs phpstan/phpstan#11111
the also issue mentions
which makes sense, but a separate problem for a separate PR IMO