Merge pull request #28 from kynx/static-analysis-tests

More collection fixes; added static analysis tests
kynx · Feb 25, 2024 · eca4987 · eca4987
2 parents a0b4a21 + 8259d48
commit eca4987
Show file tree

Hide file tree

Showing 16 changed files with 208 additions and 37 deletions.
diff --git a/.laminas-ci/pre-run.sh b/.laminas-ci/pre-run.sh
@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+
+# Regenerate docblocks on test/StaticAnalysis files so psalm will pick up any breakages
+
+WORKDIR=$2
+JOB=$3
+COMMAND=$(echo "${JOB}" | jq -r '.command')
+if [[ ! ${COMMAND} =~ psalm ]];then
+    exit 0
+fi
+
+cd "$WORKDIR"
+vendor/bin/laminas --container test/container.php form:psalm-type test/StaticAnalysis
diff --git a/psalm.xml b/psalm.xml
@@ -26,6 +26,7 @@
         <UnusedClass>
             <errorLevel type="suppress">
                 <directory name="test/Locator/Asset"/>
+                <directory name="test/StaticAnalysis"/>
             </errorLevel>
         </UnusedClass>
     </issueHandlers>

diff --git a/src/Form/FormVisitor.php b/src/Form/FormVisitor.php
@@ -121,7 +121,9 @@ private function convertCollectionFilters(
                 assert($collectionFilter instanceof BaseInputFilter);
 
                 $childFilter = new CollectionInputFilter();
-                $childFilter->setInputFilter($collectionFilter);
+                $childFilter->setIsRequired($inputOrFilter->getIsRequired())
+                    ->setCount($inputOrFilter->getCount())
+                    ->setInputFilter($collectionFilter);
             } else {
                 $childFilter = $this->convertCollectionFilters($elementOrFieldset, $inputOrFilter);
             }
@@ -141,7 +143,7 @@ private function convertCollectionFilters(
             $count    = $required ? $elementOrFieldset->getCount() : 0;
 
             if ($target instanceof InputInterface) {
-                $inputOrFilter = CollectionInput::fromInput($target, $count, ! $required);
+                $inputOrFilter = CollectionInput::fromInput($target, $count);
             } else {
                 $inputOrFilter = new CollectionInputFilter();
                 $inputOrFilter->setIsRequired($required);

diff --git a/src/InputFilter/ArrayInputVisitor.php b/src/InputFilter/ArrayInputVisitor.php
@@ -20,7 +20,7 @@ public function __construct(private InputVisitor $inputVisitor)
 
     public function visit(InputInterface $input): ?Union
     {
-        if (! $input instanceof ArrayInput) {
+        if (! ($input instanceof ArrayInput || $input instanceof CollectionInput)) {
             return null;
         }
 

diff --git a/src/InputFilter/CollectionInput.php b/src/InputFilter/CollectionInput.php
@@ -27,25 +27,20 @@
  */
 final readonly class CollectionInput implements InputInterface, EmptyContextInterface
 {
-    private function __construct(private InputInterface $delegate, private int $count, private bool $possiblyUndefined)
+    private function __construct(private InputInterface $delegate, private int $count)
     {
     }
 
-    public static function fromInput(InputInterface $input, int $count, bool $possiblyUndefined): self
+    public static function fromInput(InputInterface $input, int $count): self
     {
-        return new self($input, $count, $possiblyUndefined);
+        return new self($input, $count);
     }
 
     public function getCount(): int
     {
         return $this->count;
     }
 
-    public function isPossiblyUndefined(): bool
-    {
-        return $this->possiblyUndefined;
-    }
-
     /**
      * @param bool $continueIfEmpty
      */

diff --git a/src/InputFilter/CollectionInputVisitor.php b/src/InputFilter/CollectionInputVisitor.php
@@ -28,6 +28,6 @@ public function visit(InputInterface $input): ?Union
             ? new TNonEmptyArray([Type::getArrayKey(), new Union($union->getAtomicTypes())])
             : new TArray([Type::getArrayKey(), new Union($union->getAtomicTypes())]);
 
-        return new Union([$array], ['possibly_undefined' => $input->isPossiblyUndefined()]);
+        return new Union([$array]);
     }
 }
diff --git a/src/InputFilter/InputFilterVisitor.php b/src/InputFilter/InputFilterVisitor.php
@@ -71,7 +71,7 @@ private function visitCollectionInputFilter(
             return new Union([new TNonEmptyArray([Type::getArrayKey(), $collection])]);
         }
 
-        return new Union([new TArray([Type::getArrayKey(), $collection])], ['possibly_undefined' => true]);
+        return new Union([new TArray([Type::getArrayKey(), $collection])]);
     }
 
     private function visitInput(InputInterface $input): Union

diff --git a/test/Form/FormCollectionSmokeTest.php b/test/Form/FormCollectionSmokeTest.php
@@ -136,7 +136,7 @@ public function testElementAsTargetElementValidates(): void
     {
         $expected = <<<EOT
         array{
-            foo?: array<array-key, non-empty-string>,
+            foo: array<array-key, non-empty-string>,
         }
         EOT;
         $data     = ['foo' => ['[email protected]']];

diff --git a/test/Form/FormVisitorTest.php b/test/Form/FormVisitorTest.php
@@ -72,7 +72,7 @@ public function testVisitCollectionWithFieldsetTargetElement(): void
                             ]),
                         ]),
                     ]),
-                ], ['possibly_undefined' => true]),
+                ]),
             ]),
         ]);
 
@@ -109,7 +109,7 @@ public function testVisitCollectionWithInputFilterProviderTargetElement(): void
                             ]),
                         ]),
                     ]),
-                ], ['possibly_undefined' => true]),
+                ]),
             ]),
         ]);
 
@@ -162,7 +162,7 @@ public function testVisitCollectionWithTextTargetElement(): void
                         Type::getArrayKey(),
                         new Union([new TString(), new TNull()]),
                     ]),
-                ], ['possibly_undefined' => true]),
+                ]),
             ]),
         ]);
 
@@ -171,6 +171,13 @@ public function testVisitCollectionWithTextTargetElement(): void
         $collection->setTargetElement(new Text());
         $form->add($collection);
 
+        $clone = clone $form;
+        $clone->setData([]);
+        $clone->isValid();
+        /** @var array $data */
+        $data = $clone->getData();
+        self::assertArrayHasKey('foo', $data);
+
         $actual = $this->visitor->visit($form, []);
         self::assertEquals($expected, $actual);
     }
@@ -189,11 +196,11 @@ public function testVisitNestedCollection(): void
                                         Type::getArrayKey(),
                                         new Union([new TString(), new TNull()]),
                                     ]),
-                                ], ['possibly_undefined' => true]),
+                                ]),
                             ]),
                         ]),
                     ]),
-                ], ['possibly_undefined' => true]),
+                ]),
             ]),
         ]);
 
@@ -254,7 +261,7 @@ public function testVisitCollectionWithImportTypes(): void
             new TKeyedArray([
                 'foo' => new Union([
                     new TArray([Type::getArrayKey(), new Union([$typeAlias])]),
-                ], ['possibly_undefined' => true]),
+                ]),
             ]),
         ]);
 

diff --git a/test/InputFilter/CollectionInputTest.php b/test/InputFilter/CollectionInputTest.php
@@ -18,9 +18,8 @@ final class CollectionInputTest extends TestCase
 {
     public function testFromInputSetsProperties(): void
     {
-        $actual = CollectionInput::fromInput(new Input(), 42, false);
+        $actual = CollectionInput::fromInput(new Input(), 42);
         self::assertSame(42, $actual->getCount());
-        self::assertSame(false, $actual->isPossiblyUndefined());
     }
 
     public function testFromInputDelegatesProperties(): void
@@ -34,7 +33,7 @@ public function testFromInputDelegatesProperties(): void
             ->setFilterChain((new FilterChain())->attach(new ToInt()))
             ->setValidatorChain((new ValidatorChain())->attach(new NotEmpty()));
 
-        $actual = CollectionInput::fromInput($input, 0, true);
+        $actual = CollectionInput::fromInput($input, 0);
 
         self::assertSame($input->getName(), $actual->getName());
         self::assertSame($input->isRequired(), $actual->isRequired());
@@ -47,7 +46,7 @@ public function testFromInputDelegatesProperties(): void
 
     public function testIsValidDelegates(): void
     {
-        $input = CollectionInput::fromInput((new Input())->setRequired(true), 0, true);
+        $input = CollectionInput::fromInput((new Input())->setRequired(true), 0);
         $input->setValue([]);
 
         $actual = $input->isValid();

diff --git a/test/InputFilter/CollectionInputVisitorFactoryTest.php b/test/InputFilter/CollectionInputVisitorFactoryTest.php
@@ -26,7 +26,7 @@ public function testInvokeReturnsConfiguredInstance(): void
 
         $factory  = new CollectionInputVisitorFactory();
         $instance = $factory($container);
-        $input    = CollectionInput::fromInput(new Input(), 0, true);
+        $input    = CollectionInput::fromInput(new Input(), 0);
 
         $union = $instance->visit($input);
         self::assertNotNull($union);

diff --git a/test/InputFilter/CollectionInputVisitorTest.php b/test/InputFilter/CollectionInputVisitorTest.php
@@ -38,7 +38,7 @@ public function testVisitReturnsNonEmptyArray(): void
         $expected = new Union([
             new TNonEmptyArray([Type::getArrayKey(), new Union([new TString(), new TNull()])]),
         ]);
-        $input    = CollectionInput::fromInput(new Input(), 42, false);
+        $input    = CollectionInput::fromInput(new Input(), 42);
 
         $actual = $this->visitor->visit($input);
         self::assertEquals($expected, $actual);
@@ -49,18 +49,9 @@ public function testVisitReturnsArray(): void
         $expected = new Union([
             new TArray([Type::getArrayKey(), new Union([new TString(), new TNull()])]),
         ]);
-        $input    = CollectionInput::fromInput(new Input(), 0, false);
+        $input    = CollectionInput::fromInput(new Input(), 0);
 
         $actual = $this->visitor->visit($input);
         self::assertEquals($expected, $actual);
     }
-
-    public function testVisitReturnsPossiblyUndefinedUnion(): void
-    {
-        $input = CollectionInput::fromInput(new Input(), 0, true);
-
-        $actual = $this->visitor->visit($input);
-        self::assertNotNull($actual);
-        self::assertTrue($actual->possibly_undefined);
-    }
 }
diff --git a/test/InputFilter/InputFilterVisitorTest.php b/test/InputFilter/InputFilterVisitorTest.php
@@ -59,7 +59,7 @@ public function testVisitReturnsUnion(): void
     #[DataProvider('collectionProvider')]
     public function testVisitReturnsCollectionUnion(bool $required, TArray $array): void
     {
-        $expected = new Union([$array], ['possibly_undefined' => ! $required]);
+        $expected = new Union([$array]);
 
         $collectionFilter = new InputFilter();
         $collectionFilter->add(new Input('foo'));
@@ -90,6 +90,37 @@ public static function collectionProvider(): array
         ];
     }
 
+    public function testVisitNestedCollectionReturnsDefinedUnion(): void
+    {
+        $expected              = new Union([
+            new TKeyedArray([
+                'foo' => new Union([
+                    new TArray([
+                        Type::getArrayKey(),
+                        new Union([
+                            new TKeyedArray([
+                                'bar' => new Union([new TString(), new TNull()]),
+                            ]),
+                        ]),
+                    ]),
+                ]),
+            ]),
+        ]);
+        $inputFilter           = new InputFilter();
+        $collectionInputFilter = new CollectionInputFilter();
+        $collectionFilter      = new InputFilter();
+        $collectionFilter->add(new Input('bar'));
+        $collectionInputFilter->setInputFilter($collectionFilter);
+        $inputFilter->add($collectionInputFilter, 'foo');
+
+        $clone = clone $inputFilter;
+        $clone->setData([]);
+        self::assertTrue($clone->isValid());
+
+        $actual = $this->visitor->visit($inputFilter, new ImportTypes([]));
+        self::assertEquals($expected, $actual);
+    }
+
     public function testVisitNestedInputFilterReturnsNestedUnion(): void
     {
         $expected = new Union([

diff --git a/test/StaticAnalysis/Album.php b/test/StaticAnalysis/Album.php
@@ -0,0 +1,49 @@
+<?php
+
+declare(strict_types=1);
+
+namespace KynxTest\Laminas\FormShape\StaticAnalysis;
+
+use Laminas\Form\Element\Hidden;
+use Laminas\Form\Element\Number;
+use Laminas\Form\Element\Select;
+use Laminas\Form\Element\Text;
+use Laminas\Form\Fieldset;
+use Laminas\InputFilter\InputFilterProviderInterface;
+
+/**
+ * @psalm-type TAlbumData = array{
+ *     id:     null|string,
+ *     title:  non-empty-string,
+ *     genre:  '1'|'2'|'3',
+ *     rating: numeric-string,
+ * }
+ */
+final class Album extends Fieldset implements InputFilterProviderInterface
+{
+    /**
+     * @param int|null|string $name
+     */
+    public function __construct($name = null, array $options = [])
+    {
+        parent::__construct($name, $options);
+
+        $this->add(new Hidden('id'));
+        $this->add(new Text('title'));
+        $this->add(new Select('genre', ['value_options' => [1 => 'Good', 2 => 'Bad', 3 => "Ugly"]]));
+        $this->add(new Number('rating', ['min' => 1, 'max' => 10]));
+    }
+
+    public function getInputFilterSpecification(): array
+    {
+        return [
+            'id'    => [
+                'required'    => true,
+                'allow_empty' => true,
+            ],
+            'title' => [
+                'required' => true,
+            ],
+        ];
+    }
+}
diff --git a/test/StaticAnalysis/Artist.php b/test/StaticAnalysis/Artist.php
@@ -0,0 +1,36 @@
+<?php
+
+declare(strict_types=1);
+
+namespace KynxTest\Laminas\FormShape\StaticAnalysis;
+
+use Laminas\Form\Element\Collection;
+use Laminas\Form\Element\Hidden;
+use Laminas\Form\Element\Text;
+use Laminas\Form\Form;
+
+/**
+ * @psalm-import-type TAlbumData from Album
+ * @psalm-type TArtistData = array{
+ *     id:     null|string,
+ *     name:   null|string,
+ *     albums: array<array-key, TAlbumData>,
+ * }
+ * @extends Form<TArtistData>
+ */
+final class Artist extends Form
+{
+    /**
+     * @param int|null|string $name
+     */
+    public function __construct($name = null, array $options = [])
+    {
+        parent::__construct($name, $options);
+
+        $this->add(new Hidden('id'));
+        $this->add(new Text('name'));
+        $collection = new Collection('albums');
+        $collection->setTargetElement(new Album());
+        $this->add($collection);
+    }
+}