Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: isEnum class-string narrowing #4474

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

Draft
SamMousa wants to merge 4 commits into phpstan:2.1.x
base: 2.1.x
Choose a base branch
Loading
from SamMousa:feat-improve-reflectionstub

Conversation

@SamMousa
Copy link

@SamMousa SamMousa commented Oct 24, 2025

Copy link
Author

It seems many of the failures are not related to my changes....

Copy link
Contributor

staabm commented Oct 24, 2025

yes, atm a few spurious errors are in CI.

these here are related though: https://github.com/phpstan/phpstan-src/actions/runs/18773626957/job/53563151392?pr=4474

@SamMousa SamMousa marked this pull request as draft October 24, 2025 10:31
Copy link
Author

I'll have another look after the weekend, for now any annotation I add seems to have no effect on the asserted type.

Copy link
Author

It feels like the annotations in the ReflectionClass.stub file are ignored.

 /**
 * @return false
 */
 public function isEnum(): bool
 {
 }
Failed assertions in /projects/phpstan-src/tests/PHPStan/Analyser/nsrt/reflectionclass-isEnum.php:
Line 13:
Expected: false
Actual: bool

Is there some kind of build step for the stubs that i'm missing?

Copy link
Member

There's no build step for stubs. I don't know why it does not work for you but I'd argue that:

 /**
 * @return false
 */
 public function isEnum(): bool
 {
 }

is wrong: https://3v4l.org/AZfkh

Copy link
Author

It is wrong 100%, but my objective here was to test if the annotation is working at all.
The simplest annotation I could think of was @return false....

And if this already doesn't work I need to figure that part out first before I test my actual more complex annotations...

Copy link
Member

Do not forget to also update ReflectionClassWithLazyObjects.stub.

SamMousa reacted with laugh emoji

Copy link
Author

Do not forget to also update ReflectionClassWithLazyObjects.stub.

That was the cause, I did not realize there are 2 stubs for this class. Thanks, now I can get to work on the actual implementation

/**
* @phpstan-assert-if-true class-string<\UnitEnum> $this->name
* @phpstan-assert-if-true class-string<\UnitEnum> $this->getName()
* @phpstan-assert-if-true ReflectionClass<UnitEnum> $this
Copy link
Author

@SamMousa SamMousa Oct 27, 2025
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion does not work currently. Is there anyway to narrow the type T for a generic?

I've tried using @phsptan-assert-if-true UnitEnum T, but that doesn't work.

Copy link
Author

SamMousa commented Oct 27, 2025
edited
Loading

Current implementation works! I'd like to actually narrow the class (template type T) of the reflection object as well, but that doesn't seem to work yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@staabm staabm staabm left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /