-
Notifications
You must be signed in to change notification settings - Fork 488
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
More precise array_keys
return type
#3590
base: 2.1.x
Are you sure you want to change the base?
Conversation
if (!$keysArray->isConstantArray()->no()) { | ||
$newArrayType = new ArrayType( | ||
$keysArray->getIterableKeyType()->generalize(GeneralizePrecision::lessSpecific()), | ||
$keysArray->getIterableValueType()->generalize(GeneralizePrecision::lessSpecific()), | ||
); | ||
} |
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.
we could make this more sophisticated and support more filtering on constant array, but I think this is good enough for now.
until a few days ago I didn't even know array_keys
can filter by values - it feels pretty rare
This pull request has been marked as ready for review. |
Just wondering if any of this could also be done inside See also #2117 where I played around with adding filter support 😊 |
Nice, sorry, noticed too late and should have mentioned it earlier. Later on, e.g. in #3514 I used |
I used $strict as bool, because other methods in Type also use it. Do you see a use-case where Trinary would come in handy? |
Not necessarily. If others use that too, then consistency makes sense IMO. |
21fc500
to
c16ce86
Compare
src/Type/Type.php
Outdated
@@ -134,7 +134,7 @@ public function setExistingOffsetValueType(Type $offsetType, Type $valueType): T | |||
|
|||
public function unsetOffset(Type $offsetType): Type; | |||
|
|||
public function getKeysArray(): Type; | |||
public function getKeysArray(?Type $filterValueType = null, bool $strict = false): Type; |
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.
- This a BC break
- I don't like optional parameters at all.
The solution is to add another method that you'll use in the places where you're passing in the parameters now.
656e6e2
to
05c0c60
Compare
@@ -0,0 +1,51 @@ | |||
<?php | |||
|
|||
namespace Bug11928; |
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.
closes phpstan/phpstan#11928