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

Fix ReflectionEnum::newInstance() return type #1379

Merged
merged 1 commit into from
Nov 19, 2023

Conversation

ondrejmirtes
Copy link
Contributor

PHP does not complain about this (because parent has object return type) but it's wrong. I found out about it after introducing generics to ReflectionEnum: phpstan/phpstan-src@9ce8faf

 ------ ------------------------------------------------------------------------- 
  Line   src/Reflection/Adapter/ReflectionEnum.php                                
 ------ ------------------------------------------------------------------------- 
  364    Return type                                                              
         (Roave\BetterReflection\Reflection\Adapter\ReflectionEnum) of method     
         Roave\BetterReflection\Reflection\Adapter\ReflectionEnum::newInstance()  
         should be compatible with return type (UnitEnum) of method               
         ReflectionClass<UnitEnum>::newInstance()                                 
 ------ ------------------------------------------------------------------------- 

@Ocramius
Copy link
Member

Wait, did this pass before because self is a subtype of object? Sounds like we never call this API in our tests? 🤔

@Ocramius Ocramius added the bug label Nov 19, 2023
@Ocramius Ocramius added this to the 6.17.0 milestone Nov 19, 2023
@ondrejmirtes
Copy link
Contributor Author

@Ocramius It's fine even when called because the method always throws...

@Ocramius
Copy link
Member

Waiting for CI, then shipping this 👍

@Ocramius Ocramius self-assigned this Nov 19, 2023
@Ocramius Ocramius merged commit beb49d9 into Roave:6.17.x Nov 19, 2023
@Ocramius
Copy link
Member

Thanks @ondrejmirtes!

@Ocramius Ocramius mentioned this pull request Nov 19, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants