Merge pull request #139 from fre5h/opt-update

Opt update
fre5h · Dec 7, 2018 · f1ce1b7 · f1ce1b7
2 parents 6bb66b5 + 5555513
commit f1ce1b7
Show file tree

Hide file tree

Showing 10 changed files with 103 additions and 38 deletions.
diff --git a/DBAL/Types/AbstractEnumType.php b/DBAL/Types/AbstractEnumType.php
@@ -67,7 +67,7 @@ public function convertToPHPValue($value, AbstractPlatform $platform)
             return $value;
         }
 
-        // Check whether choice list is using integers as valies
+        // Check whether choice list is using integers as values
         $choice = static::$choices[$value];
         $choices = \array_flip(static::$choices);
         if (\is_int($choices[$choice])) {
@@ -177,8 +177,6 @@ public static function assertValidChoice(string $value): void
      * @static
      *
      * @return string $value Value in readable format
-     *
-     * @throws \InvalidArgumentException
      */
     public static function getReadableValue(string $value): string
     {

diff --git a/Form/EnumTypeGuesser.php b/Form/EnumTypeGuesser.php
@@ -28,7 +28,7 @@
  */
 class EnumTypeGuesser extends DoctrineOrmTypeGuesser
 {
-    /** @var AbstractEnumType[]|string[] */
+    /** @var string[] */
     protected $registeredEnumTypes = [];
 
     /**
@@ -84,9 +84,9 @@ public function guessType($class, $property): ?TypeGuess
             return null;
         }
 
-        // Get the choices from the fully qualified class name
+        /** @var AbstractEnumType $registeredEnumTypeFQCN */
         $parameters = [
-            'choices' => $registeredEnumTypeFQCN::getChoices(),
+            'choices' => $registeredEnumTypeFQCN::getChoices(), // Get the choices from the fully qualified class name
             'required' => !$metadata->isNullable($property),
         ];
 

diff --git a/FreshDoctrineEnumBundle.php b/FreshDoctrineEnumBundle.php
@@ -10,6 +10,8 @@
 
 namespace Fresh\DoctrineEnumBundle;
 
+use Doctrine\Common\Persistence\ConnectionRegistry;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpKernel\Bundle\Bundle;
 
 /**
@@ -22,6 +24,7 @@ class FreshDoctrineEnumBundle extends Bundle
     /**
      * {@inheritdoc}
      *
+     * @throws \InvalidArgumentException
      * @throws \Doctrine\DBAL\DBALException
      * @throws \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException
      * @throws \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException
@@ -30,8 +33,10 @@ public function boot(): void
     {
         parent::boot();
 
-        /** @var \Doctrine\Bundle\DoctrineBundle\Registry $doctrine */
-        $doctrine = $this->container->get('doctrine');
+        $doctrine = $this->container->get('doctrine', ContainerInterface::NULL_ON_INVALID_REFERENCE);
+        if (!$doctrine instanceof ConnectionRegistry) {
+            throw new \InvalidArgumentException('Service "doctrine" is missed in container');
+        }
 
         /** @var \Doctrine\DBAL\Connection $connection */
         foreach ($doctrine->getConnections() as $connection) {

diff --git a/Tests/FreshDoctrineEnumBundleTest.php b/Tests/FreshDoctrineEnumBundleTest.php
@@ -16,7 +16,7 @@
 use Fresh\DoctrineEnumBundle\FreshDoctrineEnumBundle;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\DependencyInjection\Container;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
  * FreshDoctrineEnumBundleTest
@@ -25,7 +25,7 @@
  */
 class FreshDoctrineEnumBundleTest extends TestCase
 {
-    /** @var Container|MockObject */
+    /** @var ContainerInterface|MockObject */
     private $container;
 
     /** @@var ManagerRegistry|MockObject */
@@ -34,23 +34,26 @@ class FreshDoctrineEnumBundleTest extends TestCase
     protected function setUp(): void
     {
         $this->doctrine = $this->createMock(ManagerRegistry::class);
-        $this->container = $this->createMock(Container::class);
-
-        $this->container
-            ->expects($this->once())
-            ->method('get')
-            ->with('doctrine')
-            ->willReturn($this->doctrine)
-        ;
+        $this->container = $this->createMock(ContainerInterface::class);
     }
 
     protected function tearDown(): void
     {
-        unset($this->container, $this->doctrine);
+        unset(
+            $this->container,
+            $this->doctrine
+        );
     }
 
     public function testEnumMappingRegistration(): void
     {
+        $this->container
+            ->expects(self::once())
+            ->method('get')
+            ->with('doctrine', ContainerInterface::NULL_ON_INVALID_REFERENCE)
+            ->willReturn($this->doctrine)
+        ;
+
         /**
          * @var AbstractPlatform|MockObject $databasePlatformAbc
          * @var AbstractPlatform|MockObject $databasePlatformDef
@@ -61,21 +64,20 @@ public function testEnumMappingRegistration(): void
         $connectionAbc = $this->createMock(Connection::class);
 
         $connectionAbc
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('getDatabasePlatform')
             ->willReturn($databasePlatformAbc)
         ;
 
         $connectionDef = $this->createMock(Connection::class);
-
         $connectionDef
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('getDatabasePlatform')
             ->willReturn($databasePlatformDef)
         ;
 
         $this->doctrine
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('getConnections')
             ->willReturn([$connectionAbc, $connectionDef])
         ;
@@ -93,19 +95,25 @@ public function testEnumMappingRegistration(): void
 
     public function testAlreadyRegisteredEnumMapping(): void
     {
+        $this->container
+            ->expects(self::once())
+            ->method('get')
+            ->with('doctrine', ContainerInterface::NULL_ON_INVALID_REFERENCE)
+            ->willReturn($this->doctrine)
+        ;
+
         /** @var AbstractPlatform|MockObject $databasePlatformAbc */
         $databasePlatformAbc = $this->getMockForAbstractClass(AbstractPlatform::class);
 
         $connectionAbc = $this->createMock(Connection::class);
-
         $connectionAbc
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('getDatabasePlatform')
             ->willReturn($databasePlatformAbc)
         ;
 
         $this->doctrine
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('getConnections')
             ->willReturn([$connectionAbc])
         ;
@@ -122,19 +130,26 @@ public function testAlreadyRegisteredEnumMapping(): void
 
     public function testEnumMappingReregistrationToString(): void
     {
+        $this->container
+            ->expects(self::once())
+            ->method('get')
+            ->with('doctrine', ContainerInterface::NULL_ON_INVALID_REFERENCE)
+            ->willReturn($this->doctrine)
+        ;
+
         /** @var AbstractPlatform|MockObject $databasePlatformAbc */
         $databasePlatformAbc = $this->getMockForAbstractClass(AbstractPlatform::class);
 
         $connectionAbc = $this->createMock(Connection::class);
 
         $connectionAbc
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('getDatabasePlatform')
             ->willReturn($databasePlatformAbc)
         ;
 
         $this->doctrine
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('getConnections')
             ->willReturn([$connectionAbc])
         ;
@@ -148,4 +163,21 @@ public function testEnumMappingReregistrationToString(): void
         self::assertTrue($databasePlatformAbc->hasDoctrineTypeMappingFor('enum'));
         self::assertEquals('string', $databasePlatformAbc->getDoctrineTypeMapping('enum'));
     }
+
+    public function testMissedDoctrine(): void
+    {
+        $this->container
+            ->expects(self::once())
+            ->method('get')
+            ->with('doctrine', ContainerInterface::NULL_ON_INVALID_REFERENCE)
+            ->willReturn(null)
+        ;
+
+        self::expectException(\InvalidArgumentException::class);
+        self::expectExceptionMessage('Service "doctrine" is missed in container');
+
+        $bundle = new FreshDoctrineEnumBundle();
+        $bundle->setContainer($this->container);
+        $bundle->boot();
+    }
 }
diff --git a/Tests/Twig/Extension/EnumConstantExtensionTest.php b/Tests/Twig/Extension/EnumConstantExtensionTest.php
@@ -18,6 +18,7 @@
 use Fresh\DoctrineEnumBundle\Exception\EnumType\NoRegisteredEnumTypesException;
 use Fresh\DoctrineEnumBundle\Tests\Fixtures\DBAL\Types\BasketballPositionType;
 use Fresh\DoctrineEnumBundle\Tests\Fixtures\DBAL\Types\MapLocationType;
+use Fresh\DoctrineEnumBundle\Tests\Fixtures\DBAL\Types\NumericType;
 use Fresh\DoctrineEnumBundle\Twig\Extension\EnumConstantTwigExtension;
 use PHPUnit\Framework\TestCase;
 use Twig\TwigFilter;
@@ -37,6 +38,7 @@ public function setUp(): void
         $this->enumConstantExtension = new EnumConstantTwigExtension([
             'BasketballPositionType' => ['class' => BasketballPositionType::class],
             'MapLocationType' => ['class' => MapLocationType::class],
+            'NumericType' => ['class' => NumericType::class],
         ]);
     }
 
@@ -54,7 +56,7 @@ public function testGetFilters(): void
     }
 
     /**
-     * @dataProvider dataProviderForGetReadableEnumValueTest
+     * @dataProvider dataProviderForGetEnumConstantTest
      */
     public function testGetEnumConstant(string $expectedValueOfConstant, string $enumConstant, ?string $enumType): void
     {
@@ -64,13 +66,14 @@ public function testGetEnumConstant(string $expectedValueOfConstant, string $enu
         );
     }
 
-    public function dataProviderForGetReadableEnumValueTest(): array
+    public function dataProviderForGetEnumConstantTest(): array
     {
         return [
             ['PG', 'POINT_GUARD', 'BasketballPositionType'],
             ['PG', 'POINT_GUARD', null],
             ['C', 'CENTER', 'BasketballPositionType'],
             ['C', 'CENTER', 'MapLocationType'],
+            ['3', 'THREE', 'NumericType'],
         ];
     }
 

diff --git a/Tests/Twig/Extension/EnumValuesAsArrayExtensionTest.php b/Tests/Twig/Extension/EnumValuesAsArrayExtensionTest.php
@@ -85,4 +85,19 @@ public function testNoRegisteredEnumTypesException(): void
         $this->expectException(NoRegisteredEnumTypesException::class);
         (new EnumValuesAsArrayTwigExtension([]))->getEnumValuesAsArray('MapLocationType');
     }
+
+    public function testInvalidCallable(): void
+    {
+        $extension = new EnumValuesAsArrayTwigExtension([]);
+
+        $property = new \ReflectionProperty(EnumValuesAsArrayTwigExtension::class, 'registeredEnumTypes');
+        $property->setAccessible(true);
+        $property->setValue($extension, ['invalid_callable' => 'dummy']);
+        $property->setAccessible(false);
+
+        self::expectException(\LogicException::class);
+        self::expectExceptionMessage('dummy::getReadableValues is not a valid exception');
+
+        $extension->getReadableEnumValuesAsArray('invalid_callable');
+    }
 }
diff --git a/Tests/Validator/EnumValidatorTest.php b/Tests/Validator/EnumValidatorTest.php
@@ -68,7 +68,7 @@ public function testValidBasketballPositionType(): void
         ]);
 
         $this->context
-            ->expects($this->never())
+            ->expects(self::never())
             ->method('buildViolation')
         ;
 
@@ -85,20 +85,20 @@ public function testInvalidBasketballPositionType(): void
         $constraintValidationBuilder = $this->createMock(ConstraintViolationBuilder::class);
 
         $constraintValidationBuilder
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('setParameter')
             ->with($this->equalTo('{{ value }}'), $this->equalTo('"Pitcher"'))
             ->will($this->returnSelf())
         ;
 
         $constraintValidationBuilder
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('setCode')
             ->will($this->returnSelf())
         ;
 
         $this->context
-            ->expects($this->once())
+            ->expects(self::once())
             ->method('buildViolation')
             ->with($this->equalTo('The value you selected is not a valid choice.'))
             ->will($this->returnValue($constraintValidationBuilder))

diff --git a/Twig/Extension/AbstractEnumTwigExtension.php b/Twig/Extension/AbstractEnumTwigExtension.php
@@ -24,7 +24,7 @@
  */
 abstract class AbstractEnumTwigExtension extends AbstractExtension
 {
-    /** @var AbstractEnumType[] */
+    /** @var string[] */
     protected $registeredEnumTypes = [];
 
     /** @var array */
@@ -36,8 +36,9 @@ abstract class AbstractEnumTwigExtension extends AbstractExtension
     public function __construct(array $registeredTypes)
     {
         foreach ($registeredTypes as $type => $details) {
-            if (\is_subclass_of($details['class'], AbstractEnumType::class)) {
-                $this->registeredEnumTypes[$type] = $details['class'];
+            $class = $details['class'];
+            if (\is_subclass_of($class, AbstractEnumType::class) && \is_string($class)) {
+                $this->registeredEnumTypes[$type] = $class;
             }
         }
     }

diff --git a/Twig/Extension/EnumValuesAsArrayTwigExtension.php b/Twig/Extension/EnumValuesAsArrayTwigExtension.php
@@ -74,7 +74,13 @@ private function callEnumTypeStaticMethod(string $enumType, string $staticMethod
         if ($this->hasRegisteredEnumTypes()) {
             $this->throwExceptionIfEnumTypeIsNotRegistered($enumType);
 
-            return \call_user_func([$this->registeredEnumTypes[$enumType], $staticMethodName]);
+            $function = [$this->registeredEnumTypes[$enumType], $staticMethodName];
+
+            if (!\is_callable($function)) {
+                throw new \LogicException(\sprintf('%s::%s is not a valid exception', $this->registeredEnumTypes[$enumType], $staticMethodName));
+            }
+
+            return \call_user_func($function);
         }
 
         throw $this->createNoRegisteredEnumTypesException();

diff --git a/phpstan.neon b/phpstan.neon
@@ -1,4 +1,9 @@
+includes:
+    - 'vendor/phpstan/phpstan-phpunit/extension.neon'
+    - 'vendor/phpstan/phpstan-phpunit/rules.neon'
+    - 'vendor/phpstan/phpstan-doctrine/extension.neon'
 parameters:
+    level: 7
     excludes_analyse:
         - '%rootDir%/../../../Tests/*'
         - '%rootDir%/../../../vendor/*'