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

ReflectionClass: show enums differently from classes, test output #15956

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DanielEScherzer
Copy link
Contributor

While internally enums are mostly the same as classes, their output in
ReflectionClass::__toString() should show the enum as the developer wrote it,
rather than as the engine stored it. Accordingly

  • Say that the enum is an enum, not a final class

  • Include the backing type, if any, in the declaration line

  • Remove the UnitEnum and BackedEnum interfaces from the list of interfaces
    implemented

  • List enum cases separately from constants, and show the underlying values, if
    any

Also add tests for the output - the tests are added in the first commit, so
that the changes in the second commit can be more easily confirmed.

GH-15766

In preparation for improving it, phpGH-15766
While internally enums are mostly the same as classes, their output in
`ReflectionClass::__toString()` should show the enum as the developer wrote it,
rather than as the engine stored it. Accordingly

- Say that the enum is an enum, not a final class

- Include the backing type, if any, in the declaration line

- Remove the `UnitEnum` and `BackedEnum` interfaces from the list of interfaces
implemented

- List enum cases separately from constants, and show the underlying values, if
any

phpGH-15766
@nielsdos
Copy link
Member

I didn't look at the code, but in general I'm in favor of the idea. It's too late for 8.4 though, and this may have BC implications if people rely on the output. So when we start on 8.5 you may need to email the ML to pitch this.

@DanielEScherzer
Copy link
Contributor Author

I didn't look at the code, but in general I'm in favor of the idea. It's too late for 8.4 though, and this may have BC implications if people rely on the output. So when we start on 8.5 you may need to email the ML to pitch this.

Yeah, there is no rush on this. Just wondering though, is the next version going to be 8.5, or 9.0? I wasn't able to find documentation on how many minor releases are done for each major release - since the github milestones show 8.4 and then 9.0, I assumed 9.0 was next - whichever it is, this can totally wait

@nielsdos
Copy link
Member

The version will be bumped to 9.0 if it's necessary to do so (e.g. large engine changes).
The next version is most likely 8.5 unless decided otherwise.

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

Successfully merging this pull request may close these issues.

2 participants