Skip to content

Add generic typings to array_uintersect - #3806

Closed
axlon wants to merge 5 commits into
phpstan:2.1.xfrom
axlon:array_uintersect
Closed

Add generic typings to array_uintersect#3806
axlon wants to merge 5 commits into
phpstan:2.1.xfrom
axlon:array_uintersect

Conversation

@axlon

@axlon axlon commented Feb 4, 2025

Copy link
Copy Markdown
Contributor

This PR adds generic typings to array_uintersect, I copied the annotations from array_udiff

@axlon

axlon commented Feb 4, 2025

Copy link
Copy Markdown
Contributor Author

This PR is a partial fix for phpstan/phpstan#7707, if this PR is the correct way to do this I can add support for the other functions mentioned as well

@axlon
axlon marked this pull request as draft February 4, 2025 10:14
@axlon
axlon marked this pull request as ready for review February 4, 2025 10:28
@phpstan-bot

Copy link
Copy Markdown
Collaborator

This pull request has been marked as ready for review.

Comment thread stubs/arrayFunctions.stub Outdated
@schlndh

schlndh commented Feb 7, 2025

Copy link
Copy Markdown
Contributor

@axlon BTW: The same fix is already implemented here: #3282

@axlon

axlon commented Feb 7, 2025

Copy link
Copy Markdown
Contributor Author

@axlon BTW: The same fix is already implemented here: #3282

I see, I missed this when looking through issues. However there's a slight difference in that the arrays can be different per @ondrejmirtes' suggestion.

The same issue with that PR is still present here; the definitions of these functions are wrong, but merging this would get us closer to PHPStan fully understanding these functions

@schlndh

schlndh commented Feb 7, 2025

Copy link
Copy Markdown
Contributor

@axlon You're right. Feel free to take whatever you want from the original PR, if you decide to expand this one to cover all similar functions. Alternatively, I'm also around to update my PR if ondrejmirtes prefers that.

Comment thread stubs/arrayFunctions.stub Outdated
@axlon
axlon marked this pull request as draft February 10, 2025 09:40
@axlon

axlon commented Feb 10, 2025

Copy link
Copy Markdown
Contributor Author

I'll try to take a look at this soon

@axlon

axlon commented Feb 25, 2025

Copy link
Copy Markdown
Contributor Author

I think I've found a way to document these functions where they're quite accurate (so far), but I hit a roadblock with array shapes; PHPStan seems to add an integer key to the return type whenever the first argument is an array shape. I've added failing tests in the last commit and I will try to figure out why this is happening

@schlndh

schlndh commented Feb 25, 2025

Copy link
Copy Markdown
Contributor

@axlon It looks like it might be possible to work around it by using array<K, V> and then K, instead of T of array and key-of<T>: https://phpstan.org/r/f5f9cb93-4b83-4042-a91f-cd76d4a1789c

@axlon

axlon commented Feb 25, 2025

Copy link
Copy Markdown
Contributor Author

@axlon It looks like it might be possible to work around it by using array<K, V> and then K, instead of T of array and key-of<T>: https://phpstan.org/r/f5f9cb93-4b83-4042-a91f-cd76d4a1789c

I'll check that out! Not only does this fix the issue, it also makes the return types more accurate 👌

@axlon

axlon commented Feb 25, 2025

Copy link
Copy Markdown
Contributor Author

@ondrejmirtes please check this out when you find the time, and let me know if this is the way forward. If it is I'll work on adding the remaining array functions listed in the original issue.

As far as the failing tests go:

  • The PHP 7.4 tests probably fail because of mixed (I suppose I should exclude them for PHP 7.4?)
  • The other failing workflows seem unrelated (to my eyes)

@axlon
axlon marked this pull request as ready for review February 25, 2025 19:05
@phpstan-bot

Copy link
Copy Markdown
Collaborator

This pull request has been marked as ready for review.

@ondrejmirtes

Copy link
Copy Markdown
Member

#3282 seemed a bit more complete to me so I merged that. Thank you.

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.

4 participants