From 92ce6298d8c2a505d9e918e308a53fdcdc126366 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Tue, 29 Sep 2026 16:09:17 +1000 Subject: [PATCH 1/3] Bring ResultSet down to twenty baselined findings Narrows $dataSource from Iterator|IteratorAggregate|ResultInterface|null to ?Iterator. ResultInterface already extends Iterator, and initialize() already unwrapped aggregates, so the union was never accurate. Both phpstan-ignore suppressions go with it. initialize() now walks a chain of IteratorAggregate rather than calling getIterator() once, since that method returns Traversable and may hand back another aggregate. A chain that never bottoms out in an Iterator is rejected instead of stored. current(), next(), rewind() and valid() reach the data source through an accessor that reports an uninitialised result set rather than calling a method on null. The three capability interfaces now extend ResultSetInterface. Every setter returned ResultSetInterface&XInterface only because they did not, and self cannot appear in an intersection type, so the return types are now plain self. That retires prefer-self-return-type, which emitted source that does not parse and made mago lint --fix unsafe against src. TemporaryResultSet extends the abstract base rather than ResultSet: the adapter only ever asks for ResultSetInterface. With nothing left subclassing them, the four concrete result sets and both exceptions are final, and EventFeatureTest doubles the interface it actually needs. Five defects fall out of the same fact, that a row need not be an array. The fetch mode is caller selectable across thirteen modes, so a row can be a string keyed array, an integer keyed array, an object, true under FETCH_BOUND, or false once the results are exhausted: - initialize() measured the first element of the array rather than the array, so an array of objects raised an uncaught TypeError - HydratingResultSet::toArray() handed non-objects to the hydrator; the throws tag promised an exception that was never raised - ResultSet::current() and RowPrototypeResultSet::current() declared return types that excluded objects the parent can return - count() returned null while count($resultSet) returned zero, against the Countable contract - getObjectPrototype() declared null but delegates to a method that cannot return it Two tests asserted the old behaviour and are corrected: one mocked an IteratorAggregate returning itself and expected it to be stored, the other expected null rather than zero from an uncountable data source. Of the twenty that remain, nine are the mixed family that follows from a row being mixed, five are complexity metrics, three are deferred to RowGateway with RowPrototypeInterface, two are array shapes no truthful annotation can narrow, and one is a Mago stub that treats DOMNodeList and its kin as Traversable but not Iterator. --- analyzer-baseline.toml | 266 ++---------------- lint-baseline.toml | 86 +----- src/ResultSet/AbstractResultSet.php | 134 ++++++--- .../ArrayObjectResultSetInterface.php | 6 +- src/ResultSet/ArrayResultSet.php | 2 +- .../Exception/InvalidArgumentException.php | 19 +- src/ResultSet/Exception/RuntimeException.php | 18 +- src/ResultSet/HydratingResultSet.php | 30 +- src/ResultSet/HydratingResultSetInterface.php | 6 +- src/ResultSet/ResultSet.php | 22 +- src/ResultSet/ResultSetInterface.php | 11 +- src/ResultSet/RowPrototypeInterface.php | 4 +- src/ResultSet/RowPrototypeResultSet.php | 6 +- .../RowPrototypeResultSetInterface.php | 6 +- test/unit/ResultSet/AbstractResultSetTest.php | 17 +- .../ResultSet/ResultSetIntegrationTest.php | 23 +- .../TableGateway/Feature/EventFeatureTest.php | 4 +- test/unit/TestAsset/TemporaryResultSet.php | 23 +- 18 files changed, 282 insertions(+), 401 deletions(-) diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index e17bd353..5c7a8c74 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -540,6 +540,18 @@ code = "incompatible-return-type" message = 'Return type `mixed` of `PhpDb\Adapter\Driver\Pdo\Result::next()` is incompatible with parent return type `void` of `Iterator::next()`' count = 1 +[[issues]] +file = "src/Adapter/Driver/Pdo/Result.php" +code = "less-specific-nested-argument-type" +message = 'Argument type mismatch for argument #1 of `PhpDb\ResultSet\AbstractResultSet::initialize`: expected `iterable`, but provided type `$this(PhpDb\Adapter\Driver\Pdo\Result)` is less specific.' +count = 1 + +[[issues]] +file = "src/Adapter/Driver/Pdo/Result.php" +code = "less-specific-nested-argument-type" +message = 'Argument type mismatch for argument #1 of `PhpDb\ResultSet\ResultSetInterface::initialize`: expected `iterable`, but provided type `$this(PhpDb\Adapter\Driver\Pdo\Result)` is less specific.' +count = 1 + [[issues]] file = "src/Adapter/Driver/Pdo/Result.php" code = "missing-constant-type" @@ -1500,12 +1512,6 @@ code = "missing-constant-type" message = "Class constant `DEFAULT_SCHEMA` is missing a type hint." count = 1 -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "imprecise-type" -message = "Type `array` in property `$buffer` is imprecise, equivalent to `array`." -count = 1 - [[issues]] file = "src/ResultSet/AbstractResultSet.php" code = "imprecise-type" @@ -1514,20 +1520,8 @@ count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" -code = "imprecise-type" -message = "Type `iterable` in parameter `$dataSource` is imprecise, equivalent to `iterable`." -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "incompatible-return-type" -message = 'Return type `int|null` of `PhpDb\ResultSet\AbstractResultSet::count()` is incompatible with parent return type `int` of `Countable::count()`' -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "invalid-property-assignment-value" -message = 'Invalid type for property `$dataSource`: expected `Iterator|IteratorAggregate|PhpDb\Adapter\Driver\ResultInterface|null`, but got `iterable`.' +code = "invalid-return-statement" +message = 'Invalid return type for function `PhpDb\ResultSet\AbstractResultSet::resolveIterator`: expected `Iterator`, but found `DOMNodeList|DatePeriod|Ds\Collection|Generator|Iterator|PDOStatement`.' count = 1 [[issues]] @@ -1536,18 +1530,6 @@ code = "invalid-type-cast" message = "Casting `mixed` to `array`." count = 1 -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "missing-api-or-internal" -message = 'Abstract class `PhpDb\ResultSet\AbstractResultSet` is missing an `@api` or `@internal` annotation.' -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "mixed-argument" -message = "Invalid argument type for argument #1 of `count`: expected `Countable|array`, but found `mixed`." -count = 1 - [[issues]] file = "src/ResultSet/AbstractResultSet.php" code = "mixed-assignment" @@ -1556,110 +1538,20 @@ count = 3 [[issues]] file = "src/ResultSet/AbstractResultSet.php" -code = "mixed-return-statement" -message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::current`. Saw type `mixed`.' +code = "mixed-property-type-coercion" +message = "A value with a less specific type `mixed` is being assigned to property `$fieldCount` (int|null)." count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" code = "mixed-return-statement" -message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::current`. Saw type `nonnull`.' +message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::current`. Saw type `mixed`.' count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" code = "mixed-return-statement" -message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::valid`. Saw type `mixed`.' -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "non-existent-method" -message = "Method `current` does not exist on type `IteratorAggregate`." -count = 3 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "non-existent-method" -message = "Method `key` does not exist on type `IteratorAggregate`." -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "non-existent-method" -message = "Method `next` does not exist on type `IteratorAggregate`." -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "non-existent-method" -message = "Method `rewind` does not exist on type `IteratorAggregate`." -count = 3 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "non-existent-method" -message = "Method `valid` does not exist on type `IteratorAggregate`." -count = 2 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "possible-method-access-on-null" -message = "Attempting to call a method on `null`." -count = 6 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "possibly-invalid-argument" -message = 'Possible argument type mismatch for argument #1 of `count`: expected `Countable|array`, but possibly received `Iterator|IteratorAggregate|PhpDb\Adapter\Driver\ResultInterface`.' -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "property-type-coercion" -message = 'A value of a less specific type `Traversable` is being assigned to property `$dataSource` (Iterator|IteratorAggregate|PhpDb\Adapter\Driver\ResultInterface|null).' -count = 1 - -[[issues]] -file = "src/ResultSet/ArrayObjectResultSetInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\ResultSet\ArrayObjectResultSetInterface` is missing an `@api` or `@internal` annotation.' -count = 1 - -[[issues]] -file = "src/ResultSet/ArrayResultSet.php" -code = "class-must-be-final" -message = 'Class `PhpDb\ResultSet\ArrayResultSet` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/ResultSet/ArrayResultSet.php" -code = "imprecise-type" -message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/ResultSet/Exception/InvalidArgumentException.php" -code = "class-must-be-final" -message = 'Class `PhpDb\ResultSet\Exception\InvalidArgumentException` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/ResultSet/Exception/RuntimeException.php" -code = "class-must-be-final" -message = 'Class `PhpDb\ResultSet\Exception\RuntimeException` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "class-must-be-final" -message = 'Class `PhpDb\ResultSet\HydratingResultSet` should be declared `final`.' -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "imprecise-type" -message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." +message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::current`. Saw type `nonnull`.' count = 1 [[issues]] @@ -1674,102 +1566,12 @@ code = "mixed-return-statement" message = 'Could not infer a precise return type for function `PhpDb\ResultSet\HydratingResultSet::current`. Saw type `nonnull`.' count = 1 -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "non-existent-method" -message = "Method `current` does not exist on type `IteratorAggregate`." -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "overly-wide-return-type" -message = "Declared return type `null|object` for `getobjectprototype` has unused branches: `null`." -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "possible-method-access-on-null" -message = "Attempting to call a method on `null`." -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "possibly-null-argument" -message = 'Argument #1 of method `Laminas\Hydrator\ExtractionInterface::extract` is possibly `null`, but parameter type `object` does not accept it.' -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSetInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\ResultSet\HydratingResultSetInterface` is missing an `@api` or `@internal` annotation.' -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "class-must-be-final" -message = 'Class `PhpDb\ResultSet\ResultSet` should be declared `final`.' -count = 1 - [[issues]] file = "src/ResultSet/ResultSet.php" code = "imprecise-type" message = "Type `array` in return type of `current` is imprecise, equivalent to `array`." count = 1 -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "imprecise-type" -message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "invalid-return-statement" -message = 'Invalid return type for function `PhpDb\ResultSet\ResultSet::getReturnType`: expected `enum(PhpDb\ResultSet\ResultSetReturnType)`, but found `enum(PhpDb\ResultSet\ResultSetReturnType)|string`.' -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "less-specific-return-statement" -message = 'Returned type `array|null|object` is less specific than the declared return type `ArrayObject|array|null` for function `PhpDb\ResultSet\ResultSet::current`.' -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "missing-constant-type" -message = "Class constant `TYPE_ARRAYOBJECT` is missing a type hint." -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "missing-constant-type" -message = "Class constant `TYPE_ARRAY` is missing a type hint." -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSetInterface.php" -code = "imprecise-type" -message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSetInterface.php" -code = "imprecise-type" -message = "Type `iterable` in parameter `$dataSource` is imprecise, equivalent to `iterable`." -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSetInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\ResultSet\ResultSetInterface` is missing an `@api` or `@internal` annotation.' -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSetInterface.php" -code = "missing-template-parameter" -message = "Too few template arguments for `Iterator`: expected at least 2, but found 0." -count = 1 - [[issues]] file = "src/ResultSet/RowPrototypeInterface.php" code = "imprecise-type" @@ -1782,42 +1584,12 @@ code = "imprecise-type" message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." count = 1 -[[issues]] -file = "src/ResultSet/RowPrototypeInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\ResultSet\RowPrototypeInterface` is missing an `@api` or `@internal` annotation.' -count = 1 - -[[issues]] -file = "src/ResultSet/RowPrototypeResultSet.php" -code = "class-must-be-final" -message = 'Class `PhpDb\ResultSet\RowPrototypeResultSet` should be declared `final`.' -count = 1 - [[issues]] file = "src/ResultSet/RowPrototypeResultSet.php" code = "imprecise-type" message = "Type `array` in return type of `current` is imprecise, equivalent to `array`." count = 1 -[[issues]] -file = "src/ResultSet/RowPrototypeResultSet.php" -code = "imprecise-type" -message = "Type `array` in return type of `toArray` is imprecise, equivalent to `array`." -count = 1 - -[[issues]] -file = "src/ResultSet/RowPrototypeResultSet.php" -code = "less-specific-return-statement" -message = 'Returned type `null|object` is less specific than the declared return type `PhpDb\ResultSet\RowPrototypeInterface|array|null` for function `PhpDb\ResultSet\RowPrototypeResultSet::current`.' -count = 1 - -[[issues]] -file = "src/ResultSet/RowPrototypeResultSetInterface.php" -code = "missing-api-or-internal" -message = 'Interface `PhpDb\ResultSet\RowPrototypeResultSetInterface` is missing an `@api` or `@internal` annotation.' -count = 1 - [[issues]] file = "src/RowGateway/AbstractRowGateway.php" code = "imprecise-type" @@ -5259,7 +5031,7 @@ count = 1 [[issues]] file = "src/TableGateway/AbstractTableGateway.php" code = "mixed-argument" -message = 'Invalid argument type for argument #1 of `PhpDb\ResultSet\ResultSetInterface::initialize`: expected `iterable`, but found `mixed`.' +message = 'Invalid argument type for argument #1 of `PhpDb\ResultSet\ResultSetInterface::initialize`: expected `iterable`, but found `mixed`.' count = 1 [[issues]] diff --git a/lint-baseline.toml b/lint-baseline.toml index a167d2d1..de4db269 100644 --- a/lint-baseline.toml +++ b/lint-baseline.toml @@ -260,92 +260,14 @@ count = 2 [[issues]] file = "src/ResultSet/AbstractResultSet.php" -code = "no-else-clause" -message = "Avoid `else` clauses." -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "no-else-clause" -message = "Avoid `elseif` clauses." -count = 3 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "no-isset" -message = "Use of the `isset` construct." -count = 2 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "no-redundant-else" -message = "The `if` branch always terminates; the trailing branches can be extracted." +code = "kan-defect" +message = "Class has a high kan defect score." count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" -code = "yoda-conditions" -message = "Use Yoda condition style for safer comparisons" -count = 5 - -[[issues]] -file = "src/ResultSet/ArrayObjectResultSetInterface.php" -code = "prefer-self-return-type" -message = "Return type `ArrayObjectResultSetInterface` refers to the enclosing class; use `self` instead." -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "halstead" -message = "Method has a high halstead difficulty" -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "no-else-clause" -message = "Avoid `elseif` clauses." -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "no-isset" -message = "Use of the `isset` construct." -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "yoda-conditions" -message = "Use Yoda condition style for safer comparisons" -count = 1 - -[[issues]] -file = "src/ResultSet/HydratingResultSetInterface.php" -code = "prefer-self-return-type" -message = "Return type `HydratingResultSetInterface` refers to the enclosing class; use `self` instead." -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSet.php" -code = "yoda-conditions" -message = "Use Yoda condition style for safer comparisons" -count = 1 - -[[issues]] -file = "src/ResultSet/ResultSetInterface.php" -code = "prefer-self-return-type" -message = "Return type `ResultSetInterface` refers to the enclosing class; use `self` instead." -count = 1 - -[[issues]] -file = "src/ResultSet/RowPrototypeInterface.php" -code = "prefer-self-return-type" -message = "Return type `RowPrototypeInterface` refers to the enclosing class; use `self` instead." -count = 1 - -[[issues]] -file = "src/ResultSet/RowPrototypeResultSetInterface.php" -code = "prefer-self-return-type" -message = "Return type `RowPrototypeResultSetInterface` refers to the enclosing class; use `self` instead." +code = "too-many-methods" +message = "Class has too many methods." count = 1 [[issues]] diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index b4ddc698..ab2e4e8a 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -13,40 +13,79 @@ use PhpDb\Adapter\Driver\ResultInterface; use PhpDb\ResultSet\Exception\InvalidArgumentException; use PhpDb\ResultSet\Exception\RuntimeException; -use ReturnTypeWillChange; +use Traversable; +use function array_key_exists; use function count; use function current; use function is_array; use function reset; +/** + * @api + */ abstract class AbstractResultSet implements ResultSetInterface { + /** How deeply initialize() will unwrap a chain of IteratorAggregate. */ + private const int MAX_ITERATOR_DEPTH = 8; + /** * if -1, datasource is already buffered * if -2, implicitly disabling buffering in ResultSet * if false, explicitly disabled * if null, default state - nothing, but can buffer until iteration started * if array, already buffering + * + * @var int|array|bool|null */ protected int|array|bool|null $buffer = null; protected ?int $count = null; - protected Iterator|IteratorAggregate|ResultInterface|null $dataSource = null; + protected ?Iterator $dataSource = null; protected ?int $fieldCount = null; protected int $position = 0; + /** + * Resolve an IteratorAggregate chain down to the Iterator it wraps. + * + * IteratorAggregate::getIterator() is declared to return Traversable, so it may + * hand back another aggregate. Anything that never bottoms out in an Iterator is + * rejected rather than stored, since the result set can only iterate an Iterator. + * + * @throws InvalidArgumentException + * @throws Exception If the data source raises one while handing over its iterator. + */ + private static function resolveIterator(Traversable $dataSource): Iterator + { + $depth = 0; + while ($dataSource instanceof IteratorAggregate) { + if (++$depth > self::MAX_ITERATOR_DEPTH) { + throw InvalidArgumentException::forUnresolvableIterator(self::MAX_ITERATOR_DEPTH); + } + + $dataSource = $dataSource->getIterator(); + } + + if ($dataSource instanceof Iterator) { + return $dataSource; + } + + throw InvalidArgumentException::forNonIteratorDataSource($dataSource::class); + } + /** * @throws RuntimeException */ public function buffer(): ResultSetInterface { - if ($this->buffer === -2) { + if (-2 === $this->buffer) { throw RuntimeException::forUnbufferedIteration(); - } elseif ($this->buffer === null) { + } + + if (null === $this->buffer) { $this->buffer = []; if ($this->dataSource instanceof ResultInterface) { $this->dataSource->rewind(); @@ -60,38 +99,47 @@ public function buffer(): ResultSetInterface * Countable: return count of rows */ #[Override] - #[ReturnTypeWillChange] - public function count(): ?int + public function count(): int { - if ($this->count !== null) { + if (null !== $this->count) { return $this->count; } if ($this->dataSource instanceof Countable) { $this->count = count($this->dataSource); + + return $this->count; } - return $this->count; + return 0; } /** * Iterator: get current item + * + * @throws RuntimeException */ #[Override] public function current(): array|object|null { if (-1 === $this->buffer) { // datasource was an array when the resultset was initialized - return $this->dataSource->current(); + return $this->dataSource()->current(); } - if ($this->buffer === null) { + if (null === $this->buffer) { $this->buffer = -2; // implicitly disable buffering from here on - } elseif (is_array($this->buffer) && isset($this->buffer[$this->position])) { + } + + if ( + is_array($this->buffer) + && array_key_exists($this->position, $this->buffer) + && null !== $this->buffer[$this->position] + ) { return $this->buffer[$this->position]; } - $data = $this->dataSource->current(); + $data = $this->dataSource()->current(); if (is_array($this->buffer)) { $this->buffer[$this->position] = $data; } @@ -102,7 +150,7 @@ public function current(): array|object|null /** * Get the data source used to create the result set */ - public function getDataSource(): ResultInterface|IteratorAggregate|Iterator|null + public function getDataSource(): ?Iterator { return $this->dataSource; } @@ -166,21 +214,19 @@ public function initialize(iterable $dataSource): ResultSetInterface return $this; } - if (is_array($dataSource)) { - // its safe to get numbers from an array - $first = current($dataSource); - reset($dataSource); - $this->fieldCount = $first === false ? 0 : count($first); - $this->dataSource = new ArrayIterator($dataSource); - $this->buffer = -1; // array's are a natural buffer - } elseif ($dataSource instanceof IteratorAggregate) { - /** @phpstan-ignore assign.propertyType */ - $this->dataSource = $dataSource->getIterator(); - } else { - /** @phpstan-ignore assign.propertyType */ - $this->dataSource = $dataSource; + if ($dataSource instanceof Traversable) { + $this->dataSource = self::resolveIterator($dataSource); + + return $this; } + // the array is safe to measure, but its first row can be any shape at all + $first = current($dataSource); + $this->fieldCount = is_array($first) || $first instanceof Countable ? count($first) : 0; + reset($dataSource); + $this->dataSource = new ArrayIterator($dataSource); + $this->buffer = -1; // array's are a natural buffer + return $this; } @@ -200,16 +246,18 @@ public function key(): int /** * Iterator: move pointer to next item + * + * @throws RuntimeException */ #[Override] public function next(): void { - if ($this->buffer === null) { + if (null === $this->buffer) { $this->buffer = -2; // implicitly disable buffering from here on } - if (! is_array($this->buffer) || $this->position === $this->dataSource->key()) { - $this->dataSource->next(); + if (! is_array($this->buffer) || $this->position === $this->dataSource()->key()) { + $this->dataSource()->next(); } $this->position++; @@ -217,12 +265,14 @@ public function next(): void /** * Iterator: rewind + * + * @throws RuntimeException */ #[Override] public function rewind(): void { if (! is_array($this->buffer)) { - $this->dataSource->rewind(); + $this->dataSource()->rewind(); } $this->position = 0; @@ -230,14 +280,34 @@ public function rewind(): void /** * Iterator: is pointer valid? + * + * @throws RuntimeException */ #[Override] public function valid(): bool { - if (is_array($this->buffer) && isset($this->buffer[$this->position])) { + if ( + is_array($this->buffer) + && array_key_exists($this->position, $this->buffer) + && null !== $this->buffer[$this->position] + ) { return true; } - return $this->dataSource->valid(); + return $this->dataSource()->valid(); + } + + /** + * The data source, once initialize() has supplied one. + * + * @throws RuntimeException + */ + protected function dataSource(): Iterator + { + if (null === $this->dataSource) { + throw RuntimeException::forUninitialisedDataSource(); + } + + return $this->dataSource; } } diff --git a/src/ResultSet/ArrayObjectResultSetInterface.php b/src/ResultSet/ArrayObjectResultSetInterface.php index f73b82ce..3b8d4b77 100644 --- a/src/ResultSet/ArrayObjectResultSetInterface.php +++ b/src/ResultSet/ArrayObjectResultSetInterface.php @@ -8,10 +8,12 @@ /** * Capability interface for a ResultSet whose rows clone an ArrayObject prototype. + * + * @api */ -interface ArrayObjectResultSetInterface +interface ArrayObjectResultSetInterface extends ResultSetInterface { public function getRowPrototype(): ArrayObject; - public function setRowPrototype(ArrayObject $rowPrototype): ResultSetInterface&ArrayObjectResultSetInterface; + public function setRowPrototype(ArrayObject $rowPrototype): self; } diff --git a/src/ResultSet/ArrayResultSet.php b/src/ResultSet/ArrayResultSet.php index 2f93eb4e..14f159a9 100644 --- a/src/ResultSet/ArrayResultSet.php +++ b/src/ResultSet/ArrayResultSet.php @@ -6,7 +6,7 @@ use Override; -class ArrayResultSet extends AbstractResultSet +final class ArrayResultSet extends AbstractResultSet { /** {@inheritDoc} */ #[Override] diff --git a/src/ResultSet/Exception/InvalidArgumentException.php b/src/ResultSet/Exception/InvalidArgumentException.php index eea805db..47da2ca1 100644 --- a/src/ResultSet/Exception/InvalidArgumentException.php +++ b/src/ResultSet/Exception/InvalidArgumentException.php @@ -6,4 +6,21 @@ use PhpDb\Exception; -class InvalidArgumentException extends Exception\InvalidArgumentException {} +use function sprintf; + +final class InvalidArgumentException extends Exception\InvalidArgumentException +{ + final public const string UNRESOLVABLE_ITERATOR = 'The data source nests IteratorAggregate more than %d levels deep and cannot be resolved to an Iterator'; + + final public const string NON_ITERATOR_DATA_SOURCE = 'The data source resolved to "%s", which is not an Iterator and cannot be traversed by a result set'; + + public static function forNonIteratorDataSource(string $type): self + { + return new self(sprintf(self::NON_ITERATOR_DATA_SOURCE, $type)); + } + + public static function forUnresolvableIterator(int $maxDepth): self + { + return new self(sprintf(self::UNRESOLVABLE_ITERATOR, $maxDepth)); + } +} diff --git a/src/ResultSet/Exception/RuntimeException.php b/src/ResultSet/Exception/RuntimeException.php index 4b972d66..eb5c7f90 100644 --- a/src/ResultSet/Exception/RuntimeException.php +++ b/src/ResultSet/Exception/RuntimeException.php @@ -6,12 +6,28 @@ use PhpDb\Exception; -class RuntimeException extends Exception\RuntimeException +use function sprintf; + +final class RuntimeException extends Exception\RuntimeException { final public const string UNBUFFERED_ITERATION = 'Buffering must be enabled before iteration is started'; + final public const string UNINITIALISED_DATA_SOURCE = 'The result set has no data source; call initialize() first'; + public static function forUnbufferedIteration(): self { return new self(self::UNBUFFERED_ITERATION); } + + final public const string UNHYDRATABLE_ROW = 'Cannot extract a row of type "%s"; the hydrator requires an object'; + + public static function forUnhydratableRow(string $type): self + { + return new self(sprintf(self::UNHYDRATABLE_ROW, $type)); + } + + public static function forUninitialisedDataSource(): self + { + return new self(self::UNINITIALISED_DATA_SOURCE); + } } diff --git a/src/ResultSet/HydratingResultSet.php b/src/ResultSet/HydratingResultSet.php index 1653e380..f45ee63d 100644 --- a/src/ResultSet/HydratingResultSet.php +++ b/src/ResultSet/HydratingResultSet.php @@ -8,10 +8,14 @@ use Laminas\Hydrator\ArraySerializableHydrator; use Laminas\Hydrator\HydratorInterface; use Override; +use PhpDb\ResultSet\Exception\RuntimeException; +use function array_key_exists; +use function get_debug_type; use function is_array; +use function is_object; -class HydratingResultSet extends AbstractResultSet implements HydratingResultSetInterface +final class HydratingResultSet extends AbstractResultSet implements HydratingResultSetInterface { public function __construct( private ?HydratorInterface $hydrator = null, @@ -20,16 +24,25 @@ public function __construct( /** * Iterator: get current item + * + * @throws RuntimeException */ #[Override] public function current(): ?object { - if ($this->buffer === null) { + if (null === $this->buffer) { $this->buffer = -2; // implicitly disable buffering from here on - } elseif (is_array($this->buffer) && isset($this->buffer[$this->position])) { + } + + if ( + is_array($this->buffer) + && array_key_exists($this->position, $this->buffer) + && null !== $this->buffer[$this->position] + ) { return $this->buffer[$this->position]; } - $data = $this->dataSource->current(); + + $data = $this->dataSource()->current(); $current = is_array($data) ? $this->getHydrator()->hydrate($data, clone $this->getRowPrototype()) : null; if (is_array($this->buffer)) { @@ -48,7 +61,7 @@ public function getHydrator(): HydratorInterface } /** @deprecated use getRowPrototype() */ - public function getObjectPrototype(): ?object + public function getObjectPrototype(): object { return $this->getRowPrototype(); } @@ -77,7 +90,7 @@ public function setObjectPrototype(object $objectPrototype): ResultSetInterface /** {@inheritDoc} */ #[Override] - public function setRowPrototype(object $rowPrototype): ResultSetInterface&HydratingResultSetInterface + public function setRowPrototype(object $rowPrototype): self { $this->rowPrototype = $rowPrototype; return $this; @@ -93,8 +106,13 @@ public function toArray(): array { $return = []; foreach ($this as $row) { + if (! is_object($row)) { + throw RuntimeException::forUnhydratableRow(get_debug_type($row)); + } + $return[] = $this->getHydrator()->extract($row); } + return $return; } } diff --git a/src/ResultSet/HydratingResultSetInterface.php b/src/ResultSet/HydratingResultSetInterface.php index 011a90b5..594ec20d 100644 --- a/src/ResultSet/HydratingResultSetInterface.php +++ b/src/ResultSet/HydratingResultSetInterface.php @@ -6,10 +6,12 @@ /** * Capability interface for a ResultSet whose rows are hydrated onto an arbitrary object prototype. + * + * @api */ -interface HydratingResultSetInterface +interface HydratingResultSetInterface extends ResultSetInterface { public function getRowPrototype(): object; - public function setRowPrototype(object $rowPrototype): ResultSetInterface&HydratingResultSetInterface; + public function setRowPrototype(object $rowPrototype): self; } diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index f49d0bef..a728d0d4 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -10,33 +10,33 @@ use function is_array; use function is_string; -class ResultSet extends AbstractResultSet implements ArrayObjectResultSetInterface +final class ResultSet extends AbstractResultSet implements ArrayObjectResultSetInterface { /** @deprecated use ResultSetReturnType */ - public const TYPE_ARRAYOBJECT = 'arrayobject'; - public const TYPE_ARRAY = 'array'; + public const string TYPE_ARRAYOBJECT = 'arrayobject'; + public const string TYPE_ARRAY = 'array'; + + private ResultSetReturnType $returnType; public function __construct( - private ResultSetReturnType|string $returnType = ResultSetReturnType::ArrayObject, + ResultSetReturnType|string $returnType = ResultSetReturnType::ArrayObject, private ArrayObject $rowPrototype = new ArrayObject( [], ArrayObject::ARRAY_AS_PROPS, ), ) { - if (is_string($this->returnType)) { - $this->returnType = ResultSetReturnType::from($this->returnType); - } + $this->returnType = is_string($returnType) ? ResultSetReturnType::from($returnType) : $returnType; } /** * Iterator: get current item */ #[Override] - public function current(): array|ArrayObject|null + public function current(): array|object|null { $data = parent::current(); - if ($this->returnType === ResultSetReturnType::ArrayObject && is_array($data)) { + if (ResultSetReturnType::ArrayObject === $this->returnType && is_array($data)) { $ao = clone $this->getRowPrototype(); $ao->exchangeArray($data); @@ -74,14 +74,14 @@ public function getRowPrototype(): ArrayObject * * @deprecated use setRowPrototype() */ - public function setArrayObjectPrototype(ArrayObject $arrayObjectPrototype): ResultSetInterface&ArrayObjectResultSetInterface + public function setArrayObjectPrototype(ArrayObject $arrayObjectPrototype): self { return $this->setRowPrototype($arrayObjectPrototype); } /** {@inheritDoc} */ #[Override] - public function setRowPrototype(ArrayObject $rowPrototype): ResultSetInterface&ArrayObjectResultSetInterface + public function setRowPrototype(ArrayObject $rowPrototype): self { $this->rowPrototype = $rowPrototype; diff --git a/src/ResultSet/ResultSetInterface.php b/src/ResultSet/ResultSetInterface.php index dc0a3525..d06e4ed1 100644 --- a/src/ResultSet/ResultSetInterface.php +++ b/src/ResultSet/ResultSetInterface.php @@ -7,6 +7,11 @@ use Countable; use Iterator; +/** + * @api + * + * @extends Iterator + */ interface ResultSetInterface extends Iterator, Countable { /** @@ -18,11 +23,15 @@ public function getFieldCount(): int; /** * Can be anything iterable|array + * + * @param iterable $dataSource */ - public function initialize(iterable $dataSource): ResultSetInterface; + public function initialize(iterable $dataSource): self; /** * Get all rows as an array + * + * @return list */ public function toArray(): array; } diff --git a/src/ResultSet/RowPrototypeInterface.php b/src/ResultSet/RowPrototypeInterface.php index 44b3ece1..5d3c1db2 100644 --- a/src/ResultSet/RowPrototypeInterface.php +++ b/src/ResultSet/RowPrototypeInterface.php @@ -10,13 +10,15 @@ * Row prototypes are cloned (but do not have to be) for each row and populated via populate(). * This interface allows custom row objects (like RowGateway) to be used * as prototypes without depending on ArrayObject. + * + * @api */ interface RowPrototypeInterface { /** * Populate the prototype with row data. Mutating vs. returning a new instance is up to the implementation. */ - public function populate(array $data): RowPrototypeInterface; + public function populate(array $data): self; /** * Current data as an array and match current RowGateway implementations. diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index 898a66bb..7e189e80 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -8,7 +8,7 @@ use function is_array; -class RowPrototypeResultSet extends AbstractResultSet implements RowPrototypeResultSetInterface +final class RowPrototypeResultSet extends AbstractResultSet implements RowPrototypeResultSetInterface { public function __construct( private RowPrototypeInterface $rowPrototype, @@ -18,7 +18,7 @@ public function __construct( * Iterator: get current item */ #[Override] - public function current(): array|RowPrototypeInterface|null + public function current(): array|object|null { $data = parent::current(); @@ -38,7 +38,7 @@ public function getRowPrototype(): RowPrototypeInterface /** {@inheritDoc} */ #[Override] - public function setRowPrototype(RowPrototypeInterface $rowPrototype): ResultSetInterface&RowPrototypeResultSetInterface + public function setRowPrototype(RowPrototypeInterface $rowPrototype): self { $this->rowPrototype = $rowPrototype; diff --git a/src/ResultSet/RowPrototypeResultSetInterface.php b/src/ResultSet/RowPrototypeResultSetInterface.php index ce10aa20..f6158cdf 100644 --- a/src/ResultSet/RowPrototypeResultSetInterface.php +++ b/src/ResultSet/RowPrototypeResultSetInterface.php @@ -6,10 +6,12 @@ /** * Capability interface for a ResultSet whose rows clone a RowPrototypeInterface prototype. + * + * @api */ -interface RowPrototypeResultSetInterface +interface RowPrototypeResultSetInterface extends ResultSetInterface { public function getRowPrototype(): RowPrototypeInterface; - public function setRowPrototype(RowPrototypeInterface $rowPrototype): ResultSetInterface&RowPrototypeResultSetInterface; + public function setRowPrototype(RowPrototypeInterface $rowPrototype): self; } diff --git a/test/unit/ResultSet/AbstractResultSetTest.php b/test/unit/ResultSet/AbstractResultSetTest.php index 83bd3b7c..29a144b4 100644 --- a/test/unit/ResultSet/AbstractResultSetTest.php +++ b/test/unit/ResultSet/AbstractResultSetTest.php @@ -23,6 +23,7 @@ use TypeError; use function assert; +use function count; #[CoversMethod(AbstractResultSet::class, 'initialize')] #[CoversMethod(AbstractResultSet::class, 'buffer')] @@ -98,6 +99,18 @@ public function bufferIterations(): void static::assertSame(3, $data['id']); } + /** + * @throws Exception + */ + #[Test] + public function countHonoursTheCountableContractForUncountableDataSource(): void + { + $resultSet = $this->createResultSetMock(); + $resultSet->initialize(new NoRewindIterator(new ArrayIterator([['id' => 1]]))); + + static::assertSame($resultSet->count(), count($resultSet)); + } + #[Test] public function countReturnsCachedResult(): void { @@ -112,13 +125,13 @@ public function countReturnsCachedResult(): void } #[Test] - public function countReturnsNullForUncountableDataSource(): void + public function countReturnsZeroForUncountableDataSource(): void { $resultSet = $this->createResultSetMock(); $iterator = new NoRewindIterator(new ArrayIterator([['id' => 1]])); $resultSet->initialize($iterator); - static::assertNull($resultSet->count()); + static::assertSame(0, $resultSet->count()); } /** diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index b1bfd67b..ff98c1f5 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -9,6 +9,7 @@ use Override; use PhpDb\Adapter\Driver\ResultInterface; use PhpDb\ResultSet\AbstractResultSet; +use PhpDb\ResultSet\Exception\InvalidArgumentException; use PhpDb\ResultSet\Exception\RuntimeException; use PhpDb\ResultSet\ResultSet; use PhpDb\ResultSet\ResultSetReturnType; @@ -97,13 +98,14 @@ public function canProvideArrayAsDataSource(): void #[Test] public function canProvideIteratorAggregateAsDataSource(): void { + $iterator = new ArrayIterator([['id' => 1, 'name' => 'one']]); $iteratorAggregate = $this->getMockBuilder('IteratorAggregate') ->onlyMethods(['getIterator']) ->getMock(); - $iteratorAggregate->expects($this->any())->method('getIterator')->willReturn($iteratorAggregate); - // Initialize with IteratorAggregate and verify its iterator is used + $iteratorAggregate->expects($this->any())->method('getIterator')->willReturn($iterator); + // Initialize with IteratorAggregate and verify the iterator it wraps is stored $this->resultSet->initialize($iteratorAggregate); - static::assertSame($iteratorAggregate->getIterator(), $this->resultSet->getDataSource()); + static::assertSame($iterator, $this->resultSet->getDataSource()); } /** @@ -282,6 +284,21 @@ public function invalidDataSourceRaisesException(mixed $dataSource): void $this->resultSet->initialize($dataSource); } + /** + * @throws \Exception + */ + #[Test] + public function rejectsAnIteratorAggregateThatNeverResolvesToAnIterator(): void + { + $iteratorAggregate = $this->getMockBuilder('IteratorAggregate') + ->onlyMethods(['getIterator']) + ->getMock(); + $iteratorAggregate->expects($this->any())->method('getIterator')->willReturn($iteratorAggregate); + + self::expectException(InvalidArgumentException::class); + $this->resultSet->initialize($iteratorAggregate); + } + #[Test] public function returnTypeIsObjectByDefault(): void { diff --git a/test/unit/TableGateway/Feature/EventFeatureTest.php b/test/unit/TableGateway/Feature/EventFeatureTest.php index 105a39c2..3d4465d7 100644 --- a/test/unit/TableGateway/Feature/EventFeatureTest.php +++ b/test/unit/TableGateway/Feature/EventFeatureTest.php @@ -9,7 +9,7 @@ use Override; use PhpDb\Adapter\Driver\ResultInterface; use PhpDb\Adapter\Driver\StatementInterface; -use PhpDb\ResultSet\ResultSet; +use PhpDb\ResultSet\ResultSetInterface; use PhpDb\Sql\Delete; use PhpDb\Sql\Insert; use PhpDb\Sql\Select; @@ -144,7 +144,7 @@ static function (EventFeature\TableGatewayEvent $e) use (&$closureHasRun, &$even $stmt = $this->getMockBuilder(StatementInterface::class)->getMock(); $result = $this->getMockBuilder(ResultInterface::class)->getMock(); - $resultset = $this->getMockBuilder(ResultSet::class)->getMock(); + $resultset = $this->getMockBuilder(ResultSetInterface::class)->getMock(); $this->feature->postSelect($stmt, $result, $resultset); static::assertTrue($closureHasRun); diff --git a/test/unit/TestAsset/TemporaryResultSet.php b/test/unit/TestAsset/TemporaryResultSet.php index bdcf4fe6..4816bef4 100644 --- a/test/unit/TestAsset/TemporaryResultSet.php +++ b/test/unit/TestAsset/TemporaryResultSet.php @@ -4,6 +4,25 @@ namespace PhpDbTest\TestAsset; -use PhpDb\ResultSet\ResultSet; +use Override; +use PhpDb\ResultSet\AbstractResultSet; -class TemporaryResultSet extends ResultSet {} +/** + * A distinct concrete result set, used where a test needs to prove a prototype + * was honoured. It extends the abstract base rather than ResultSet so that the + * concrete result sets can stay final. + */ +final class TemporaryResultSet extends AbstractResultSet +{ + /** {@inheritDoc} */ + #[Override] + public function toArray(): array + { + $return = []; + foreach ($this as $row) { + $return[] = $row; + } + + return $return; + } +} From 9aa802bc63ddc734ebea5c5aafd64a593b053fce Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Tue, 29 Sep 2026 16:48:06 +1000 Subject: [PATCH 2/3] Return object rows rather than discarding them AbstractResultSet::current() ended with is_array($data) ? $data : null. The method declares array|object|null, so objects were meant through; the guard is there to turn false, meaning the results are exhausted, into null, and it took objects with it. Because Pdo\Result::isBuffered() is always false, a PDO result never takes the buffer === -1 path that returns the row untouched, so the guard saw every row. Under FETCH_OBJ and FETCH_LAZY every row became null. No exception, no warning, just no data. FETCH_BOUND still yields null, which is right: that mode carries its row in variables bound by reference and returns only true, so there is nothing to hand back. ResultSetFetchModeTest drives a real SQLite result through the PDO driver across every entry in Result::VALID_FETCH_MODES. Nine modes return rows, FETCH_BOUND returns null while its bound columns fill, and three throw: FETCH_CLASS and FETCH_INTO because setFetchMode() takes only an int and cannot pass the class name or target object PDO needs, and FETCH_FUNC because PDO allows it only in fetchAll(). Those three are covered here so the gap stays visible, and are raised separately against the adapter. --- src/ResultSet/AbstractResultSet.php | 3 +- .../unit/ResultSet/ResultSetFetchModeTest.php | 149 ++++++++++++++++++ 2 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 test/unit/ResultSet/ResultSetFetchModeTest.php diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index ab2e4e8a..90405c40 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -19,6 +19,7 @@ use function count; use function current; use function is_array; +use function is_object; use function reset; /** @@ -144,7 +145,7 @@ public function current(): array|object|null $this->buffer[$this->position] = $data; } - return is_array($data) ? $data : null; + return is_array($data) || is_object($data) ? $data : null; } /** diff --git a/test/unit/ResultSet/ResultSetFetchModeTest.php b/test/unit/ResultSet/ResultSetFetchModeTest.php new file mode 100644 index 00000000..9b78eb3d --- /dev/null +++ b/test/unit/ResultSet/ResultSetFetchModeTest.php @@ -0,0 +1,149 @@ + + */ + public static function rowYieldingModeProvider(): array + { + return [ + 'FETCH_LAZY hands back a PDORow' => [PDO::FETCH_LAZY, PDORow::class], + 'FETCH_ASSOC fills the ArrayObject prototype' => [PDO::FETCH_ASSOC, ArrayObject::class], + 'FETCH_NUM fills the ArrayObject prototype' => [PDO::FETCH_NUM, ArrayObject::class], + 'FETCH_BOTH fills the ArrayObject prototype' => [PDO::FETCH_BOTH, ArrayObject::class], + 'FETCH_OBJ hands back a stdClass' => [PDO::FETCH_OBJ, stdClass::class], + 'FETCH_NAMED fills the ArrayObject prototype' => [PDO::FETCH_NAMED, ArrayObject::class], + 'FETCH_KEY_PAIR fills the ArrayObject prototype' => [PDO::FETCH_KEY_PAIR, ArrayObject::class], + 'FETCH_PROPS_LATE is a flag, so PDO falls back' => [PDO::FETCH_PROPS_LATE, ArrayObject::class], + 'FETCH_CLASSTYPE is a flag, so PDO falls back' => [PDO::FETCH_CLASSTYPE, ArrayObject::class], + ]; + } + + /** + * Modes Result::setFetchMode() accepts but cannot actually drive, because it takes + * only an int and PDO needs more. Documented here so the gap stays visible. + * + * @return array + */ + public static function unusableModeProvider(): array + { + return [ + 'FETCH_CLASS needs a class name PDO is never given' => [PDO::FETCH_CLASS], + 'FETCH_INTO needs a target object PDO is never given' => [PDO::FETCH_INTO], + 'FETCH_FUNC is rejected by PDO outside fetchAll()' => [PDO::FETCH_FUNC], + ]; + } + + /** + * FETCH_BOUND carries its row in variables bound by reference, so there is nothing + * for the result set to hand back. Null is the correct answer here, not a loss. + */ + #[Test] + public function boundFetchModeYieldsNullAndPopulatesTheBoundColumns(): void + { + $result = new Result(); + $result->initialize($this->pdo->query('SELECT id, name FROM t'), null); + $result->setFetchMode(PDO::FETCH_BOUND); + + $id = null; + $resource = $result->getResource(); + $resource->bindColumn(1, $id); + + $resultSet = new ResultSet(); + $resultSet->initialize($result); + + $seen = []; + foreach ($resultSet as $row) { + static::assertNull($row); + $seen[] = (int) $id; + } + + static::assertSame([1, 2], $seen); + } + + #[Test] + #[DataProvider('rowYieldingModeProvider')] + public function everyRowYieldingFetchModeReturnsBothRows(int $fetchMode): void + { + static::assertCount(2, $this->rowsFor($fetchMode)); + } + + #[Test] + #[DataProvider('rowYieldingModeProvider')] + public function everyRowYieldingFetchModeReturnsItsRows(int $fetchMode, string $expectedType): void + { + $rows = $this->rowsFor($fetchMode); + + static::assertContainsOnlyInstancesOf($expectedType, $rows); + } + + #[Test] + #[DataProvider('unusableModeProvider')] + public function fetchModesNeedingExtraArgumentsCannotBeDriven(int $fetchMode): void + { + $this->expectException(Throwable::class); + + $this->rowsFor($fetchMode); + } + + #[Test] + public function objectRowsKeepTheirColumnValues(): void + { + $rows = $this->rowsFor(PDO::FETCH_OBJ); + + static::assertSame([1, 2], array_map(static fn(stdClass $row): int => (int) $row->id, $rows)); + } + + protected function setUp(): void + { + $this->pdo = new PDO('sqlite::memory:'); + $this->pdo->exec('CREATE TABLE t (id INTEGER, name TEXT)'); + $this->pdo->exec('INSERT INTO t VALUES (1, "one"), (2, "two")'); + } + + /** @return list */ + private function rowsFor(int $fetchMode): array + { + $result = new Result(); + $result->initialize($this->pdo->query('SELECT id, name FROM t'), null); + $result->setFetchMode($fetchMode); + + $resultSet = new ResultSet(); + $resultSet->initialize($result); + + return iterator_to_array($resultSet, preserve_keys: false); + } +} From b748bd6f381f3c202f0b39c98a0ea7801a3a0256 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Tue, 29 Sep 2026 16:57:57 +1000 Subject: [PATCH 3/3] Cover the paths this branch introduced Patch coverage on the pull request sat at 63.93%. Most of the gap was attribution rather than absent tests: CoversMethod restricts coverage to the methods it names, and resolveIterator() and dataSource() were new, so their lines were never credited even though every iteration runs them. Both are now declared on the tests that exercise them. The rest were genuinely untested, all of them paths this branch added: - current() and valid() on a result set that was never initialised, so forUninitialisedDataSource() is exercised rather than assumed - HydratingResultSet::toArray() over rows that cannot hydrate, which is the throw the docblock had always promised - the four new named constructors, following the convention already set by RuntimeExceptionTest The throw for a data source that resolves to something other than an Iterator is marked ignored, with the reasoning beside it. Userland cannot implement Traversable without Iterator or IteratorAggregate, and the internal classes that do are aggregates the loop above has unwrapped, so nothing can reach it. It stays because the return type depends on it. src/ResultSet is now 191 of 191 lines. --- src/ResultSet/AbstractResultSet.php | 6 +++ test/unit/ResultSet/AbstractResultSetTest.php | 24 ++++++++++ .../InvalidArgumentExceptionTest.php | 48 +++++++++++++++++++ .../Exception/RuntimeExceptionTest.php | 32 +++++++++++++ .../unit/ResultSet/HydratingResultSetTest.php | 13 +++++ .../ResultSet/ResultSetIntegrationTest.php | 1 + 6 files changed, 124 insertions(+) create mode 100644 test/unit/ResultSet/Exception/InvalidArgumentExceptionTest.php diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index 90405c40..409d85a7 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -74,7 +74,13 @@ private static function resolveIterator(Traversable $dataSource): Iterator return $dataSource; } + // Userland cannot implement Traversable without Iterator or IteratorAggregate, and the + // internal classes that do (PDOStatement, DOMNodeList, DatePeriod) are all aggregates + // the loop above has already unwrapped. Kept so the return type cannot be violated. + // @codeCoverageIgnoreStart throw InvalidArgumentException::forNonIteratorDataSource($dataSource::class); + + // @codeCoverageIgnoreEnd } /** diff --git a/test/unit/ResultSet/AbstractResultSetTest.php b/test/unit/ResultSet/AbstractResultSetTest.php index 29a144b4..b5a69dd7 100644 --- a/test/unit/ResultSet/AbstractResultSetTest.php +++ b/test/unit/ResultSet/AbstractResultSetTest.php @@ -26,6 +26,8 @@ use function count; #[CoversMethod(AbstractResultSet::class, 'initialize')] +#[CoversMethod(AbstractResultSet::class, 'resolveIterator')] +#[CoversMethod(AbstractResultSet::class, 'dataSource')] #[CoversMethod(AbstractResultSet::class, 'buffer')] #[CoversMethod(AbstractResultSet::class, 'isBuffered')] #[CoversMethod(AbstractResultSet::class, 'getDataSource')] @@ -166,6 +168,17 @@ public function current(): void static::assertEquals(['id' => 1, 'name' => 'one'], $resultSet->current()); } + #[Test] + public function currentReportsAnUninitialisedDataSourceRatherThanFailingOnNull(): void + { + $resultSet = $this->createResultSetMock(); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage(RuntimeException::UNINITIALISED_DATA_SOURCE); + + $resultSet->current(); + } + #[Test] public function currentReturnsBufferedDataOnSecondPass(): void { @@ -558,6 +571,17 @@ public function validAnswersFromTheBufferAfterTheDataSourceIsExhausted(): void static::assertTrue($resultSet->valid()); } + #[Test] + public function validReportsAnUninitialisedDataSourceRatherThanFailingOnNull(): void + { + $resultSet = $this->createResultSetMock(); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage(RuntimeException::UNINITIALISED_DATA_SOURCE); + + $resultSet->valid(); + } + #[Test] public function validReturnsFalseAfterLastElement(): void { diff --git a/test/unit/ResultSet/Exception/InvalidArgumentExceptionTest.php b/test/unit/ResultSet/Exception/InvalidArgumentExceptionTest.php new file mode 100644 index 00000000..c330e5b2 --- /dev/null +++ b/test/unit/ResultSet/Exception/InvalidArgumentExceptionTest.php @@ -0,0 +1,48 @@ +getMessage(), + ); + } + + #[Test] + public function forNonIteratorDataSourceReturnsTheComponentExceptionType(): void + { + self::assertInstanceOf( + ExceptionInterface::class, + InvalidArgumentException::forNonIteratorDataSource('DatePeriod'), + ); + } + + #[Test] + public function forUnresolvableIteratorNamesTheDepthItGaveUpAt(): void + { + self::assertStringContainsString('8', InvalidArgumentException::forUnresolvableIterator(8)->getMessage()); + } + + #[Test] + public function forUnresolvableIteratorReturnsTheComponentExceptionType(): void + { + self::assertInstanceOf(ExceptionInterface::class, InvalidArgumentException::forUnresolvableIterator(8)); + } +} diff --git a/test/unit/ResultSet/Exception/RuntimeExceptionTest.php b/test/unit/ResultSet/Exception/RuntimeExceptionTest.php index bab9cbdb..b8d89c08 100644 --- a/test/unit/ResultSet/Exception/RuntimeExceptionTest.php +++ b/test/unit/ResultSet/Exception/RuntimeExceptionTest.php @@ -13,6 +13,8 @@ #[Group('unit')] #[CoversMethod(RuntimeException::class, 'forUnbufferedIteration')] +#[CoversMethod(RuntimeException::class, 'forUninitialisedDataSource')] +#[CoversMethod(RuntimeException::class, 'forUnhydratableRow')] final class RuntimeExceptionTest extends TestCase { #[Test] @@ -29,4 +31,34 @@ public function forUnbufferedIterationReturnsTheComponentExceptionType(): void { self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUnbufferedIteration()); } + + #[Test] + public function forUnhydratableRowNamesTheOffendingType(): void + { + self::assertStringContainsString( + 'bool', + RuntimeException::forUnhydratableRow('bool')->getMessage(), + ); + } + + #[Test] + public function forUnhydratableRowReturnsTheComponentExceptionType(): void + { + self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUnhydratableRow('bool')); + } + + #[Test] + public function forUninitialisedDataSourceRendersItsTemplate(): void + { + self::assertSame( + RuntimeException::UNINITIALISED_DATA_SOURCE, + RuntimeException::forUninitialisedDataSource()->getMessage(), + ); + } + + #[Test] + public function forUninitialisedDataSourceReturnsTheComponentExceptionType(): void + { + self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUninitialisedDataSource()); + } } diff --git a/test/unit/ResultSet/HydratingResultSetTest.php b/test/unit/ResultSet/HydratingResultSetTest.php index d8e79255..35d4bcca 100644 --- a/test/unit/ResultSet/HydratingResultSetTest.php +++ b/test/unit/ResultSet/HydratingResultSetTest.php @@ -207,6 +207,19 @@ public function toArray(): void static::assertIsArray($obj); } + #[Test] + public function toArrayReportsARowTheHydratorCannotExtract(): void + { + $hydratingRs = new HydratingResultSet(); + // Scalars never hydrate into an object, so current() yields null for each row. + $hydratingRs->initialize(new ArrayIterator([1, 2])); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage('null'); + + $hydratingRs->toArray(); + } + #[Test] public function toArrayUsesHydratorExtract(): void { diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index ff98c1f5..15189c73 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -29,6 +29,7 @@ use function var_export; #[CoversMethod(AbstractResultSet::class, 'current')] +#[CoversMethod(AbstractResultSet::class, 'resolveIterator')] #[CoversMethod(AbstractResultSet::class, 'buffer')] #[CoversMethod(ResultSet::class, 'current')] #[CoversMethod(ResultSet::class, 'getReturnType')]