AllowListParser allows empty list by default + always appends null type

kynx · Jan 27, 2024 · 9e823d1 · 9e823d1
1 parent 12bac9a
commit 9e823d1
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 30 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,5 +1,6 @@
 /.phpcs-cache
 /.phpunit.cache
 /.phpunit.result.cache
+/coverage/
 /phpunit.xml
 /vendor/
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -8,7 +8,7 @@
          failOnRisky="true"
          failOnWarning="true"
          beStrictAboutOutputDuringTests="true"
-         beStrictAboutCoverageMetadata="true">
+         beStrictAboutCoverageMetadata="false">
   <testsuites>
     <testsuite name="default">
       <directory suffix="Test.php">test</directory>

diff --git a/src/ArrayShape/Filter/AllowListParser.php b/src/ArrayShape/Filter/AllowListParser.php
@@ -26,7 +26,7 @@
 {
     public const DEFAULT_MAX_LITERALS = 10;
 
-    public function __construct(private int $maxLiterals = self::DEFAULT_MAX_LITERALS)
+    public function __construct(private bool $allowEmptyList = true, private int $maxLiterals = self::DEFAULT_MAX_LITERALS)
     {
     }
 
@@ -36,15 +36,22 @@ public function getTypes(FilterInterface $filter, array $existing): array
             return $existing;
         }
 
-        if (count($filter->getList()) > $this->maxLiterals) {
+        $list = $filter->getList();
+        if ($list === []) {
+            return $this->allowEmptyList ? $existing : [PsalmType::Null];
+        }
+
+        $existing[] = PsalmType::Null;
+
+        if (count($list) > $this->maxLiterals) {
             return $filter->getStrict() === true
-                ? $this->getStrictTypes($filter->getList(), $existing)
-                : $this->getLaxTypes($filter->getList(), $existing);
+                ? $this->getStrictTypes($list, $existing)
+                : $this->getLaxTypes($list, $existing);
         }
 
         $types = $filter->getStrict() === true
-            ? $this->getStrictLiteral($filter->getList(), $existing)
-            : $this->getLaxLiteral($filter->getList(), $existing);
+            ? $this->getStrictLiteral($list, $existing)
+            : $this->getLaxLiteral($list, $existing);
         return $this->appendUnique(PsalmType::Null, $types, $existing);
     }
 
@@ -93,6 +100,8 @@ private function getStrictLiteral(array $list, array $existing): array
             }
         }
 
+        $types = $this->appendUnique(PsalmType::Null, $types, $existing);
+
         if ($literals !== []) {
             $types[] = new Literal($literals);
         }
@@ -125,6 +134,7 @@ private function getLaxLiteral(array $list, array $existing): array
         if ($types !== []) {
             $types = $this->appendUnique(PsalmType::String, $types, $existing);
         }
+        $types = $this->appendUnique(PsalmType::Null, $types, $existing);
 
         if ($literals !== []) {
             $types[] = new Literal($literals);

diff --git a/src/ArrayShape/Filter/AllowListParserFactory.php b/src/ArrayShape/Filter/AllowListParserFactory.php
@@ -15,9 +15,10 @@ public function __invoke(ContainerInterface $container): AllowListParser
         /** @var ConfigProviderArray $config */
         $config = $container->get('config');
         // phpcs:disable Generic.Files.LineLength.TooLong
+        $allowEmpty = (bool)($config['laminas-form-cli']['array-shape']['filter']['allow-list']['allow-empty-list'] ?? true);
         $maxLiterals = (int) ($config['laminas-form-cli']['array-shape']['filter']['allow-list']['max-literals'] ?? AllowListParser::DEFAULT_MAX_LITERALS);
         // phpcs:enable
 
-        return new AllowListParser($maxLiterals);
+        return new AllowListParser($allowEmpty, $maxLiterals);
     }
 }
diff --git a/test/ArrayShape/Filter/AllowListParserFactoryTest.php b/test/ArrayShape/Filter/AllowListParserFactoryTest.php
@@ -8,12 +8,11 @@
 use Kynx\Laminas\FormCli\ArrayShape\Type\Literal;
 use Kynx\Laminas\FormCli\ArrayShape\Type\PsalmType;
 use Laminas\Filter\AllowList;
+use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\TestCase;
 use Psr\Container\ContainerInterface;
 
-/**
- * @covers \Kynx\Laminas\FormCli\ArrayShape\Filter\AllowListParserFactory
- */
+#[CoversClass(AllowListParserFactory::class)]
 final class AllowListParserFactoryTest extends TestCase
 {
     public function testInvokeReturnsDefaultInstance(): void
@@ -25,26 +24,33 @@ public function testInvokeReturnsDefaultInstance(): void
         $factory  = new AllowListParserFactory();
         $instance = $factory($container);
 
-        $expected = [new Literal(["'a'"])];
+        $expected = [PsalmType::Null, new Literal(["'a'"])];
         $filter   = new AllowList(['list' => ['a'], 'strict' => true]);
         $actual   = $instance->getTypes($filter, [PsalmType::String]);
 
         self::assertEquals($expected, $actual);
     }
 
-    public function testInvokeReturnsConfiguredInstance(): void
+    public function testInvokeConfiguresAllowEmptyList(): void
     {
-        $config    = [
-            'laminas-form-cli' => [
-                'array-shape' => [
-                    'filter' => [
-                        'allow-list' => [
-                            'max-literals' => 0,
-                        ],
-                    ],
-                ],
-            ],
-        ];
+        $config    = $this->getConfig(['allow-empty-list' => false]);
+        $container = $this->createStub(ContainerInterface::class);
+        $container->method('get')
+            ->willReturnMap([['config', $config]]);
+
+        $factory  = new AllowListParserFactory();
+        $instance = $factory($container);
+
+        $expected = [PsalmType::Null];
+        $filter   = new AllowList(['list' => []]);
+        $actual   = $instance->getTypes($filter, [PsalmType::String]);
+
+        self::assertSame($expected, $actual);
+    }
+
+    public function testInvokeConfiguresMaxLiteral(): void
+    {
+        $config    = $this->getConfig(['max-literals' => 0]);
         $container = $this->createStub(ContainerInterface::class);
         $container->method('get')
             ->willReturnMap([['config', $config]]);
@@ -58,4 +64,17 @@ public function testInvokeReturnsConfiguredInstance(): void
 
         self::assertSame($expected, $actual);
     }
+
+    private function getConfig(array $config): array
+    {
+        return [
+            'laminas-form-cli' => [
+                'array-shape' => [
+                    'filter' => [
+                        'allow-list' => $config,
+                    ],
+                ],
+            ],
+        ];
+    }
 }
diff --git a/test/ArrayShape/Filter/AllowListParserTest.php b/test/ArrayShape/Filter/AllowListParserTest.php
@@ -11,13 +11,14 @@
 use Laminas\Filter\AllowList;
 use Laminas\Filter\Boolean;
 use Laminas\Filter\FilterInterface;
+use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 
 /**
- * @covers \Kynx\Laminas\FormCli\ArrayShape\Filter\AllowListParser
  * @psalm-import-type ParsedArray from AbstractParsedType
  */
+#[CoversClass(AllowListParser::class)]
 final class AllowListParserTest extends TestCase
 {
     /**
@@ -36,10 +37,13 @@ public static function getTypeLiteralProvider(): array
         // phpcs:disable Generic.Files.LineLength.TooLong
         return [
             'invalid'         => [new Boolean(), [PsalmType::Int], [PsalmType::Int]],
-            'strict list'     => [new AllowList(['list' => ['foo', 123], 'strict' => true]), [PsalmType::Int, PsalmType::String, PsalmType::Null], [new Literal(["'foo'", 123]), PsalmType::Null]],
-            'lax list'        => [new AllowList(['list' => ['foo', 123], 'strict' => false]), [PsalmType::Int, PsalmType::String, PsalmType::Null], [new Literal(["'foo'", 123, "'123'"]), PsalmType::Null]],
-            'strict existing' => [new AllowList(['list' => ['foo', 123], 'strict' => true]), [PsalmType::Int], [new Literal([123])]],
-            'lax existing'    => [new AllowList(['list' => ['foo', 123], 'strict' => false]), [PsalmType::String], [new Literal(["'foo'", "'123'"])]],
+            'empty list'      => [new AllowList(['list' => []]), [PsalmType::Bool], [PsalmType::Bool]],
+            'strict list'     => [new AllowList(['list' => ['foo', 123], 'strict' => true]), [PsalmType::Int, PsalmType::String], [PsalmType::Null, new Literal(["'foo'", 123])]],
+            'strict not literal' => [new AllowList(['list' => [1.23], 'strict' => true]), [PsalmType::Float], [PsalmType::Float, PsalmType::Null]],
+            'lax list'        => [new AllowList(['list' => ['foo', 123], 'strict' => false]), [PsalmType::Int, PsalmType::String], [PsalmType::Null, new Literal(["'foo'", 123, "'123'"])]],
+            'lax not literal' => [new AllowList(['list' => [1.23], 'strict' => false]), [PsalmType::Float], [PsalmType::Float, PsalmType::Null]],
+            'strict existing' => [new AllowList(['list' => ['foo', 123], 'strict' => true]), [PsalmType::Int], [PsalmType::Null, new Literal([123])]],
+            'lax existing'    => [new AllowList(['list' => ['foo', 123], 'strict' => false]), [PsalmType::String], [PsalmType::Null, new Literal(["'foo'", "'123'"])]],
         ];
         // phpcs:enable
     }
@@ -50,7 +54,7 @@ public static function getTypeLiteralProvider(): array
     #[DataProvider('getTypeTypeProvider')]
     public function testGetTypesReturnsTypes(FilterInterface $filter, array $existing, array $expected): void
     {
-        $parser = new AllowListParser(1);
+        $parser = new AllowListParser(true, 1);
         $actual = $parser->getTypes($filter, $existing);
         self::assertSame($expected, $actual);
     }
@@ -66,4 +70,13 @@ public static function getTypeTypeProvider(): array
         ];
         // phpcs:enable
     }
+
+    public function testGetTypesDisallowsEmptyList(): void
+    {
+        $expected = [PsalmType::Null];
+        $parser = new AllowListParser(false);
+        $filter = new AllowList(['list' => []]);
+        $actual = $parser->getTypes($filter, [PsalmType::String]);
+        self::assertSame($expected, $actual);
+    }
 }