From 20d25744702da0986455231c6cf47734a1bb1f46 Mon Sep 17 00:00:00 2001 From: matt Date: Mon, 26 Feb 2024 09:27:58 +0000 Subject: [PATCH 1/2] Fix matching deeply-nested import types --- src/Form/FormVisitor.php | 22 ++----- src/InputFilter/ImportTypes.php | 17 +++--- src/InputFilter/InputFilterVisitor.php | 25 ++++---- src/InputFilterVisitorInterface.php | 3 +- test/Form/FormElementSmokeTest.php | 2 +- test/InputFilter/ImportTypesTest.php | 28 +++++---- .../InputFilterVisitorFactoryTest.php | 4 +- test/InputFilter/InputFilterVisitorTest.php | 59 +++++++++++++++---- 8 files changed, 92 insertions(+), 68 deletions(-) diff --git a/src/Form/FormVisitor.php b/src/Form/FormVisitor.php index 530bb2b..b9cc25e 100644 --- a/src/Form/FormVisitor.php +++ b/src/Form/FormVisitor.php @@ -162,32 +162,22 @@ private function convertCollectionFilters( /** * @param array $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 $importTypes - * @return 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 index a661aef..29de3c6 100644 --- a/src/InputFilter/ImportTypes.php +++ b/src/InputFilter/ImportTypes.php @@ -7,20 +7,19 @@ final readonly class ImportTypes { /** - * @param array $importTypes + * @param array $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 $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 index 3082b1d..64b1c64 100644 --- 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 index 1eff298..66f95f2 100644 --- 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 index e38cb63..550adf5 100644 --- 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 index c085743..0721b8f 100644 --- 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 index 8d75f9d..c0bc97f 100644 --- 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); diff --git a/test/InputFilter/InputFilterVisitorTest.php b/test/InputFilter/InputFilterVisitorTest.php index dea6a6b..a92eb6a 100644 --- a/test/InputFilter/InputFilterVisitorTest.php +++ b/test/InputFilter/InputFilterVisitorTest.php @@ -52,7 +52,7 @@ public function testVisitReturnsUnion(): void $inputFilter->add(new Input('foo')); $inputFilter->add(new Input('bar')); - $actual = $this->visitor->visit($inputFilter, new ImportTypes([])); + $actual = $this->visitor->visit($inputFilter, new ImportTypes()); self::assertEquals($expected, $actual); } @@ -67,7 +67,7 @@ public function testVisitReturnsCollectionUnion(bool $required, TArray $array): $inputFilter->setIsRequired($required); $inputFilter->setInputFilter($collectionFilter); - $actual = $this->visitor->visit($inputFilter, new ImportTypes([])); + $actual = $this->visitor->visit($inputFilter, new ImportTypes()); self::assertEquals($expected, $actual); } @@ -117,7 +117,7 @@ public function testVisitNestedCollectionReturnsDefinedUnion(): void $clone->setData([]); self::assertTrue($clone->isValid()); - $actual = $this->visitor->visit($inputFilter, new ImportTypes([])); + $actual = $this->visitor->visit($inputFilter, new ImportTypes()); self::assertEquals($expected, $actual); } @@ -140,7 +140,7 @@ public function testVisitNestedInputFilterReturnsNestedUnion(): void $inputFilter = new InputFilter(); $inputFilter->add($childFilter, 'foo'); - $actual = $this->visitor->visit($inputFilter, new ImportTypes([])); + $actual = $this->visitor->visit($inputFilter, new ImportTypes()); self::assertEquals($expected, $actual); } @@ -152,7 +152,7 @@ public function testVisitFilterWithNoInputsReturnsMixedArray(): void $inputFilter = new InputFilter(); - $actual = $this->visitor->visit($inputFilter, new ImportTypes([])); + $actual = $this->visitor->visit($inputFilter, new ImportTypes()); self::assertEquals($expected, $actual); } @@ -169,7 +169,7 @@ public function testVisitReturnsPossiblyUndefinedUnion(): void $inputFilter = new OptionalInputFilter(); $inputFilter->add($input); - $actual = $this->visitor->visit($inputFilter, new ImportTypes([])); + $actual = $this->visitor->visit($inputFilter, new ImportTypes()); self::assertEquals($expected, $actual); } @@ -186,7 +186,7 @@ public function testVisitReturnsPossiblyUndefinedUnionWhenAllElementsUndefined() $inputFilter->add((new Input('foo'))->setRequired(false)); $inputFilter->add((new Input('bar'))->setRequired(false)); - $actual = $this->visitor->visit($inputFilter, new ImportTypes([])); + $actual = $this->visitor->visit($inputFilter, new ImportTypes()); self::assertEquals($expected, $actual); } @@ -203,7 +203,7 @@ public function testVisitReturnsRequiredUnionWhenOneElementRequired(): void $inputFilter->add((new Input('foo'))->setRequired(false)); $inputFilter->add(new Input('bar')); - $actual = $this->visitor->visit($inputFilter, new ImportTypes([])); + $actual = $this->visitor->visit($inputFilter, new ImportTypes()); self::assertEquals($expected, $actual); } @@ -220,7 +220,7 @@ public function testVisitReturnsMatchingImportedType(): void $inputFilter->add(new Input('foo')); $importType = new ImportType(new TTypeAlias('self', 'TImportType'), $importUnion); - $actual = $this->visitor->visit($inputFilter, $importType); + $actual = $this->visitor->visit($inputFilter, new ImportTypes($importType)); self::assertEquals($expected, $actual); } @@ -243,7 +243,7 @@ public function testVisitReturnsCalculatedTypeForNonMatchingImportType(): void $inputFilter->add(new Input('bar')); $importType = new ImportType(new TTypeAlias('self', 'TImportType'), $importUnion); - $actual = $this->visitor->visit($inputFilter, $importType); + $actual = $this->visitor->visit($inputFilter, new ImportTypes($importType)); self::assertEquals($expected, $actual); } @@ -266,7 +266,42 @@ public function testVisitReturnsNestedImportType(): void $inputFilter->add($childFilter, 'foo'); $importType = new ImportType(new TTypeAlias('self', 'TImportType'), $importUnion); - $actual = $this->visitor->visit($inputFilter, new ImportTypes(['foo' => $importType])); + $actual = $this->visitor->visit($inputFilter, new ImportTypes(null, ['foo' => new ImportTypes($importType)])); + self::assertEquals($expected, $actual); + } + + public function testVisitReturnsDeeplyNestedImportType(): void + { + $expected = new Union([ + new TKeyedArray([ + 'foo' => new Union([new TTypeAlias('self', 'TImportType')]), + ]), + ]); + $importUnion = new Union([ + new TKeyedArray([ + 'bar' => new Union([new TTypeAlias('self', 'TNestedImportType')]), + ]), + ]); + $nestedImportUnion = new Union([ + new TKeyedArray([ + 'baz' => new Union([new TString(), new TNull()]), + ]), + ]); + + $inputFilter = new InputFilter(); + $childFilter = new InputFilter(); + $nestedFilter = new InputFilter(); + $nestedFilter->add(new Input('baz')); + $childFilter->add($nestedFilter, 'bar'); + $inputFilter->add($childFilter, 'foo'); + $nestedType = new ImportType(new TTypeAlias('self', 'TNestedImportType'), $nestedImportUnion); + $importType = new ImportType(new TTypeAlias('self', 'TImportType'), $importUnion); + + $actual = $this->visitor->visit($inputFilter, new ImportTypes(null, [ + 'foo' => new ImportTypes($importType, [ + 'bar' => new ImportTypes($nestedType), + ]), + ])); self::assertEquals($expected, $actual); } @@ -280,6 +315,6 @@ public function testVisitNoValidInputVisitorThrowsException(): void self::expectException(InputVisitorException::class); self::expectExceptionMessage($expected); - $visitor->visit($inputFilter, new ImportTypes([])); + $visitor->visit($inputFilter, new ImportTypes()); } } From 1863bc70ac9b883876b02673d7125b47461453cd Mon Sep 17 00:00:00 2001 From: matt Date: Mon, 26 Feb 2024 09:39:38 +0000 Subject: [PATCH 2/2] Update usage to show diff of changed form --- README.md | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 7e94d6b..e097d51 100644 --- 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, ++ * } ++ * @extends Form ++ */ + final class Artist extends Form + { + /** ``` To see a full list of options: