Skip to content
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

Simplify ForeachAnalyzer::getKeyValueParamsForTraversableObject. #8052

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

AndrolGenhald
Copy link
Collaborator

Depends on #8044.

 - Correctly infer `array` and `list` instead of `non-empty-array` and `non-empty-list` (fixes vimeo#7296)
 - Add support for spreading string keys (fixes vimeo#7297).
 - Show issue when trying to unpack non-iterable
 - Show issue when trying to unpack iterable with non-array-key key
 - Re-added invalid PHP 8.0 tests removed in vimeo#6613
 - Unpacked lists with known keys will be inferred as eg `array{0: int, 1: int}<int<0, max>, int>` now but will still be treated as lists
 - Unpacked arrays with known keys will now be inferred as eg `array{a: string, b: string}<int, int>` instead of `array<int|string, int|string>`
@AndrolGenhald AndrolGenhald added release:internal The PR will be included in 'Internal changes' section of the release notes PR: blocked labels Jun 2, 2022
@orklah
Copy link
Collaborator

orklah commented Dec 20, 2022

@AndrolGenhald the blocker is no longer blocking. Should we merge this after conflicts are resolved?

@orklah orklah marked this pull request as draft December 20, 2022 18:34
@AndrolGenhald
Copy link
Collaborator Author

I'll take a look at it soon, I think this was a pretty good simplification reusing other code instead of duplicating a bunch of complicated functionality, so as long as the tests pass it should be ok to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:internal The PR will be included in 'Internal changes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants