Skip to content

Commit

Permalink
General type inference improvements
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@gsteel
gsteel committed Nov 19, 2023
1 parent 9382d98 commit af0134e
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 40 deletions.
21 changes: 6 additions & 15 deletions psalm-baseline.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@
<code><![CDATA[$this->object instanceof Traversable]]></code>
<code>is_array($object)</code>
<code><![CDATA[is_array($this->object)]]></code>
<code>is_object($elementOrFieldset)</code>
</DocblockTypeContradiction>
<ImplementedParamTypeMismatch>
<code>$object</code>
Expand All @@ -105,12 +104,9 @@
<PropertyNotSetInConstructor>
<code>$object</code>
</PropertyNotSetInConstructor>
<RedundantCast>
<RedundantCastGivenDocblockType>
<code>(string) $name</code>
</RedundantCast>
<RedundantConditionGivenDocblockType>
<code>gettype($elementOrFieldset)</code>
</RedundantConditionGivenDocblockType>
</RedundantCastGivenDocblockType>
<TooManyArguments>
<code>bindValues</code>
</TooManyArguments>
Expand Down Expand Up @@ -153,7 +149,6 @@
</file>
<file src="src/Fieldset.php">
<DocblockTypeContradiction>
<code>is_object($elementOrFieldset)</code>
<code>is_object($object)</code>
</DocblockTypeContradiction>
<LessSpecificReturnStatement>
Expand All @@ -170,21 +165,14 @@
<PossiblyInvalidArgument>
<code>$object</code>
</PossiblyInvalidArgument>
<RedundantConditionGivenDocblockType>
<code>gettype($elementOrFieldset)</code>
</RedundantConditionGivenDocblockType>
<TooManyArguments>
<code>bindValues</code>
</TooManyArguments>
</file>
<file src="src/FieldsetInterface.php">
<MissingTemplateParam>
<code>IteratorAggregate</code>
</MissingTemplateParam>
</file>
<file src="src/Form.php">
<ArgumentTypeCoercion>
<code>$childFieldset</code>
<code>$fieldset</code>
<code>$filter</code>
<code>$spec</code>
<code>$validationGroup</code>
Expand Down Expand Up @@ -414,6 +402,9 @@
<code>$products</code>
<code>$shop</code>
</PossiblyUndefinedVariable>
<RawObjectIteration>
<code><![CDATA[$address->get('phones')]]></code>
</RawObjectIteration>
</file>
<file src="test/Element/ColorTest.php">
<PossiblyInvalidMethodCall>
Expand Down
7 changes: 3 additions & 4 deletions src/Element/Collection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
));
}

Expand Down Expand Up @@ -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),
));
}

Expand Down
34 changes: 19 additions & 15 deletions src/Fieldset.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -28,16 +30,16 @@ class Fieldset extends Element implements FieldsetInterface
/** @var null|Factory */
protected $factory;

/** @var array */
/** @var array<string, ElementInterface> */
protected $elements = [];

/** @var FieldsetInterface[] */
/** @var array<string, FieldsetInterface> */
protected $fieldsets = [];

/** @var array */
protected $messages = [];

/** @var PriorityList */
/** @var PriorityList<string, ElementInterface> */
protected $iterator;

/**
Expand Down Expand Up @@ -74,7 +76,9 @@ class Fieldset extends Element implements FieldsetInterface
*/
public function __construct($name = null, array $options = [])
{
$this->iterator = new PriorityList();
/** @var PriorityList<string, ElementInterface> $list */
$list = new PriorityList();
$this->iterator = $list;
$this->iterator->isLIFO(false);
parent::__construct($name, $options);
}
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -392,6 +394,8 @@ public function count(): int

/**
* IteratorAggregate: return internal iterator
*
* @return PriorityList<string, ElementInterface>
*/
public function getIterator(): PriorityList
{
Expand Down
1 change: 1 addition & 0 deletions src/FieldsetInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Laminas\Hydrator\HydratorInterface;
use Traversable;

/** @extends IteratorAggregate<string, ElementInterface> */
interface FieldsetInterface extends
Countable,
IteratorAggregate,
Expand Down
6 changes: 3 additions & 3 deletions src/Form.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
7 changes: 4 additions & 3 deletions test/Element/CollectionTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
Expand Down
39 changes: 39 additions & 0 deletions test/StaticAnalysis/FormIterableTypes.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

declare(strict_types=1);

namespace LaminasTest\Form\StaticAnalysis;

use Exception;
use Laminas\Form\ElementInterface;
use Laminas\Form\FieldsetInterface;
use Laminas\Form\Form;

use function array_keys;
use function array_values;
use function iterator_to_array;

/** @psalm-suppress UnusedClass, UnusedMethod */
final class FormIterableTypes
{
public function firstElement(Form $form): ElementInterface
{
foreach ($form as $element) {
return $element;
}

throw new Exception('Form is empty');
}

/** @return list<ElementInterface> */
public function elementList(FieldsetInterface $fieldset): array
{
return array_values(iterator_to_array($fieldset));
}

/** @return list<string> */
public function elementNames(FieldsetInterface $fieldset): array
{
return array_keys(iterator_to_array($fieldset));
}
}

0 comments on commit af0134e

Please sign in to comment.