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

add ReflectionIntersectionType stub #7621

Merged
merged 1 commit into from
Feb 9, 2022
Merged

Conversation

orklah
Copy link
Collaborator

@orklah orklah commented Feb 9, 2022

This stub was missing, the goal is to fix current CI failure

@orklah
Copy link
Collaborator Author

orklah commented Feb 9, 2022

Ok, I understand what's going on.

First, the issue is due to our phpunit being updated by Matt yesterday, that's what changed.

But then, the issue come from phpunit requiring PHP 8.0 while still using ReflectionIntersectionType which is a PHP 8.1 class only.

However, this is not an issue because it's used in an instanceof, that doesn't emit PHP error in this case, but it's not handled by Psalm currently. PHPStan does not like that either: https://phpstan.org/r/9c8f7205-b3c0-40e7-818c-b555e2b55206

I guess we should just baseline those, it's quite an uncommon pattern and it deserve to be warned against in most cases

@AndrolGenhald
Copy link
Collaborator

You know, with the extension changes I'm doing I was planning to just leave internal extensions alone and let them use reflection, but this is actually a use case where it would make sense. If you want to run Psalm with PHP 8.0 on a codebase that requires PHP 8.1 it would be nice to have PHP 8.1 stubs for internal extensions.

It does seem like it's probably uncommon, so I'll worry about it last.

@orklah orklah added the release:feature The PR will be included in 'Features' section of the release notes label Feb 9, 2022
@orklah orklah merged commit fb1fd84 into vimeo:4.x Feb 9, 2022
@orklah
Copy link
Collaborator Author

orklah commented Feb 9, 2022

Internals are pretty useful too, that's why we have stubs of them. However, we just don't load stubs for higher versions when dealing with lower versions, hence the CI failure here

@AndrolGenhald
Copy link
Collaborator

Oh I see, I misunderstood what was going on then. So the problem here is that it's doing an instanceof check for a PHP 8.1 class while the project still allows PHP 8.0. I was thinking it was due to analyzing a PHP 8.1 project while running Psalm with PHP 8.0.

@sebastianbergmann
Copy link
Contributor

the issue come from phpunit requiring PHP 8.0

No stable version of PHPUnit requires PHP 8.0.

@orklah
Copy link
Collaborator Author

orklah commented Feb 10, 2022

@sebastianbergmann Thanks for your input :) we don't use a stable version however, we're hooked on your master branch (frozen on a fixed version of whenever we refreshed it)

right now, it needs PHP 8.0 or more: https://github.com/sebastianbergmann/phpunit/blob/master/composer.json#L25 but use a symbol introduced in 8.1, causing the error here

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

Successfully merging this pull request may close these issues.

3 participants