dbalabka / php-enumeration

Implementation of enumeration classes in PHP. The better alternative for enums
MIT License
53 stars 4 forks source link

Abstract implementation of Enumeration seems to break StaticConstructorLoader #18

Closed HaisojYellams closed 4 years ago

HaisojYellams commented 4 years ago

Please consider the following example (simplified code for readability):

EnumerationWrapper.php

abstract class EnumerationWrapper extends Enumeration{
// added some wrapper logic
}

FieldReadMode.php

final class FieldReadMode extends EnumerationWrapper{
    public static $none;
    public static $all;
}

If I add the FieldReadMode::initiailize() call to the end of the FieldReadMode.php file, everything works as expected, but if I instead use the StaticConstructorLoader class in order to simplify the initialization, I end up with an error like:

PHP Fatal error: During class fetch: Uncaught Error: Cannot instantiate abstract class EnumerationWrapper

I believe this is due to the __constructStatic() method within Enumeration initializing any child class, including abstract ones.

If I overlooked something and this is user error, please let me know. Otherwise I've come up with a solution below.


I was able to circumvent this issue by adding the following method to Enumeration:

protected static function shouldInitialize(): bool
{
    return true;
}

and changing the __constructStatic() method to be:

final public static function __constructStatic(): void
{
    if (self::class === static::class || !static::shouldInitialize())
    {
        return;
    }
    static::initialize();
}

and finally overriding the new method in EnumerationWrapper to be

protected static function shouldInitialize(): bool
{
    return self::class !== static::class;
}

I'm not saying this is the most elegant or valid solution, but it's one that is working for me currently.

If you would prefer I fork the project and make a PR with this implementation let me know. Thanks for your work on this project!

dbalabka commented 4 years ago

@HaisojYellams thanks you for reporting this but. The bug has been reproduced and will be fixed ASAP.

dbalabka commented 4 years ago

I would like to avoid any additional methods that will be required to know. It will affect developer experience. There is another option that will give the possibility to recognize abstract classes automatically but I don't like it. Need to make some benchmarks before fixing it.

HaisojYellams commented 4 years ago

Thanks for the update. Let me know if I can help in any way.