From dd4d4f77708b20ad4c3b8a7c5b276cef45a28e3d Mon Sep 17 00:00:00 2001 From: matt Date: Sat, 24 Feb 2024 22:20:22 +0000 Subject: [PATCH 1/5] Added form for testing static analysis --- psalm.xml | 1 + test/StaticAnalysis/Album.php | 27 +++++++++++++++++++++++++++ test/StaticAnalysis/Artist.php | 27 +++++++++++++++++++++++++++ 3 files changed, 55 insertions(+) create mode 100644 test/StaticAnalysis/Album.php create mode 100644 test/StaticAnalysis/Artist.php diff --git a/psalm.xml b/psalm.xml index 0995cdb..9e163d2 100644 --- a/psalm.xml +++ b/psalm.xml @@ -26,6 +26,7 @@ + diff --git a/test/StaticAnalysis/Album.php b/test/StaticAnalysis/Album.php new file mode 100644 index 0000000..e9cbb0e --- /dev/null +++ b/test/StaticAnalysis/Album.php @@ -0,0 +1,27 @@ +add(new Hidden('id')); + $this->add(new Text('title')); + $this->add(new Select('genre', ['value_options' => [1 => 'Punk', 2 => 'Shlock']])); + $this->add(new Number('chart_position', ['min' => 1, 'max' => 100])); + } +} diff --git a/test/StaticAnalysis/Artist.php b/test/StaticAnalysis/Artist.php new file mode 100644 index 0000000..4436d7a --- /dev/null +++ b/test/StaticAnalysis/Artist.php @@ -0,0 +1,27 @@ +add(new Hidden('id')); + $this->add(new Text('name')); + $collection = new Collection('albums'); + $collection->setTargetElement(new Album()); + $this->add($collection); + } +} From aeb79021ef35d2ca3dad16c1954216e808b44715 Mon Sep 17 00:00:00 2001 From: matt Date: Sat, 24 Feb 2024 22:50:04 +0000 Subject: [PATCH 2/5] Added types with `form:psalm-type` --- test/StaticAnalysis/Album.php | 28 +++++++++++++++++++++++++--- test/StaticAnalysis/Artist.php | 9 +++++++++ test/StaticAnalysis/test.php | 22 ++++++++++++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 test/StaticAnalysis/test.php diff --git a/test/StaticAnalysis/Album.php b/test/StaticAnalysis/Album.php index e9cbb0e..8d3e4a4 100644 --- a/test/StaticAnalysis/Album.php +++ b/test/StaticAnalysis/Album.php @@ -9,8 +9,17 @@ use Laminas\Form\Element\Select; use Laminas\Form\Element\Text; use Laminas\Form\Fieldset; +use Laminas\InputFilter\InputFilterProviderInterface; -final class Album extends Fieldset +/** + * @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 @@ -21,7 +30,20 @@ public function __construct($name = null, array $options = []) $this->add(new Hidden('id')); $this->add(new Text('title')); - $this->add(new Select('genre', ['value_options' => [1 => 'Punk', 2 => 'Shlock']])); - $this->add(new Number('chart_position', ['min' => 1, 'max' => 100])); + $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 index 4436d7a..0b58b89 100644 --- a/test/StaticAnalysis/Artist.php +++ b/test/StaticAnalysis/Artist.php @@ -9,6 +9,15 @@ 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, + * } + * @extends Form + */ final class Artist extends Form { /** diff --git a/test/StaticAnalysis/test.php b/test/StaticAnalysis/test.php new file mode 100644 index 0000000..a876c8e --- /dev/null +++ b/test/StaticAnalysis/test.php @@ -0,0 +1,22 @@ + 'Mr NoAlbums', +]; + +$artist = new Artist(); +$artist->setData($data); +assert($artist->isValid()); + +$formData = $artist->getData(); +assert(is_array($formData)); +assert($formData['id'] === null); +assert($formData['name'] !== null); +assert($formData['albums'] === null); + From 427737ac7cf197b99298953c94b5613087949679 Mon Sep 17 00:00:00 2001 From: matt Date: Sun, 25 Feb 2024 14:44:28 +0000 Subject: [PATCH 3/5] Added tests to prove that collections are not possibly_undefined --- test/InputFilter/InputFilterVisitorTest.php | 30 ++++++++++++++- test/StaticAnalysis/Artist.php | 6 +-- test/StaticAnalysis/test.php | 41 +++++++++++++++++---- 3 files changed, 65 insertions(+), 12 deletions(-) diff --git a/test/InputFilter/InputFilterVisitorTest.php b/test/InputFilter/InputFilterVisitorTest.php index bd370b8..4c6deb1 100644 --- 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,34 @@ 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/Artist.php b/test/StaticAnalysis/Artist.php index 0b58b89..eb8bb24 100644 --- a/test/StaticAnalysis/Artist.php +++ b/test/StaticAnalysis/Artist.php @@ -12,9 +12,9 @@ /** * @psalm-import-type TAlbumData from Album * @psalm-type TArtistData = array{ - * id: null|string, - * name: null|string, - * albums: array, + * id: null|string, + * name: null|string, + * albums: array, * } * @extends Form */ diff --git a/test/StaticAnalysis/test.php b/test/StaticAnalysis/test.php index a876c8e..31067aa 100644 --- a/test/StaticAnalysis/test.php +++ b/test/StaticAnalysis/test.php @@ -6,17 +6,42 @@ require 'vendor/autoload.php'; -$data = [ - 'name' => 'Mr NoAlbums', +$nigel = new Artist(); +$data = [ + 'name' => 'Nigel NoAlbums', ]; +$nigel->setData($data); +assert($nigel->isValid()); -$artist = new Artist(); -$artist->setData($data); -assert($artist->isValid()); - -$formData = $artist->getData(); +$formData = $nigel->getData(); assert(is_array($formData)); assert($formData['id'] === null); assert($formData['name'] !== null); -assert($formData['albums'] === null); +$albums = $formData['albums']; +assert(! isset($albums[0])); + +$wendy = new Artist(); +$data = [ + 'id' => '123', + 'name' => 'Wendy OneAlbum', + 'albums' => [ + [ + 'id' => '', + 'title' => 'Woe is Wendy', + 'genre' => '3', + 'rating' => '10', + ], + ], +]; +$wendy->setData($data); +$isValid = $wendy->isValid(); +assert($isValid); + +$formData = $wendy->getData(); +assert(is_array($formData)); +assert($formData['id'] !== null); + +$albums = $formData['albums']; +assert(isset($albums[0])); +assert($albums[0]['genre'] === '3'); From 61b33e13b21808b78e59e602513840971d1c83fb Mon Sep 17 00:00:00 2001 From: matt Date: Sun, 25 Feb 2024 15:15:41 +0000 Subject: [PATCH 4/5] Removed collection possibly_undefined stuff --- src/Form/FormVisitor.php | 6 ++++-- src/InputFilter/ArrayInputVisitor.php | 2 +- src/InputFilter/CollectionInput.php | 11 +++-------- src/InputFilter/CollectionInputVisitor.php | 2 +- src/InputFilter/InputFilterVisitor.php | 2 +- test/Form/FormCollectionSmokeTest.php | 2 +- test/Form/FormVisitorTest.php | 19 +++++++++++++------ test/InputFilter/CollectionInputTest.php | 7 +++---- .../CollectionInputVisitorFactoryTest.php | 2 +- .../CollectionInputVisitorTest.php | 13 ++----------- test/InputFilter/InputFilterVisitorTest.php | 17 ++++++++++------- 11 files changed, 40 insertions(+), 43 deletions(-) diff --git a/src/Form/FormVisitor.php b/src/Form/FormVisitor.php index 353f055..530bb2b 100644 --- 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 index f6a9b5c..c0bac49 100644 --- 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 index ef61c7d..ac447e9 100644 --- a/src/InputFilter/CollectionInput.php +++ b/src/InputFilter/CollectionInput.php @@ -27,13 +27,13 @@ */ 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 @@ -41,11 +41,6 @@ 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 index 0135627..773150e 100644 --- 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 index d806947..3082b1d 100644 --- 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 index ceb6ddf..c748daf 100644 --- a/test/Form/FormCollectionSmokeTest.php +++ b/test/Form/FormCollectionSmokeTest.php @@ -136,7 +136,7 @@ public function testElementAsTargetElementValidates(): void { $expected = <<, + foo: array, } EOT; $data = ['foo' => ['slarty@example.com']]; diff --git a/test/Form/FormVisitorTest.php b/test/Form/FormVisitorTest.php index 4f14936..33f490f 100644 --- 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 index 6e63275..4df9ca0 100644 --- 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 index 1dc9a61..0fcd24c 100644 --- 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 index e7a349a..584be82 100644 --- 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 index 4c6deb1..dea6a6b 100644 --- a/test/InputFilter/InputFilterVisitorTest.php +++ b/test/InputFilter/InputFilterVisitorTest.php @@ -92,20 +92,23 @@ public static function collectionProvider(): array public function testVisitNestedCollectionReturnsDefinedUnion(): void { - $expected = new Union([ + $expected = new Union([ new TKeyedArray([ 'foo' => new Union([ - new TArray([Type::getArrayKey(), new Union([ - new TKeyedArray([ - 'bar' => new Union([new TString(), new TNull()]), + new TArray([ + Type::getArrayKey(), + new Union([ + new TKeyedArray([ + 'bar' => new Union([new TString(), new TNull()]), + ]), ]), - ])]), + ]), ]), ]), ]); - $inputFilter = new InputFilter(); + $inputFilter = new InputFilter(); $collectionInputFilter = new CollectionInputFilter(); - $collectionFilter = new InputFilter(); + $collectionFilter = new InputFilter(); $collectionFilter->add(new Input('bar')); $collectionInputFilter->setInputFilter($collectionFilter); $inputFilter->add($collectionInputFilter, 'foo'); From 8259d48c2f954aea1eac4f83a88ad70b329f13cd Mon Sep 17 00:00:00 2001 From: matt Date: Sun, 25 Feb 2024 15:55:30 +0000 Subject: [PATCH 5/5] Add pre-run ci script for regenerating static analysis docblocks prior to running psalm --- .laminas-ci/pre-run.sh | 13 +++++++++++++ test/StaticAnalysis/Artist.php | 6 +++--- 2 files changed, 16 insertions(+), 3 deletions(-) create mode 100755 .laminas-ci/pre-run.sh diff --git a/.laminas-ci/pre-run.sh b/.laminas-ci/pre-run.sh new file mode 100755 index 0000000..99f96b2 --- /dev/null +++ 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/test/StaticAnalysis/Artist.php b/test/StaticAnalysis/Artist.php index eb8bb24..f85086d 100644 --- a/test/StaticAnalysis/Artist.php +++ b/test/StaticAnalysis/Artist.php @@ -12,9 +12,9 @@ /** * @psalm-import-type TAlbumData from Album * @psalm-type TArtistData = array{ - * id: null|string, - * name: null|string, - * albums: array, + * id: null|string, + * name: null|string, + * albums: array, * } * @extends Form */