Skip to content

Treat ArrayType as covariant in its keys and values#2465

Merged
ondrejmirtes merged 2 commits intophpstan:1.10.xfrom
jiripudil:simplify-array-variance
Jun 17, 2023
Merged

Treat ArrayType as covariant in its keys and values#2465
ondrejmirtes merged 2 commits intophpstan:1.10.xfrom
jiripudil:simplify-array-variance

Conversation

@jiripudil
Copy link
Copy Markdown
Contributor

Following up on #2464, I believe the implementation in ArrayType can be simplified accordingly

@phpstan-bot
Copy link
Copy Markdown
Collaborator

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

@jiripudil jiripudil force-pushed the simplify-array-variance branch from d3d5390 to d8ddfbc Compare June 17, 2023 13:47
@jiripudil jiripudil changed the base branch from 1.11.x to 1.10.x June 17, 2023 13:54
@jiripudil jiripudil force-pushed the simplify-array-variance branch from d8ddfbc to 0673df9 Compare June 17, 2023 13:55
@ondrejmirtes
Copy link
Copy Markdown
Member

Is there a situation where the behaviour is different? Would be nice to have a test that fails without this change :)

This should be safe because it is copy-on-write
@jiripudil jiripudil force-pushed the simplify-array-variance branch from 0673df9 to 1cbd55b Compare June 17, 2023 16:56
@jiripudil
Copy link
Copy Markdown
Contributor Author

Actually, there is. Added a test. It looks like edge cases, but I guess it could happen that this change introduces new errors for someone 😬

@ondrejmirtes
Copy link
Copy Markdown
Member

I just checked the last PR that touched this: https://github.com/phpstan/phpstan-src/pull/30/files

And looks like all the tests that were added back then still pass :)

Thank you!

@ondrejmirtes ondrejmirtes merged commit 5a1a6af into phpstan:1.10.x Jun 17, 2023
@jiripudil jiripudil deleted the simplify-array-variance branch June 17, 2023 18:08
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.

3 participants