Modify the enum implementation for compatibility with PHP 8.1 enumeration

[drealecs]

This is intended as a new version that will be compatible with some of the feature of the enumeration implemented in PHP 8.1
Also in the same time this will enforce good patterns of using the enum, similarly with how they will be enforced in PHP 8.1 enumeration so library dependents will be able to easily migrate.

This can only be released as a new major version, 2.0.

BC breaks related to v1:

• class must be declared final
• constructor is private and the way to obtain an instance by key is using static method call with key name
• value property is private instead of protected
• static cache properties are private instead of protected
• public methods are final
• values can only be int or string
• all values of an enum must be of only one type, either int or string

There is also a new method tryFrom(), similar with from() that returns null when value is not part of the enum.

There is currently a discussion so that enum keywork will not be reserved and this way the library will be compatible with PHP 8.1 and higher until 9.0 maybe. (https://externals.io/message/113218)

[mnapoli]

Hi!

Thanks for the PR. I'll be honest, I get the "how/what" you did, but I don't understand the "why".

This is intended as a new version that will be compatible with most of the feature of the enumeration implemented in PHP 8.1

Right, but why do we need to break backward compatibility when we are so close to migrating to native enums? The last thing I want is to mess up everyone's usage of this library, and a bc break for a soon-to-be-obsolete package doesn't make sense unless we have a very strong reason for this, does that make sense?

[drealecs]

Hi @mnapoli, and thank you for the "why?" question.
If I would have answered it for myself earlier, I would have found the answer for my issue, if it should be for 7.3 or 8.0.

The My C-Labs Enum implementation, when it was created 7-8 years ago (mostly by you) was not designed with a strict purpose. However, given it's flexibility and the fact that it was simpler and better than anything that existed allowed it to gain good traction and that's good.

Unfortunately, I have seen a great deal of cases where the library was used in a wrong way and along these years, I think you and the community noted several times the BC breaking changes that would have improve it:

• Enum instances aren't singletons #8 (comment)
• Construction of enum object with invalid value is possible #95 (comment)

For a few years already, the only way (mostly) an usage of the enum library passes on code reviews for me is if the class is defined like this:

/**
 * @method static EnumType FOO()
 * @method static EnumType BAR()
 */
final class EnumType extends \MyCLabs\Enum\Enum
{
    private const FOO = 'foo';
    private const BAR = 'bar';
}

• classes are final as the inheritance for enums just doesn't makes sense.
• constants are private to discourage using the constant value directly
• no construct implementation as it should have been final private
• no static method overriding as default behavior is the correct for enum

I've see "wrong" usages that breaks the enum concept.

• using only the value of the constants and not the class instances; this should not be extending Enum as it's not using it's features
• using the keys() and toArray() method only and not the class instances; this should not be extending Enum as it's not using it's features and it would be better implementing the method without reflection.
• calling getValue() too early, like immediately after creation and not taking advantage of Enum as a type on methods.
• ... and others

I'm happy that most of the places I've reviewed and/or fixed are compatible with native enums in PHP 8.1.
I would like to improve on the way enum library is used so the library dependents's code quality improves.
As PHP 8.1 is not out yet and I think it will take around 3-4 years until php native enums will start being used, I would actually want to have the changes for PHP 7.3, not 8.0 as implemented now, answering my own question.

I thought about it a bit more now and from the previously listed BC breaks, I think we can drop some that can be dealt with when moving from userland enum 2.0 to native enums in PHP >= 8.1, namely these 2:

• values can only be int or string
• all values of an enum must be of only one type, either int or string

This can be dealt with later and it's not breaking on the "correctness" of the enum usage, only some performance optimizations looking up the value.
Let me know what you think.
I'll find some time probably in about 2-3 days to provide an implementation based on the changes I propose to the current version: minimum 7.3 and drop some BC breaks.

[drealecs]

Updated the implementation

[mnapoli]

Thanks, I still don't understand it.

The project is about to die in a few years, we've lasted more than 8 year without BC breaks, why do that now?

Teams that want a stricter approach will probably those that can afford to upgrade to PHP 8.1.

For open-source projects that can't require PHP 8.1 soon, releasing a php-enum v2.0 will not be useful to them as this could create conflicts all over the place (like Guzzle 4/5/6 but worse).

[drealecs]

Yes, I understand what you mean and I agree with it.

Maybe some guidelines on how to upgrade, once enums in PHP 8.1 are available will be just enough.

My intention was not to have a version 2.0 that would deprecate the 1.x version immediately.
I still believe it's very easy not to use the recommended way (available in readme) and have a harder time when upgrading to native enums but I agree that for current users it might just be a bigger overhead to migrate and probably they won't care about doing.

I'll ponder a bit more on how the current code direction could help, I could use it in some private projects or find another way to release it as a separate package if that might be needed. In the end this might be a better option, to not force the upgrade of myclabs/enum but offer another enum package that should be used in new places, if that's needed, to allow for a simpler migration.