Clean up AllowList filter

- Removes inheritance - Private properties instead of protected - Native return types - Native parameter types - Options can no longer be Traversable Signed-off-by: George Steel <[email protected]>
laminas · Nov 6, 2023 · fab100b · fab100b
1 parent 5a9b126
commit fab100b
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 61 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -26,15 +26,6 @@
       <code>is_object($options)</code>
     </RedundantConditionGivenDocblockType>
   </file>
-  <file src="src/AllowList.php">
-    <PossiblyUnusedMethod>
-      <code>setList</code>
-      <code>setStrict</code>
-    </PossiblyUnusedMethod>
-    <RedundantCastGivenDocblockType>
-      <code>(bool) $strict</code>
-    </RedundantCastGivenDocblockType>
-  </file>
   <file src="src/Boolean.php">
     <ArgumentTypeCoercion>
       <code>$typeOrOptions</code>
@@ -977,17 +968,6 @@
     </PossiblyUnusedMethod>
   </file>
   <file src="test/AllowListTest.php">
-    <InvalidArgument>
-      <code>$strict</code>
-    </InvalidArgument>
-    <MixedArrayAccess>
-      <code>$expected</code>
-      <code>$value</code>
-    </MixedArrayAccess>
-    <MixedAssignment>
-      <code>$data</code>
-      <code>[$value, $expected]</code>
-    </MixedAssignment>
     <PossiblyUnusedMethod>
       <code>defaultTestProvider</code>
       <code>listTestProvider</code>

diff --git a/src/AllowList.php b/src/AllowList.php
@@ -5,77 +5,82 @@
 namespace Laminas\Filter;
 
 use Laminas\Stdlib\ArrayUtils;
-use Traversable;
 
+use function array_values;
 use function in_array;
 use function is_array;
 
 /**
  * @psalm-type Options = array{
  *     strict?: bool,
- *     list?: array,
- *     ...
+ *     list?: iterable<array-key, mixed>,
  * }
- * @extends AbstractFilter<Options>
  */
-final class AllowList extends AbstractFilter
+final class AllowList implements FilterInterface
 {
-    /** @var bool */
-    protected $strict = false;
+    private bool $strict = false;
+    /** @var list<mixed> */
+    private array $list = [];
 
-    /** @var array */
-    protected $list = [];
+    /**
+     * @param Options $options
+     */
+    public function __construct(array $options = [])
+    {
+        $this->setOptions($options);
+    }
 
     /**
-     * @param null|array|Traversable $options
+     * @param Options $options
+     * @return $this
      */
-    public function __construct($options = null)
+    public function setOptions(array $options = []): self
     {
-        if (null !== $options) {
-            $this->setOptions($options);
-        }
+        $strict = $options['strict'] ?? false;
+        $list   = $options['list'] ?? [];
+
+        $this->setStrict($strict);
+        $this->setList($list);
+
+        return $this;
     }
 
     /**
      * Determine whether the in_array() call should be "strict" or not. See in_array docs.
-     *
-     * @param bool $strict
      */
-    public function setStrict($strict = true): void
+    public function setStrict(bool $strict): void
     {
-        $this->strict = (bool) $strict;
+        $this->strict = $strict;
     }
 
     /**
      * Returns whether the in_array() call should be "strict" or not. See in_array docs.
-     *
-     * @return bool
      */
-    public function getStrict()
+    public function getStrict(): bool
     {
         return $this->strict;
     }
 
     /**
      * Set the list of items to allow
      *
-     * @param array|Traversable $list
+     * @param iterable<array-key, mixed> $list
      */
-    public function setList($list = []): void
+    public function setList(iterable $list = []): void
     {
         if (! is_array($list)) {
             $list = ArrayUtils::iteratorToArray($list);
         }
 
-        $this->list = $list;
+        $this->list = array_values($list);
     }
 
     /**
      * Get the list of items to allow
      *
-     * @return array
+     * @return list<mixed>
      */
-    public function getList()
+    public function getList(): array
     {
         return $this->list;
     }
@@ -91,6 +96,18 @@ public function getList()
      */
     public function filter(mixed $value): mixed
     {
-        return in_array($value, $this->getList(), $this->getStrict()) ? $value : null;
+        return in_array($value, $this->list, $this->strict) ? $value : null;
+    }
+
+    /**
+     * Will return $value if its present in the allow-list. If $value is rejected then it will return null.
+     *
+     * @template T
+     * @param T $value
+     * @return T|null
+     */
+    public function __invoke(mixed $value): mixed
+    {
+        return $this->filter($value);
     }
 }
diff --git a/test/AllowListTest.php b/test/AllowListTest.php
@@ -8,11 +8,13 @@
 use Laminas\Filter\FilterPluginManager;
 use Laminas\ServiceManager\ServiceManager;
 use Laminas\Stdlib\ArrayObject;
-use Laminas\Stdlib\Exception;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
+use TypeError;
 
+use function assert;
 use function gettype;
+use function is_array;
 use function sprintf;
 use function var_export;
 
@@ -45,14 +47,6 @@ public function testWithPluginManager(): void
         self::assertInstanceOf(AllowListFilter::class, $filter);
     }
 
-    public function testNullListShouldThrowException(): void
-    {
-        $this->expectException(Exception\InvalidArgumentException::class);
-        new AllowListFilter([
-            'list' => null,
-        ]);
-    }
-
     public function testTraversableConvertsToArray(): void
     {
         $array  = ['test', 1];
@@ -63,12 +57,13 @@ public function testTraversableConvertsToArray(): void
         self::assertSame($array, $filter->getList());
     }
 
-    public function testSetStrictShouldCastToBoolean(): void
+    public function testSetStrictShouldBeBoolean(): void
     {
-        $filter = new AllowListFilter([
+        $this->expectException(TypeError::class);
+        /** @psalm-suppress InvalidArgument */
+        new AllowListFilter([
             'strict' => 1,
         ]);
-        self::assertSame(true, $filter->getStrict());
     }
 
     #[DataProvider('defaultTestProvider')]
@@ -78,6 +73,10 @@ public function testDefault(mixed $value): void
         self::assertNull($filter->filter($value));
     }
 
+    /**
+     * @param list<mixed> $list
+     * @param array{0: mixed, 1: mixed} $testData
+     */
     #[DataProvider('listTestProvider')]
     public function testList(bool $strict, array $list, array $testData): void
     {
@@ -86,13 +85,14 @@ public function testList(bool $strict, array $list, array $testData): void
             'list'   => $list,
         ]);
         foreach ($testData as $data) {
+            assert(is_array($data));
             [$value, $expected] = $data;
             $message            = sprintf(
                 '%s (%s) is not filtered as %s; type = %s',
                 var_export($value, true),
                 gettype($value),
                 var_export($expected, true),
-                $strict
+                $strict ? 'strict' : 'non-strict',
             );
             self::assertSame($expected, $filter->filter($value), $message);
         }
@@ -110,7 +110,7 @@ public static function defaultTestProvider(): array
         ];
     }
 
-    /** @return list<array{0: bool, 1: array, 2: array}> */
+    /** @return list<array{0: bool, 1: list<mixed>, 2: list<array{0: mixed, 1: mixed}>}> */
     public static function listTestProvider(): array
     {
         return [
@@ -141,4 +141,25 @@ public static function listTestProvider(): array
             ],
         ];
     }
+
+    public function testStrictModeCanBeSetAtRuntime(): void
+    {
+        $filter = new AllowListFilter();
+
+        $filter->setStrict(true);
+        self::assertTrue($filter->getStrict());
+    }
+
+    public function testListCanBeSetAtRuntime(): void
+    {
+        $filter = new AllowListFilter();
+        $filter->setList(['foo', 'bar']);
+        self::assertSame(['foo', 'bar'], $filter->getList());
+    }
+
+    public function testFilterCanBeInvoked(): void
+    {
+        $filter = new AllowListFilter(['list' => ['foo']]);
+        self::assertSame('foo', $filter->__invoke('foo'));
+    }
 }