Merge pull request #21 from dbalabka/fix-for-abstract-classes

Throw an exception on non-final enum class and public constructor. Fix abstract classes initialisation.

Author: dbalabka

.travis.yml

@@ -6,7 +6,7 @@ matrix:
     - php: '7.1'
     - php: '7.2'
     - php: '7.3'
-    - php: '7.4snapshot'
+    - php: '7.4'
 
 before_install:
   - phpenv config-rm xdebug.ini || echo "No xdebug config."

examples/Enum/CardType.php

@@ -7,11 +7,11 @@
 
 final class CardType extends Enumeration
 {
-    /** @var $this */
+    /** @var self */
     public static $amex;
-    /** @var $this */
+    /** @var self */
     public static $visa;
-    /** @var $this */
+    /** @var self */
     public static $masterCard;
 
     protected static function initializeValues() : void

examples/Enum/Circle.php

@@ -5,7 +5,7 @@
 
 use Dbalabka\Enumeration\Examples\Struct\Point;
 
-class Circle extends Shape
+final class Circle extends Shape
 {
     private Point $point;
     private float $radius;

examples/Enum/Color.php

@@ -7,16 +7,16 @@
 
 final class Color extends Enumeration
 {
-    /** @var $this */
+    /** @var self */
     public static $red;
-    /** @var $this */
+    /** @var self */
     public static $green;
-    /** @var $this */
+    /** @var self */
     public static $blue;
 
     private $value;
 
-    public function __construct(int $value)
+    protected function __construct(int $value)
     {
         $this->value = $value;
     }

examples/Enum/Option.php

@@ -11,7 +11,7 @@
  * @author Dmitry Balabka <dmitry.balabka@gmail.com>
  * @template T
  */
-class Option extends Enumeration
+abstract class Option extends Enumeration
 {
     /**
      * @psalm-var Option<mixed>

examples/Enum/Rectangle.php

@@ -5,7 +5,7 @@
 
 use Dbalabka\Enumeration\Examples\Struct\Point;
 
-class Rectangle extends Shape
+final class Rectangle extends Shape
 {
     private Point $pointA;
     private Point $pointB;

src/Enumeration/Enumeration.php

@@ -6,6 +6,7 @@
 use Dbalabka\Enumeration\Exception\EnumerationException;
 use Dbalabka\Enumeration\Exception\InvalidArgumentException;
 use Dbalabka\StaticConstructorLoader\StaticConstructorInterface;
+use ReflectionClass;
 use Serializable;
 use function array_search;
 use function get_class_vars;
@@ -31,6 +32,9 @@ abstract class Enumeration implements StaticConstructorInterface, Serializable
     /** @var array */
     private static $initializedEnums = [];
 
+    /**
+     * @throws EnumerationException
+     */
     final public static function __constructStatic() : void
     {
         if (self::class === static::class) {
@@ -44,9 +48,23 @@ final public static function initialize(): void
         if (isset(self::$initializedEnums[static::class])) {
             return;
         }
-        self::$initializedEnums[static::class] = true;
+
+        /**
+         * Using reflections we can enforce the strict rules of enums declaration.
+         * It should not rise performance issue because enumeration class initialization executes only once.
+         */
+        $classReflection = new ReflectionClass(static::class);
+        if (!$classReflection->isAbstract() && !$classReflection->isFinal()) {
+            throw new EnumerationException('Enumeration class should be declared as final');
+        }
+        $method = $classReflection->getConstructor();
+        if ($method && $method->isPublic()) {
+            throw new EnumerationException('Enumeration class constructor should not be public');
+        }
+
         static::initializeValues();
         static::initializeOrdinals();
+        self::$initializedEnums[static::class] = true;
     }
 
     final protected static function initializeOrdinals() : void
@@ -63,6 +81,14 @@ final protected static function initializeOrdinals() : void
      */
     protected static function initializeValues() : void
     {
+        /**
+         * TODO: Unfortunately, it is impossible to avoid class reflection here.
+         *       The reflection caching here should not give major performance benefit.
+         *       It should be properly benchmarked.
+         */
+        if ((new ReflectionClass(static::class))->isAbstract()) {
+            return;
+        }
         $firstEnumItem = new static();
 
         $staticVars = static::getEnumStaticVars($firstEnumItem);
@@ -116,6 +142,9 @@ protected function __construct()
     {
     }
 
+    /**
+     * @throws EnumerationException
+     */
     final public function ordinal() : int
     {
         if (null === $this->ordinal) {
@@ -131,9 +160,15 @@ final public function ordinal() : int
                 $ordinal++;
             }
         }
+        if ($this->ordinal === null) {
+            throw new EnumerationException('Ordinal initialization failed. Enum does not contain any static variables');
+        }
         return $this->ordinal;
     }
 
+    /**
+     * @throws EnumerationException
+     */
     final public function compareTo(self $enum) : int
     {
         return $this->ordinal() - $enum->ordinal();
@@ -154,7 +189,7 @@ final public function name() : string
         if (false === $name) {
             throw new EnumerationException(
                 sprintf(
-                    'Can not find $this in $s::values(). ' .
+                    'Can not find $this in %s::values(). ' .
                     'It seems that the static property was overwritten. This is not allowed.',
                     get_class($this)
                 )

tests/Enumeration/EnumerationTest.php

@@ -9,7 +9,10 @@
 use Dbalabka\Enumeration\Tests\Fixtures\Action;
 use Dbalabka\Enumeration\Tests\Fixtures\ActionWithCustomStaticProperty;
 use Dbalabka\Enumeration\Tests\Fixtures\ActionWithPublicConstructor;
+use Dbalabka\Enumeration\Tests\Fixtures\EmptyEnum;
 use Dbalabka\Enumeration\Tests\Fixtures\Flag;
+use Dbalabka\Enumeration\Tests\Fixtures\NotFinalEnum;
+use Dbalabka\Enumeration\Tests\Fixtures\PublicConstructorEnum;
 use Error;
 use PHPUnit\Framework\Error\Warning;
 use PHPUnit\Framework\TestCase;
@@ -175,7 +178,7 @@ public function testCompareTo()
 
     public function testCustomStaticProperties()
     {
-        $this->markTestSkipped('Custom static properties does not allowed');
+        $this->markTestSkipped('Custom static properties are not allowed');
         ActionWithCustomStaticProperty::initialize();
 
         ActionWithCustomStaticProperty::$customProperty = ActionWithCustomStaticProperty::$edit;
@@ -202,4 +205,27 @@ public function testNameWhenIncorrectlyInitilizedProperies()
         $this->expectException(EnumerationException::class);
         $notOk->name();
     }
+
+    public function testNotFinalEnumShouldThrowAnException()
+    {
+        $this->expectException(EnumerationException::class);
+        $this->expectExceptionMessage('Enumeration class should be declared as final');
+        NotFinalEnum::initialize();
+        NotFinalEnum::$testValue;
+    }
+
+    public function testPublicConstructorShouldThrowAnException()
+    {
+        $this->expectException(EnumerationException::class);
+        $this->expectExceptionMessage('Enumeration class constructor should not be public');
+        PublicConstructorEnum::initialize();
+        PublicConstructorEnum::$testValue;
+    }
+
+    public function testOrdinalInitilizationFailedShouldThrowException()
+    {
+        $this->expectException(EnumerationException::class);
+        $this->expectExceptionMessage('Ordinal initialization failed. Enum does not contain any static variables');
+        EmptyEnum::initialize();
+    }
 }

tests/Enumeration/Fixtures/AbstractAction.php

@@ -0,0 +1,22 @@
+<?php
+declare(strict_types=1);
+
+namespace Dbalabka\Enumeration\Tests\Fixtures;
+
+use Dbalabka\Enumeration\Enumeration;
+use function version_compare;
+use const PHP_VERSION;
+
+if (version_compare(PHP_VERSION, '7.4.0', '<')) {
+    require_once __DIR__ . '/ActionProperties.php';
+} else {
+    require_once __DIR__ . '/ActionTypedProperties.php';
+}
+
+/**
+ * Final is omitted for testing purposes
+ */
+abstract class AbstractAction extends Enumeration
+{
+    use ActionProperties;
+}

tests/Enumeration/Fixtures/Action.php

@@ -3,20 +3,7 @@
 
 namespace Dbalabka\Enumeration\Tests\Fixtures;
 
-use Dbalabka\Enumeration\Enumeration;
-use function version_compare;
-use const PHP_VERSION;
-
-if (version_compare(PHP_VERSION, '7.4.0beta', '<')) {
-    require_once __DIR__ . '/ActionProperties.php';
-} else {
-    require_once __DIR__ . '/ActionTypedProperties.php';
-}
-
-/**
- * Final is omitted for testing purposes
- */
-class Action extends Enumeration
+final class Action extends AbstractAction
 {
-    use ActionProperties;
+
 }