From af0134ea2578ff6d0b25d2821f6fa41d04296fa0 Mon Sep 17 00:00:00 2001 From: George Steel Date: Sun, 19 Nov 2023 20:25:27 +0000 Subject: [PATCH] General type inference improvements The main goal here was to get inference when iterating over a form or fieldset, both for psalm and in yer IDE - this led to fixing a few more psalm issues here and there. There are a couple of additions to the baseline occurring due to improved inference, but it would have been unwise to fix these. Signed-off-by: George Steel --- psalm-baseline.xml | 21 ++++-------- src/Element/Collection.php | 7 ++-- src/Fieldset.php | 34 +++++++++++--------- src/FieldsetInterface.php | 1 + src/Form.php | 6 ++-- test/Element/CollectionTest.php | 7 ++-- test/StaticAnalysis/FormIterableTypes.php | 39 +++++++++++++++++++++++ 7 files changed, 75 insertions(+), 40 deletions(-) create mode 100644 test/StaticAnalysis/FormIterableTypes.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 0f0cf4388..a8254f306 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -88,7 +88,6 @@ object instanceof Traversable]]> is_array($object) object)]]> - is_object($elementOrFieldset) $object @@ -105,12 +104,9 @@ $object - + (string) $name - - - gettype($elementOrFieldset) - + bindValues @@ -153,7 +149,6 @@ - is_object($elementOrFieldset) is_object($object) @@ -170,21 +165,14 @@ $object - - gettype($elementOrFieldset) - bindValues - - - IteratorAggregate - - $childFieldset + $fieldset $filter $spec $validationGroup @@ -414,6 +402,9 @@ $products $shop + + get('phones')]]> + diff --git a/src/Element/Collection.php b/src/Element/Collection.php index fec4a2faf..ee572da8c 100644 --- a/src/Element/Collection.php +++ b/src/Element/Collection.php @@ -17,10 +17,9 @@ use function assert; use function class_exists; use function count; -use function gettype; +use function get_debug_type; use function is_array; use function is_int; -use function is_object; use function is_string; use function iterator_to_array; use function max; @@ -178,7 +177,7 @@ public function setObject($object) throw new Exception\InvalidArgumentException(sprintf( '%s expects an array or Traversable object argument; received "%s"', __METHOD__, - is_object($object) ? $object::class : gettype($object) + get_debug_type($object), )); } @@ -328,7 +327,7 @@ public function setTargetElement($elementOrFieldset) '%s requires that $elementOrFieldset be an object implementing %s; received "%s"', __METHOD__, __NAMESPACE__ . '\ElementInterface', - is_object($elementOrFieldset) ? $elementOrFieldset::class : gettype($elementOrFieldset) + get_debug_type($elementOrFieldset), )); } diff --git a/src/Fieldset.php b/src/Fieldset.php index 5be533e94..acb5dad42 100644 --- a/src/Fieldset.php +++ b/src/Fieldset.php @@ -15,11 +15,13 @@ use function array_key_exists; use function assert; -use function gettype; +use function get_debug_type; use function in_array; use function is_array; use function is_iterable; +use function is_numeric; use function is_object; +use function is_string; use function ltrim; use function sprintf; @@ -28,16 +30,16 @@ class Fieldset extends Element implements FieldsetInterface /** @var null|Factory */ protected $factory; - /** @var array */ + /** @var array */ protected $elements = []; - /** @var FieldsetInterface[] */ + /** @var array */ protected $fieldsets = []; /** @var array */ protected $messages = []; - /** @var PriorityList */ + /** @var PriorityList */ protected $iterator; /** @@ -74,7 +76,9 @@ class Fieldset extends Element implements FieldsetInterface */ public function __construct($name = null, array $options = []) { - $this->iterator = new PriorityList(); + /** @var PriorityList $list */ + $list = new PriorityList(); + $this->iterator = $list; $this->iterator->isLIFO(false); parent::__construct($name, $options); } @@ -154,12 +158,12 @@ public function add($elementOrFieldset, array $flags = []) '%s requires that $elementOrFieldset be an object implementing %s; received "%s"', __METHOD__, __NAMESPACE__ . '\ElementInterface', - is_object($elementOrFieldset) ? $elementOrFieldset::class : gettype($elementOrFieldset) + get_debug_type($elementOrFieldset), )); } $name = $elementOrFieldset->getName(); - if (array_key_exists('name', $flags) && $flags['name'] !== '') { + if (isset($flags['name']) && is_string($flags['name']) && $flags['name'] !== '') { $name = $flags['name']; // Rename the element or fieldset to the specified alias @@ -173,8 +177,8 @@ public function add($elementOrFieldset, array $flags = []) )); } $order = 0; - if (array_key_exists('priority', $flags)) { - $order = $flags['priority']; + if (isset($flags['priority']) && is_numeric($flags['priority'])) { + $order = (int) $flags['priority']; } $this->iterator->insert($name, $elementOrFieldset, $order); @@ -203,13 +207,14 @@ public function has(string $elementOrFieldset): bool */ public function get(string $elementOrFieldset): ElementInterface { - if (! $this->has($elementOrFieldset)) { + $element = $this->iterator->get($elementOrFieldset); + if (! $element) { throw new Exception\InvalidElementException(sprintf( 'No element by the name of [%s] found in form', $elementOrFieldset )); } - return $this->iterator->get($elementOrFieldset); + return $element; } /** @@ -301,10 +306,7 @@ public function getMessages(?string $elementName = null): array $messages = $this->messages; foreach ($this->iterator as $name => $element) { $messageSet = $element->getMessages(); - if ( - empty($messageSet) - || (! is_array($messageSet) && ! $messageSet instanceof Traversable) - ) { + if ($messageSet === []) { continue; } $messages[$name] = $messageSet; @@ -392,6 +394,8 @@ public function count(): int /** * IteratorAggregate: return internal iterator + * + * @return PriorityList */ public function getIterator(): PriorityList { diff --git a/src/FieldsetInterface.php b/src/FieldsetInterface.php index 0ee15f390..424849435 100644 --- a/src/FieldsetInterface.php +++ b/src/FieldsetInterface.php @@ -9,6 +9,7 @@ use Laminas\Hydrator\HydratorInterface; use Traversable; +/** @extends IteratorAggregate */ interface FieldsetInterface extends Countable, IteratorAggregate, diff --git a/src/Form.php b/src/Form.php index 9fbfc1f5c..e0b63aaf7 100644 --- a/src/Form.php +++ b/src/Form.php @@ -549,12 +549,12 @@ public function getValidationGroup(): ?array protected function prepareValidationGroup(Fieldset $formOrFieldset, array $data, array &$validationGroup): void { foreach ($validationGroup as $key => &$value) { - if (! $formOrFieldset->has((string) $key)) { + $fieldset = $formOrFieldset->iterator->get((string) $key); + + if (! $fieldset) { continue; } - $fieldset = $formOrFieldset->iterator->get((string) $key); - if ($fieldset instanceof Collection) { if (! isset($data[$key]) && $fieldset->getCount() === 0) { unset($validationGroup[$key]); diff --git a/test/Element/CollectionTest.php b/test/Element/CollectionTest.php index 0cf7893eb..bb05a700a 100644 --- a/test/Element/CollectionTest.php +++ b/test/Element/CollectionTest.php @@ -1153,10 +1153,11 @@ public function testNestedCollections(): void //test for correct extract and populate $index = 0; - foreach ($addresses as $addresses) { - self::assertEquals($data[$index]['street'], $addresses->get('street')->getValue()); + foreach ($addresses as $address) { + self::assertInstanceOf(FieldsetInterface::class, $address); + self::assertEquals($data[$index]['street'], $address->get('street')->getValue()); //assuming data has just 1 phone entry - foreach ($addresses->get('phones') as $phone) { + foreach ($address->get('phones') as $phone) { self::assertEquals($data[$index]['number'], $phone->get('number')->getValue()); } $index++; diff --git a/test/StaticAnalysis/FormIterableTypes.php b/test/StaticAnalysis/FormIterableTypes.php new file mode 100644 index 000000000..a08a6f6ae --- /dev/null +++ b/test/StaticAnalysis/FormIterableTypes.php @@ -0,0 +1,39 @@ + */ + public function elementList(FieldsetInterface $fieldset): array + { + return array_values(iterator_to_array($fieldset)); + } + + /** @return list */ + public function elementNames(FieldsetInterface $fieldset): array + { + return array_keys(iterator_to_array($fieldset)); + } +}