Merge pull request #29 from kynx/type-alias-refactor

Fix matching deeply-nested import types
kynx · Feb 26, 2024 · c82cc8d · c82cc8d
2 parents eca4987 + 1863bc7
commit c82cc8d
Show file tree

Hide file tree

Showing 9 changed files with 111 additions and 81 deletions.
diff --git a/README.md b/README.md
@@ -4,9 +4,6 @@
 
 Generate [Psalm] types for [Laminas forms]
 
-**This is a work in progress**. Until we hit a `1.x` release, the examples below are more to illustrate what _can_ be
-done, not how it _will_ work once stable.
-
 ## Installation
 
 Install this package as a development dependency using [Composer]:
@@ -18,18 +15,27 @@ composer require --dev kynx/laminas-form-shape
 ## Usage
 
 ```commandline
-vendor/bin/laminas form:psalm-type src/Forms/MyForm.php
+vendor/bin/laminas form:psalm-type src/Forms/Artist.php
 ```
 
-...outputs an [array shape] something like:
-
-```text
-array{
-    name:     non-empty-string,
-    age:      numeric-string,
-    gender?:  null|string,
-    can_code: '0'|'1',
-}
+...will add an [array shape] to your `Artist` form something like:
+
+```diff
+ use Laminas\Form\Element\Text;
+ use Laminas\Form\Form;
+
++/**
++ * @psalm-import-type TAlbumData from Album
++ * @psalm-type TArtistData = array{
++ *     id:     int|null,
++ *     name:   non-empty-string,
++ *     albums: array<array-key, TAlbumData>,
++ * }
++ * @extends Form<TArtistData>
++ */
+ final class Artist extends Form
+ {
+     /**
 ```
 
 To see a full list of options:

diff --git a/src/Form/FormVisitor.php b/src/Form/FormVisitor.php
@@ -162,32 +162,22 @@ private function convertCollectionFilters(
     /**
      * @param array<ImportType> $importTypes
      */
-    private function getImportTypes(FormInterface $form, array $importTypes): ImportTypes
+    private function getImportTypes(FieldsetInterface $fieldset, array $importTypes): ImportTypes
     {
-        return new ImportTypes($this->keyTypes($form, $importTypes));
-    }
-
-    /**
-     * @param array<ImportType> $importTypes
-     * @return array<ImportType|array>
-     */
-    private function keyTypes(FieldsetInterface $fieldset, array $importTypes): array
-    {
-        $keyed = [];
+        $children = [];
         foreach ($fieldset->getFieldsets() as $childFieldset) {
             $name = (string) $childFieldset->getName();
             if ($childFieldset instanceof Collection) {
                 $targetElement = $childFieldset->getTargetElement();
-                if ($targetElement instanceof FieldsetInterface && isset($importTypes[$targetElement::class])) {
-                    $keyed[$name] = $importTypes[$targetElement::class];
+                if ($targetElement instanceof FieldsetInterface) {
+                    $children[$name] = $this->getImportTypes($targetElement, $importTypes);
                 }
                 continue;
             }
 
-            $keyed[$name] = $importTypes[$childFieldset::class]
-                ?? $this->keyTypes($childFieldset, $importTypes);
+            $children[$name] = $this->getImportTypes($childFieldset, $importTypes);
         }
 
-        return $keyed;
+        return new ImportTypes($importTypes[$fieldset::class] ?? null, $children);
     }
 }
diff --git a/src/InputFilter/ImportTypes.php b/src/InputFilter/ImportTypes.php
@@ -7,20 +7,19 @@
 final readonly class ImportTypes
 {
     /**
-     * @param array<ImportType|array> $importTypes
+     * @param array<ImportTypes> $children
      */
-    public function __construct(private array $importTypes)
+    public function __construct(private ?ImportType $type = null, private array $children = [])
     {
     }
 
-    public function get(int|string $key): ImportType|ImportTypes
+    public function get(): ?ImportType
     {
-        /** @var ImportType|array<ImportType|array> $value */
-        $value = $this->importTypes[$key] ?? [];
-        if ($value instanceof ImportType) {
-            return $value;
-        }
+        return $this->type;
+    }
 
-        return new self($value);
+    public function getChildren(int|string $key): self
+    {
+        return $this->children[$key] ?? new self(null, []);
     }
 }
diff --git a/src/InputFilter/InputFilterVisitor.php b/src/InputFilter/InputFilterVisitor.php
@@ -27,7 +27,7 @@ public function __construct(private array $inputVisitors)
     {
     }
 
-    public function visit(InputFilterInterface $inputFilter, ImportType|ImportTypes $importTypes): Union
+    public function visit(InputFilterInterface $inputFilter, ImportTypes $importTypes): Union
     {
         if ($inputFilter instanceof CollectionInputFilter) {
             return $this->visitCollectionInputFilter($inputFilter, $importTypes);
@@ -41,9 +41,7 @@ public function visit(InputFilterInterface $inputFilter, ImportType|ImportTypes
                 continue;
             }
 
-            $childTypes           = $importTypes instanceof ImportTypes
-                ? $importTypes->get($childName)
-                : new ImportTypes([]);
+            $childTypes           = $importTypes->getChildren($childName);
             $elements[$childName] = $this->visit($child, $childTypes);
         }
 
@@ -54,17 +52,11 @@ public function visit(InputFilterInterface $inputFilter, ImportType|ImportTypes
         }
 
         $union = new Union([new TKeyedArray($elements)], $properties);
-        if ($importTypes instanceof ImportType) {
-            return $this->getTypeAliasUnion($union, $importTypes);
-        }
-
-        return $union;
+        return $this->getTypeAliasUnion($union, $importTypes);
     }
 
-    private function visitCollectionInputFilter(
-        CollectionInputFilter $inputFilter,
-        ImportType|ImportTypes $importTypes
-    ): Union {
+    private function visitCollectionInputFilter(CollectionInputFilter $inputFilter, ImportTypes $importTypes): Union
+    {
         $collection = $this->visit($inputFilter->getInputFilter(), $importTypes);
 
         if ($inputFilter->getIsRequired()) {
@@ -86,8 +78,13 @@ private function visitInput(InputInterface $input): Union
         throw InputVisitorException::noVisitorForInput($input);
     }
 
-    private function getTypeAliasUnion(Union $filterUnion, ImportType $importType): Union
+    private function getTypeAliasUnion(Union $filterUnion, ImportTypes $importTypes): Union
     {
+        $importType = $importTypes->get();
+        if ($importType === null) {
+            return $filterUnion;
+        }
+
         if ($filterUnion->equals($importType->union, false, false)) {
             return new Union([$importType->type], ['possibly_undefined' => $filterUnion->possibly_undefined]);
         }

diff --git a/src/InputFilterVisitorInterface.php b/src/InputFilterVisitorInterface.php
@@ -4,12 +4,11 @@
 
 namespace Kynx\Laminas\FormShape;
 
-use Kynx\Laminas\FormShape\InputFilter\ImportType;
 use Kynx\Laminas\FormShape\InputFilter\ImportTypes;
 use Laminas\InputFilter\InputFilterInterface;
 use Psalm\Type\Union;
 
 interface InputFilterVisitorInterface
 {
-    public function visit(InputFilterInterface $inputFilter, ImportType|ImportTypes $importTypes): Union;
+    public function visit(InputFilterInterface $inputFilter, ImportTypes $importTypes): Union;
 }
diff --git a/test/Form/FormElementSmokeTest.php b/test/Form/FormElementSmokeTest.php
@@ -70,7 +70,7 @@ public function testDefaultElements(string $element, array $tests, string $expec
         $container   = include __DIR__ . '/../container.php';
         $visitor     = $container->get(InputFilterVisitorInterface::class);
         $inputFilter = $form->getInputFilter();
-        $union       = $visitor->visit($inputFilter, new ImportTypes([]));
+        $union       = $visitor->visit($inputFilter, new ImportTypes());
 
         $decorator = new PrettyPrinter();
         /** @psalm-suppress PossiblyInvalidArgument */

diff --git a/test/InputFilter/ImportTypesTest.php b/test/InputFilter/ImportTypesTest.php
@@ -21,32 +21,36 @@ public function testGetReturnsImportType(): void
             new TTypeAlias(self::class, 'TFoo'),
             new Union([new TInt()])
         );
-        $importTypes = new ImportTypes(['foo' => $expected]);
+        $importTypes = new ImportTypes($expected);
 
-        $actual = $importTypes->get('foo');
+        $actual = $importTypes->get();
         self::assertSame($expected, $actual);
     }
 
-    public function testGetReturnsNestedTypes(): void
+    public function testGetChildrenReturnsNestedTypes(): void
     {
-        $expected    = new ImportType(
+        $expected    = new ImportTypes(new ImportType(
             new TTypeAlias(self::class, 'TFoo'),
             new Union([new TInt()])
-        );
-        $importTypes = new ImportTypes(['foo' => ['bar' => $expected]]);
-
-        $nestedTypes = $importTypes->get('foo');
+        ));
+        $importTypes = new ImportTypes(null, [
+            'foo' => new ImportTypes(null, [
+                'bar' => $expected,
+            ]),
+        ]);
+
+        $nestedTypes = $importTypes->getChildren('foo');
         self::assertInstanceOf(ImportTypes::class, $nestedTypes);
-        $actual = $nestedTypes->get('bar');
+        $actual = $nestedTypes->getChildren('bar');
         self::assertSame($expected, $actual);
     }
 
     public function testGetReturnsEmptyTypes(): void
     {
-        $expected    = new ImportTypes([]);
-        $importTypes = new ImportTypes([]);
+        $expected    = new ImportTypes();
+        $importTypes = new ImportTypes();
 
-        $actual = $importTypes->get('foo');
+        $actual = $importTypes->getChildren('foo');
         self::assertEquals($expected, $actual);
     }
 }
diff --git a/test/InputFilter/InputFilterVisitorFactoryTest.php b/test/InputFilter/InputFilterVisitorFactoryTest.php
@@ -44,7 +44,7 @@ public function testInvokeReturnsConfiguredInstance(): void
         $inputFilter = new InputFilter();
         $inputFilter->add(new Input('foo'));
 
-        $actual = $instance->visit($inputFilter, new ImportTypes([]));
+        $actual = $instance->visit($inputFilter, new ImportTypes());
         self::assertEquals($expected, $actual);
     }
 
@@ -65,7 +65,7 @@ public function testInvokeSortsInputVisitors(): void
         $filter   = new InputFilter();
         $filter->add(new ArrayInput(), 'foo');
 
-        $keyedArray = $instance->visit($filter, new ImportTypes([]))->getSingleAtomic();
+        $keyedArray = $instance->visit($filter, new ImportTypes())->getSingleAtomic();
         self::assertInstanceOf(TKeyedArray::class, $keyedArray);
         $property = $keyedArray->properties['foo'] ?? null;
         self::assertInstanceOf(Union::class, $property);