Skip to content

Add support for strict $filter_value of array_keys()#2117

Closed
herndlm wants to merge 3 commits intophpstan:1.10.xfrom
herndlm:array-keys-filter-value
Closed

Add support for strict $filter_value of array_keys()#2117
herndlm wants to merge 3 commits intophpstan:1.10.xfrom
herndlm:array-keys-filter-value

Conversation

@herndlm
Copy link
Copy Markdown
Contributor

@herndlm herndlm commented Dec 15, 2022

@herndlm herndlm force-pushed the array-keys-filter-value branch from 9140453 to ac5cccb Compare December 15, 2022 21:36
@herndlm herndlm marked this pull request as draft December 15, 2022 21:36
@herndlm herndlm force-pushed the array-keys-filter-value branch from ac5cccb to 83e8c5c Compare December 15, 2022 21:45
@herndlm herndlm marked this pull request as ready for review December 15, 2022 21:56
@phpstan-bot
Copy link
Copy Markdown
Collaborator

This pull request has been marked as ready for review.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

instaneceof *Type is rarely correct, and in this case it isn't :) You also need to account for mixed or bool being passed in here (so we cannot be sure if it's strict or not), so as always, isSuperTypeOf() wins :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

adapted, I think it should be more correct now? I still exit early the same way later though

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This logic should be able to use isSuperTypeOf yes/no/maybe.

Copy link
Copy Markdown
Contributor Author

@herndlm herndlm Dec 16, 2022

Choose a reason for hiding this comment

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

this is an edge case I think, it does already above to skip values for no, but here (with maybe|yes remaining) I want to mark maybe|yes as optional EXCEPT they are actually the same. example: filtering with bool makes the key with value true optional (because it's a supertype), but filtering with true should not make it optional (it's still a supertype though). does that make sense? I had a couple of such things already in the past, where I'm interested in basically $foo->isSuperTypeOf($bar)->yes() && !$foo->equals($bar)

@herndlm herndlm requested a review from ondrejmirtes December 16, 2022 13:32
@herndlm herndlm force-pushed the array-keys-filter-value branch 2 times, most recently from dceea49 to 5817915 Compare January 3, 2023 08:35
@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Jan 24, 2023

What do you think about this one? :)

@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Feb 3, 2023

it might make sense to wait if #2216 becomes available soon, then this can support loose comparison too relatively easily I hope

@herndlm herndlm changed the base branch from 1.9.x to 1.10.x February 25, 2023 17:12
@herndlm herndlm force-pushed the array-keys-filter-value branch 2 times, most recently from 1489714 to 9f0a15f Compare February 25, 2023 19:20
@herndlm herndlm force-pushed the array-keys-filter-value branch from 9f0a15f to 0331038 Compare May 23, 2023 14:57
@herndlm herndlm marked this pull request as draft June 11, 2023 18:38
@herndlm herndlm mentioned this pull request May 12, 2024
@ondrejmirtes
Copy link
Copy Markdown
Member

Superseded by #3066

@herndlm herndlm deleted the array-keys-filter-value branch May 31, 2024 07:07
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.

array_keys doesn't always return all keys

3 participants