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

Type of ReflectionEnum::getBackingType() should/could be narrowed after ::isBacked() #10167

Closed
RobertMe opened this issue Nov 20, 2023 · 8 comments

Comments

@RobertMe
Copy link

Bug report

The formal return value of ReflectionEnum::getBackingType() is ?ReflectionNamedType. But it will always return ReflectionNamedType (i.e.: be non-null) for backed enums. So when a ReflectionEnum::isBacked condition is applied the type could/should be narrowed.

Code snippet that reproduces the problem

https://phpstan.org/r/6708f7db-5547-47c6-aa37-38dc5974a1a4

Expected output

Type could/should be just ReflectionNamedType (but is ReflectionNamedType|null).

Did PHPStan help you today? Did it make you happy in any way?

No response

@ondrejmirtes
Copy link
Member

Alternatively you can write this more SA-friendly code https://phpstan.org/r/03d3e5a0-4820-4c4f-a481-c728f6f14735

@RobertMe
Copy link
Author

RobertMe commented Nov 20, 2023

I think this would be a relatively simple issue which I might fix myself given some pointers. I think it might even be possible to implement it using simple assertions in a stub file?

Based on narrowing types (especially the hasName / getName example) I think it should be possible to write a stubs/ReflectionEnum.stub (and add it to the stubFiles in `config/config.neon). This could then just contain:

<?php

class ReflectionEnum
{
    /**
     * @phpstan-assert-if-true !null $this->getBackingType()
     */
    public function isBacked()
    {
    }
}

Correct? I'll then just need some pointers on testing the stub

@RobertMe
Copy link
Author

Alternatively you can write this more SA-friendly code https://phpstan.org/r/03d3e5a0-4820-4c4f-a481-c728f6f14735

First off, wow, that reply was way quicker then I expected 😃. Secondly, you're right, not sure why I didn't think of that. But still would be nice if PHPStan automatically narrowed the type 😅. And might be a good "first issue" for myself to learn more about PHPStan "internals".

@ondrejmirtes
Copy link
Member

Yeah, sure, should be easy. Just check how I tested the return type of getCase/getCases around the same commit ReflectionEnum stub was added.

Hint: in NodeScopeResolverTest

@franmomu
Copy link

franmomu commented Dec 5, 2023

Would be possible to also narrow the template type to BackedEnum (after calling isBacked())? that way $reflectionEnum->getName() would be class-string<BackedEnum> I guess.

https://phpstan.org/r/63e742a3-f3b7-4d5a-9c6e-3a60de5bfa32

Update: Just saw #10192 (comment)

@phpstan-bot
Copy link
Contributor

@franmomu After the latest push in 1.11.x, PHPStan now reports different result with your code snippet:

@@ @@
 ==========
 
 6: Dumped type: class-string<UnitEnum>
-8: Dumped type: class-string<UnitEnum>
+8: Dumped type: class-string<BackedEnum>
 
 PHP 7.2 – 8.0 (7 errors)
 ==========
Full report

PHP 8.1 – 8.3 (2 errors)

Line Error
6 Dumped type: class-string<UnitEnum>
8 Dumped type: class-string<BackedEnum>

PHP 7.2 – 8.0 (7 errors)

Line Error
4 Parameter $class of function test() has invalid type UnitEnum.
5 Instantiated class ReflectionEnum not found.
6 Call to method getName() on an unknown class ReflectionEnum.
6 Dumped type: *ERROR*
7 Call to method isBacked() on an unknown class ReflectionEnum.
8 Call to method getName() on an unknown class ReflectionEnum.
8 Dumped type: *ERROR*

@ondrejmirtes
Copy link
Member

Fixed phpstan/phpstan-src#2830

Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants