From 3e27bbe08b955fb550b11bbec1b5cc08095e1a26 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 10:36:43 +1000 Subject: [PATCH 1/5] Return object rows rather than discarding them AbstractResultSet::current() ended with `is_array($data) ? $data : null` while declaring `array|object|null`. The guard exists to turn `false`, meaning the results are exhausted, into `null`, and it took objects with it. Pdo\Result::isBuffered() is always false, so a PDO result never takes the `buffer === -1` path that returns the row untouched, and the guard saw every row. Under FETCH_OBJ and FETCH_LAZY every row became null, with no exception and no warning. FETCH_BOUND is unchanged on purpose: that mode carries its row in variables bound by reference and returns only true, so null is correct. Five further defects followed from the same fact, that a row need not be an array: - initialize() measured the first element of an array rather than the array, so an array of objects raised an uncaught TypeError from count(). - initialize() called getIterator() once, but that method returns Traversable and may hand back another aggregate. It now walks the chain and rejects one that never bottoms out in an Iterator. - The iteration methods reached the data source directly, so calling them before initialize() raised an Error. They now report an uninitialised result set. - HydratingResultSet::toArray() handed non-objects to the hydrator. The @throws tag promised an exception that was never raised; it is now. - ResultSet::current() and RowPrototypeResultSet::current() declared return types that excluded objects the parent can return. 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. A single FETCH_OBJ case would have missed FETCH_LAZY entirely. The three that throw are adapter defects raised separately as #198 and #199. src/ResultSet goes from 90 baselined findings to 36. The remainder are the mixed family that follows from a row being mixed, complexity metrics, array shapes no truthful annotation can narrow, and a Mago stub that treats DOMNodeList and its kin as Traversable but not Iterator. No public signature is narrowed and nothing is sealed, so the change is binary compatible for downstream consumers. --- analyzer-baseline.toml | 198 +++--------------- lint-baseline.toml | 68 +----- src/ResultSet/AbstractResultSet.php | 138 +++++++++--- .../ArrayObjectResultSetInterface.php | 2 + .../Exception/InvalidArgumentException.php | 19 +- src/ResultSet/Exception/RuntimeException.php | 16 ++ src/ResultSet/HydratingResultSet.php | 29 ++- src/ResultSet/HydratingResultSetInterface.php | 2 + src/ResultSet/ResultSet.php | 22 +- src/ResultSet/ResultSetInterface.php | 11 +- src/ResultSet/RowPrototypeInterface.php | 4 +- src/ResultSet/RowPrototypeResultSet.php | 7 +- .../RowPrototypeResultSetInterface.php | 2 + test/unit/ResultSet/AbstractResultSetTest.php | 24 +++ .../InvalidArgumentExceptionTest.php | 48 +++++ .../Exception/RuntimeExceptionTest.php | 32 +++ .../unit/ResultSet/HydratingResultSetTest.php | 13 ++ .../unit/ResultSet/ResultSetFetchModeTest.php | 149 +++++++++++++ .../ResultSet/ResultSetIntegrationTest.php | 24 ++- 19 files changed, 517 insertions(+), 291 deletions(-) create mode 100644 test/unit/ResultSet/Exception/InvalidArgumentExceptionTest.php create mode 100644 test/unit/ResultSet/ResultSetFetchModeTest.php diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index e17bd353..06c340ad 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,24 +1512,12 @@ 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" message = "Type `array` in return type of `current` is imprecise, equivalent to `array`." 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" @@ -1526,26 +1526,20 @@ 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`.' -count = 1 - -[[issues]] -file = "src/ResultSet/AbstractResultSet.php" -code = "invalid-type-cast" -message = "Casting `mixed` to `array`." +code = "invalid-return-statement" +message = 'Invalid return type for function `PhpDb\ResultSet\AbstractResultSet::dataSource`: expected `Iterator`, but found `Iterator|IteratorAggregate|PhpDb\Adapter\Driver\ResultInterface`.' 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.' +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]] file = "src/ResultSet/AbstractResultSet.php" -code = "mixed-argument" -message = "Invalid argument type for argument #1 of `count`: expected `Countable|array`, but found `mixed`." +code = "invalid-type-cast" +message = "Casting `mixed` to `array`." count = 1 [[issues]] @@ -1556,57 +1550,27 @@ 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`." +message = 'Could not infer a precise return type for function `PhpDb\ResultSet\AbstractResultSet::current`. Saw type `nonnull`.' 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 +count = 1 [[issues]] file = "src/ResultSet/AbstractResultSet.php" @@ -1614,30 +1578,12 @@ 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" @@ -1656,12 +1602,6 @@ 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`." -count = 1 - [[issues]] file = "src/ResultSet/HydratingResultSet.php" code = "mixed-assignment" @@ -1674,36 +1614,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" @@ -1716,24 +1632,6 @@ 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" @@ -1746,30 +1644,6 @@ 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,12 +1656,6 @@ 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" @@ -1800,24 +1668,6 @@ 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 +5109,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..9ff1f0d7 100644 --- a/lint-baseline.toml +++ b/lint-baseline.toml @@ -260,88 +260,28 @@ count = 2 [[issues]] file = "src/ResultSet/AbstractResultSet.php" -code = "no-else-clause" -message = "Avoid `else` clauses." +code = "kan-defect" +message = "Class has a high kan defect score." 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 = "too-many-methods" +message = "Class has too many methods." 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" diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index b4ddc698..571908ed 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -14,20 +14,31 @@ 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 is_object; 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; @@ -39,14 +50,50 @@ abstract class AbstractResultSet implements ResultSetInterface 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; + } + + // 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 + } + /** * @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(); @@ -63,7 +110,7 @@ public function buffer(): ResultSetInterface #[ReturnTypeWillChange] public function count(): ?int { - if ($this->count !== null) { + if (null !== $this->count) { return $this->count; } @@ -76,27 +123,35 @@ public function count(): ?int /** * 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; } - return is_array($data) ? $data : null; + return is_array($data) || is_object($data) ? $data : null; } /** @@ -109,6 +164,8 @@ public function getDataSource(): ResultInterface|IteratorAggregate|Iterator|null /** * Retrieve count of fields in individual rows of the result set + * + * @throws RuntimeException */ #[Override] public function getFieldCount(): int @@ -117,11 +174,12 @@ public function getFieldCount(): int return $this->fieldCount; } - $dataSource = $this->getDataSource(); - if (null === $dataSource) { + if (null === $this->dataSource) { return 0; } + $dataSource = $this->dataSource(); + $dataSource->rewind(); if (! $dataSource->valid()) { $this->fieldCount = 0; @@ -166,21 +224,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 +256,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 +275,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 +290,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 (! $this->dataSource instanceof Iterator) { + throw RuntimeException::forUninitialisedDataSource(); + } + + return $this->dataSource; } } diff --git a/src/ResultSet/ArrayObjectResultSetInterface.php b/src/ResultSet/ArrayObjectResultSetInterface.php index f73b82ce..95f9f6e1 100644 --- a/src/ResultSet/ArrayObjectResultSetInterface.php +++ b/src/ResultSet/ArrayObjectResultSetInterface.php @@ -8,6 +8,8 @@ /** * Capability interface for a ResultSet whose rows clone an ArrayObject prototype. + * + * @api */ interface ArrayObjectResultSetInterface { diff --git a/src/ResultSet/Exception/InvalidArgumentException.php b/src/ResultSet/Exception/InvalidArgumentException.php index eea805db..3757942e 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; + +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..20a063eb 100644 --- a/src/ResultSet/Exception/RuntimeException.php +++ b/src/ResultSet/Exception/RuntimeException.php @@ -6,12 +6,28 @@ use PhpDb\Exception; +use function sprintf; + 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..16725c1c 100644 --- a/src/ResultSet/HydratingResultSet.php +++ b/src/ResultSet/HydratingResultSet.php @@ -8,8 +8,12 @@ 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 { @@ -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)) { @@ -77,8 +90,9 @@ public function setObjectPrototype(object $objectPrototype): ResultSetInterface /** {@inheritDoc} */ #[Override] - public function setRowPrototype(object $rowPrototype): ResultSetInterface&HydratingResultSetInterface - { + public function setRowPrototype( + object $rowPrototype, + ): ResultSetInterface&HydratingResultSetInterface { $this->rowPrototype = $rowPrototype; return $this; } @@ -93,8 +107,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..936cec16 100644 --- a/src/ResultSet/HydratingResultSetInterface.php +++ b/src/ResultSet/HydratingResultSetInterface.php @@ -6,6 +6,8 @@ /** * Capability interface for a ResultSet whose rows are hydrated onto an arbitrary object prototype. + * + * @api */ interface HydratingResultSetInterface { diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index f49d0bef..ddf510ff 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -16,27 +16,27 @@ class ResultSet extends AbstractResultSet implements ArrayObjectResultSetInterfa public const TYPE_ARRAYOBJECT = 'arrayobject'; public const 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,15 +74,17 @@ public function getRowPrototype(): ArrayObject * * @deprecated use setRowPrototype() */ - public function setArrayObjectPrototype(ArrayObject $arrayObjectPrototype): ResultSetInterface&ArrayObjectResultSetInterface - { + public function setArrayObjectPrototype( + ArrayObject $arrayObjectPrototype, + ): ResultSetInterface&ArrayObjectResultSetInterface { return $this->setRowPrototype($arrayObjectPrototype); } /** {@inheritDoc} */ #[Override] - public function setRowPrototype(ArrayObject $rowPrototype): ResultSetInterface&ArrayObjectResultSetInterface - { + public function setRowPrototype( + ArrayObject $rowPrototype, + ): ResultSetInterface&ArrayObjectResultSetInterface { $this->rowPrototype = $rowPrototype; return $this; 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..bc5b4cff 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -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,8 +38,9 @@ public function getRowPrototype(): RowPrototypeInterface /** {@inheritDoc} */ #[Override] - public function setRowPrototype(RowPrototypeInterface $rowPrototype): ResultSetInterface&RowPrototypeResultSetInterface - { + public function setRowPrototype( + RowPrototypeInterface $rowPrototype, + ): ResultSetInterface&RowPrototypeResultSetInterface { $this->rowPrototype = $rowPrototype; return $this; diff --git a/src/ResultSet/RowPrototypeResultSetInterface.php b/src/ResultSet/RowPrototypeResultSetInterface.php index ce10aa20..2b1f316d 100644 --- a/src/ResultSet/RowPrototypeResultSetInterface.php +++ b/src/ResultSet/RowPrototypeResultSetInterface.php @@ -6,6 +6,8 @@ /** * Capability interface for a ResultSet whose rows clone a RowPrototypeInterface prototype. + * + * @api */ interface RowPrototypeResultSetInterface { diff --git a/test/unit/ResultSet/AbstractResultSetTest.php b/test/unit/ResultSet/AbstractResultSetTest.php index 83bd3b7c..a0312072 100644 --- a/test/unit/ResultSet/AbstractResultSetTest.php +++ b/test/unit/ResultSet/AbstractResultSetTest.php @@ -25,6 +25,8 @@ use function assert; #[CoversMethod(AbstractResultSet::class, 'initialize')] +#[CoversMethod(AbstractResultSet::class, 'resolveIterator')] +#[CoversMethod(AbstractResultSet::class, 'dataSource')] #[CoversMethod(AbstractResultSet::class, 'buffer')] #[CoversMethod(AbstractResultSet::class, 'isBuffered')] #[CoversMethod(AbstractResultSet::class, 'getDataSource')] @@ -153,6 +155,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 { @@ -545,6 +558,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/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); + } +} diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index b1bfd67b..15189c73 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; @@ -28,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')] @@ -97,13 +99,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 +285,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 { From c087edb1ad507940536acd662d5a86b8b6cc5265 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 10:58:06 +1000 Subject: [PATCH 2/5] Fill row prototypes from object rows instead of widening current() The previous commit widened ResultSet::current() to array|object|null and RowPrototypeResultSet::current() likewise, so that object rows could pass through untouched. That made both declarations weaker than the contract each class actually offers: a ResultSet hands back ArrayObject, and a RowPrototypeResultSet hands back its prototype. Both keep their original narrow return types. A row that already satisfies the declared type is handed back untouched, preserving instance identity. Any other object row is reduced to an array and used to fill a clone of the prototype, so FETCH_OBJ now yields an ArrayObject rather than a stdClass. HydratingResultSet did the same thing behind a ?object declaration: it tested is_array() and returned null for everything else, so object rows were still discarded there. It now hydrates them on the same terms. AbstractResultSet::rowToArray() performs the reduction. A row carrying its values as elements rather than properties, ArrayObject being the common case, is read through its iterator. Everything else goes through get_object_vars(), which sees only public properties from outside the row's scope; a plain cast would instead yield mangled keys for anything private. A row that exposes nothing either way is rejected rather than emptied. PDORow is the case that matters: FETCH_LAZY resolves columns through __get(), so both get_object_vars() and an array cast return an empty array. Converting it would drop every column, which is a quieter version of the defect this branch set out to fix, so FETCH_LAZY now raises RuntimeException::forUnconvertibleRow() through the three result sets that fill a prototype. It continues to work on AbstractResultSet, which promises nothing about the row's type. HydratingResultSet::current() gained enough work to cross the halstead threshold, so the conversion and hydration move into hydrateRow(). src/ResultSet is at 100% line coverage and 35 baselined findings. --- analyzer-baseline.toml | 6 -- src/ResultSet/AbstractResultSet.php | 34 ++++++++++++ src/ResultSet/Exception/RuntimeException.php | 7 +++ src/ResultSet/HydratingResultSet.php | 24 +++++++- src/ResultSet/ResultSet.php | 14 ++++- src/ResultSet/RowPrototypeResultSet.php | 14 ++++- .../Exception/RuntimeExceptionTest.php | 16 ++++++ .../unit/ResultSet/HydratingResultSetTest.php | 31 +++++++++++ .../unit/ResultSet/ResultSetFetchModeTest.php | 24 ++++++-- .../ResultSet/RowPrototypeResultSetTest.php | 55 +++++++++++++++++++ 10 files changed, 210 insertions(+), 15 deletions(-) diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index 06c340ad..052f7885 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -1602,12 +1602,6 @@ code = "class-must-be-final" message = 'Class `PhpDb\ResultSet\HydratingResultSet` should be declared `final`.' count = 1 -[[issues]] -file = "src/ResultSet/HydratingResultSet.php" -code = "mixed-assignment" -message = "Assigning `mixed` type to a variable may lead to unexpected behavior." -count = 1 - [[issues]] file = "src/ResultSet/HydratingResultSet.php" code = "mixed-return-statement" diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index 571908ed..e3954d6c 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -19,8 +19,11 @@ use function array_key_exists; use function count; use function current; +use function get_debug_type; +use function get_object_vars; use function is_array; use function is_object; +use function iterator_to_array; use function reset; /** @@ -320,4 +323,35 @@ protected function dataSource(): Iterator return $this->dataSource; } + + /** + * Reduce an object row to the array a prototype can be filled from. + * + * A row that carries its values as elements rather than properties, ArrayObject + * being the common case, is read through its iterator. Everything else is reduced + * with get_object_vars(), which sees only public properties from out here; a plain + * cast would instead yield mangled keys for anything private. + * + * A row that exposes nothing either way cannot fill a prototype at all. PDORow, + * which FETCH_LAZY yields, is the case that matters: it resolves columns through + * __get() and reduces to an empty array, so converting it would quietly drop every + * column. Such a row is rejected rather than emptied. + * + * @return array + * + * @throws RuntimeException + */ + protected function rowToArray(object $row): array + { + if ($row instanceof Traversable) { + return iterator_to_array($row); + } + + $data = get_object_vars($row); + if ([] === $data) { + throw RuntimeException::forUnconvertibleRow(get_debug_type($row)); + } + + return $data; + } } diff --git a/src/ResultSet/Exception/RuntimeException.php b/src/ResultSet/Exception/RuntimeException.php index 20a063eb..c23f36ea 100644 --- a/src/ResultSet/Exception/RuntimeException.php +++ b/src/ResultSet/Exception/RuntimeException.php @@ -21,6 +21,13 @@ public static function forUnbufferedIteration(): self final public const string UNHYDRATABLE_ROW = 'Cannot extract a row of type "%s"; the hydrator requires an object'; + final public const string UNCONVERTIBLE_ROW = 'A row of type "%s" exposes no properties and cannot fill a row prototype; select a fetch mode that yields arrays'; + + public static function forUnconvertibleRow(string $type): self + { + return new self(sprintf(self::UNCONVERTIBLE_ROW, $type)); + } + public static function forUnhydratableRow(string $type): self { return new self(sprintf(self::UNHYDRATABLE_ROW, $type)); diff --git a/src/ResultSet/HydratingResultSet.php b/src/ResultSet/HydratingResultSet.php index 16725c1c..ffd29230 100644 --- a/src/ResultSet/HydratingResultSet.php +++ b/src/ResultSet/HydratingResultSet.php @@ -42,8 +42,7 @@ public function current(): ?object return $this->buffer[$this->position]; } - $data = $this->dataSource()->current(); - $current = is_array($data) ? $this->getHydrator()->hydrate($data, clone $this->getRowPrototype()) : null; + $current = $this->hydrateRow($this->dataSource()->current()); if (is_array($this->buffer)) { $this->buffer[$this->position] = $current; @@ -116,4 +115,25 @@ public function toArray(): array return $return; } + + /** + * Hydrate one row onto a clone of the prototype. + * + * A row that is neither an array nor reducible to one carries nothing to hydrate, + * which is how FETCH_BOUND arrives. + * + * @throws RuntimeException + */ + private function hydrateRow(mixed $data): ?object + { + if (is_object($data)) { + $data = $this->rowToArray($data); + } + + if (! is_array($data)) { + return null; + } + + return $this->getHydrator()->hydrate($data, clone $this->getRowPrototype()); + } } diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index ddf510ff..2f17e34c 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -8,6 +8,7 @@ use Override; use function is_array; +use function is_object; use function is_string; class ResultSet extends AbstractResultSet implements ArrayObjectResultSetInterface @@ -30,12 +31,23 @@ public function __construct( /** * Iterator: get current item + * + * @throws Exception\RuntimeException If a row cannot fill the prototype. */ #[Override] - public function current(): array|object|null + public function current(): array|ArrayObject|null { $data = parent::current(); + if ($data instanceof ArrayObject) { + // already the shape this result set promises, so hand it back untouched + return $data; + } + + if (is_object($data)) { + $data = $this->rowToArray($data); + } + if (ResultSetReturnType::ArrayObject === $this->returnType && is_array($data)) { $ao = clone $this->getRowPrototype(); $ao->exchangeArray($data); diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index bc5b4cff..d24219ae 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -7,6 +7,7 @@ use Override; use function is_array; +use function is_object; class RowPrototypeResultSet extends AbstractResultSet implements RowPrototypeResultSetInterface { @@ -16,12 +17,23 @@ public function __construct( /** * Iterator: get current item + * + * @throws Exception\RuntimeException If a row cannot fill the prototype. */ #[Override] - public function current(): array|object|null + public function current(): array|RowPrototypeInterface|null { $data = parent::current(); + if ($data instanceof RowPrototypeInterface) { + // already the shape this result set promises, so hand it back untouched + return $data; + } + + if (is_object($data)) { + $data = $this->rowToArray($data); + } + if (is_array($data)) { return (clone $this->getRowPrototype())->populate($data); } diff --git a/test/unit/ResultSet/Exception/RuntimeExceptionTest.php b/test/unit/ResultSet/Exception/RuntimeExceptionTest.php index b8d89c08..fe9ef92a 100644 --- a/test/unit/ResultSet/Exception/RuntimeExceptionTest.php +++ b/test/unit/ResultSet/Exception/RuntimeExceptionTest.php @@ -15,6 +15,7 @@ #[CoversMethod(RuntimeException::class, 'forUnbufferedIteration')] #[CoversMethod(RuntimeException::class, 'forUninitialisedDataSource')] #[CoversMethod(RuntimeException::class, 'forUnhydratableRow')] +#[CoversMethod(RuntimeException::class, 'forUnconvertibleRow')] final class RuntimeExceptionTest extends TestCase { #[Test] @@ -32,6 +33,21 @@ public function forUnbufferedIterationReturnsTheComponentExceptionType(): void self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUnbufferedIteration()); } + #[Test] + public function forUnconvertibleRowNamesTheOffendingType(): void + { + self::assertStringContainsString( + 'PDORow', + RuntimeException::forUnconvertibleRow('PDORow')->getMessage(), + ); + } + + #[Test] + public function forUnconvertibleRowReturnsTheComponentExceptionType(): void + { + self::assertInstanceOf(ExceptionInterface::class, RuntimeException::forUnconvertibleRow('PDORow')); + } + #[Test] public function forUnhydratableRowNamesTheOffendingType(): void { diff --git a/test/unit/ResultSet/HydratingResultSetTest.php b/test/unit/ResultSet/HydratingResultSetTest.php index 35d4bcca..29d1c598 100644 --- a/test/unit/ResultSet/HydratingResultSetTest.php +++ b/test/unit/ResultSet/HydratingResultSetTest.php @@ -10,6 +10,7 @@ use Laminas\Hydrator\ArraySerializableHydrator; use Laminas\Hydrator\ClassMethodsHydrator; use Override; +use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\Exception\RuntimeException; use PhpDb\ResultSet\HydratingResultSet; use PHPUnit\Framework\Attributes\CoversMethod; @@ -18,11 +19,13 @@ use PHPUnit\Framework\TestCase; use stdClass; +#[CoversMethod(AbstractResultSet::class, 'rowToArray')] #[CoversMethod(HydratingResultSet::class, 'setObjectPrototype')] #[CoversMethod(HydratingResultSet::class, 'getObjectPrototype')] #[CoversMethod(HydratingResultSet::class, 'setHydrator')] #[CoversMethod(HydratingResultSet::class, 'getHydrator')] #[CoversMethod(HydratingResultSet::class, 'current')] +#[CoversMethod(HydratingResultSet::class, 'hydrateRow')] #[CoversMethod(HydratingResultSet::class, 'toArray')] #[CoversMethod(HydratingResultSet::class, '__construct')] #[CoversMethod(HydratingResultSet::class, 'setRowPrototype')] @@ -85,6 +88,34 @@ public function currentHasData(): void static::assertInstanceOf('ArrayObject', $obj); } + #[Test] + public function currentHydratesAnObjectRowRatherThanDiscardingIt(): void + { + $row = new stdClass(); + $row->id = 1; + $row->name = 'one'; + + $resultSet = new HydratingResultSet(new ArraySerializableHydrator(), new ArrayObject()); + $resultSet->initialize(new ArrayIterator([$row])); + + $current = $resultSet->current(); + + static::assertInstanceOf(ArrayObject::class, $current); + static::assertSame(['id' => 1, 'name' => 'one'], $current->getArrayCopy()); + } + + #[Test] + public function currentRejectsARowThatExposesNothing(): void + { + $resultSet = new HydratingResultSet(new ArraySerializableHydrator(), new ArrayObject()); + $resultSet->initialize(new ArrayIterator([new stdClass()])); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage('exposes no properties'); + + $resultSet->current(); + } + #[Test] public function currentWithBufferReturnsBufferedObject(): void { diff --git a/test/unit/ResultSet/ResultSetFetchModeTest.php b/test/unit/ResultSet/ResultSetFetchModeTest.php index 9b78eb3d..8fba1fe5 100644 --- a/test/unit/ResultSet/ResultSetFetchModeTest.php +++ b/test/unit/ResultSet/ResultSetFetchModeTest.php @@ -6,15 +6,15 @@ use ArrayObject; use PDO; -use PDORow; use PhpDb\Adapter\Driver\Pdo\Result; +use PhpDb\ResultSet\AbstractResultSet; +use PhpDb\ResultSet\Exception\RuntimeException; use PhpDb\ResultSet\ResultSet; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; -use stdClass; use Throwable; use function array_map; @@ -26,6 +26,7 @@ * every entry in Result::VALID_FETCH_MODES, so that a mode cannot start discarding rows * without a test noticing. */ +#[CoversMethod(AbstractResultSet::class, 'rowToArray')] #[CoversMethod(ResultSet::class, 'current')] #[Group('unit')] final class ResultSetFetchModeTest extends TestCase @@ -40,11 +41,10 @@ final class ResultSetFetchModeTest extends TestCase 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_OBJ fills the ArrayObject prototype' => [PDO::FETCH_OBJ, ArrayObject::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], @@ -119,12 +119,26 @@ public function fetchModesNeedingExtraArgumentsCannotBeDriven(int $fetchMode): v $this->rowsFor($fetchMode); } + /** + * FETCH_LAZY yields a PDORow, which resolves its columns through __get() and so + * reduces to an empty array. There is no way to fill a prototype from it, and + * emptying it silently is what this component used to do. + */ + #[Test] + public function lazyFetchModeIsRejectedRatherThanEmptied(): void + { + self::expectException(RuntimeException::class); + self::expectExceptionMessage('exposes no properties'); + + $this->rowsFor(PDO::FETCH_LAZY); + } + #[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)); + static::assertSame([1, 2], array_map(static fn(ArrayObject $row): int => (int) $row['id'], $rows)); } protected function setUp(): void diff --git a/test/unit/ResultSet/RowPrototypeResultSetTest.php b/test/unit/ResultSet/RowPrototypeResultSetTest.php index 4a588c78..2fd98387 100644 --- a/test/unit/ResultSet/RowPrototypeResultSetTest.php +++ b/test/unit/ResultSet/RowPrototypeResultSetTest.php @@ -5,18 +5,73 @@ namespace PhpDbTest\ResultSet; use ArrayIterator; +use ArrayObject; +use PhpDb\ResultSet\AbstractResultSet; +use PhpDb\ResultSet\Exception\RuntimeException; use PhpDb\ResultSet\RowPrototypeInterface; use PhpDb\ResultSet\RowPrototypeResultSet; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; +use stdClass; +#[CoversMethod(AbstractResultSet::class, 'rowToArray')] #[CoversMethod(RowPrototypeResultSet::class, 'current')] #[CoversMethod(RowPrototypeResultSet::class, 'toArray')] #[Group('unit')] final class RowPrototypeResultSetTest extends TestCase { + #[Test] + public function currentPopulatesThePrototypeFromAnObjectRow(): void + { + $resultSet = new RowPrototypeResultSet($this->createRowPrototype()); + $row = new stdClass(); + $row->id = 1; + $row->name = 'one'; + $resultSet->initialize(new ArrayIterator([$row])); + + $current = $resultSet->current(); + + static::assertInstanceOf(RowPrototypeInterface::class, $current); + static::assertSame(['id' => 1, 'name' => 'one'], $current->toArray()); + } + + #[Test] + public function currentPopulatesThePrototypeFromARowCarryingItsValuesAsElements(): void + { + $resultSet = new RowPrototypeResultSet($this->createRowPrototype()); + $resultSet->initialize(new ArrayIterator([new ArrayObject(['id' => 1, 'name' => 'one'])])); + + $current = $resultSet->current(); + + static::assertInstanceOf(RowPrototypeInterface::class, $current); + static::assertSame(['id' => 1, 'name' => 'one'], $current->toArray()); + } + + #[Test] + public function currentRejectsARowThatExposesNothing(): void + { + $resultSet = new RowPrototypeResultSet($this->createRowPrototype()); + $resultSet->initialize(new ArrayIterator([new stdClass()])); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage('exposes no properties'); + + $resultSet->current(); + } + + #[Test] + public function currentReturnsARowThatIsAlreadyAPrototypeUntouched(): void + { + $prototype = $this->createRowPrototype(); + $row = $this->createRowPrototype()->populate(['id' => 1]); + $resultSet = new RowPrototypeResultSet($prototype); + $resultSet->initialize(new ArrayIterator([$row])); + + static::assertSame($row, $resultSet->current()); + } + #[Test] public function currentReturnsDataUnchangedWhenNotArray(): void { From 1ecfa9bd0ab1312ab4e85912fe1dabf9e02522a8 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 10:59:44 +1000 Subject: [PATCH 3/5] Drop the dead array arm from RowPrototypeResultSet::current() Every array row fills the prototype and every object row is either already a prototype or reduced to an array that does, so the method could only ever return a prototype or null. The declared `array` arm was unreachable. This is the one return type in the component that narrows against 0.6.x. A subclass overriding current() with the wider declaration would no longer satisfy covariance. --- analyzer-baseline.toml | 6 ------ src/ResultSet/RowPrototypeResultSet.php | 6 ++++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/analyzer-baseline.toml b/analyzer-baseline.toml index 052f7885..6ee1f4c5 100644 --- a/analyzer-baseline.toml +++ b/analyzer-baseline.toml @@ -1656,12 +1656,6 @@ 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/RowGateway/AbstractRowGateway.php" code = "imprecise-type" diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index d24219ae..5fd541a9 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -21,7 +21,7 @@ public function __construct( * @throws Exception\RuntimeException If a row cannot fill the prototype. */ #[Override] - public function current(): array|RowPrototypeInterface|null + public function current(): ?RowPrototypeInterface { $data = parent::current(); @@ -38,7 +38,9 @@ public function current(): array|RowPrototypeInterface|null return (clone $this->getRowPrototype())->populate($data); } - return $data; + // every array fills the prototype and every object is reduced to one, so + // anything reaching here is the exhausted-or-unusable row the parent nulls + return null; } /** {@inheritDoc} */ From cd59a9aaaded71e2638b7856910e0e64100b8e6a Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 11:36:11 +1000 Subject: [PATCH 4/5] Narrow ArrayResultSet::current() to ?array and drop added commentary ArrayResultSet had no current() override, so it inherited array|object|null from the base class and could hand back an object despite being the result set that exists to yield arrays. It now declares ?array and reduces an object row through rowToArray(), on the same terms as the three prototype-filling result sets. Five cases cover the method: an array row, an object row, a row carrying its values as elements, a row that exposes nothing, and an exhausted data source. Also removes the inline comments and docblock prose added earlier in this branch. The @throws and @return tags stay, since mago analyze reports unhandled-thrown-type and imprecise-type without them. --- src/ResultSet/AbstractResultSet.php | 18 ------- src/ResultSet/ArrayResultSet.php | 21 ++++++++ src/ResultSet/HydratingResultSet.php | 3 -- src/ResultSet/ResultSet.php | 1 - src/ResultSet/RowPrototypeResultSet.php | 3 -- test/unit/ResultSet/ArrayResultSetTest.php | 59 ++++++++++++++++++++++ 6 files changed, 80 insertions(+), 25 deletions(-) diff --git a/src/ResultSet/AbstractResultSet.php b/src/ResultSet/AbstractResultSet.php index e3954d6c..5f616302 100644 --- a/src/ResultSet/AbstractResultSet.php +++ b/src/ResultSet/AbstractResultSet.php @@ -56,10 +56,6 @@ abstract class AbstractResultSet implements ResultSetInterface /** * 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. */ @@ -78,9 +74,6 @@ 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); @@ -233,7 +226,6 @@ public function initialize(iterable $dataSource): ResultSetInterface 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); @@ -327,16 +319,6 @@ protected function dataSource(): Iterator /** * Reduce an object row to the array a prototype can be filled from. * - * A row that carries its values as elements rather than properties, ArrayObject - * being the common case, is read through its iterator. Everything else is reduced - * with get_object_vars(), which sees only public properties from out here; a plain - * cast would instead yield mangled keys for anything private. - * - * A row that exposes nothing either way cannot fill a prototype at all. PDORow, - * which FETCH_LAZY yields, is the case that matters: it resolves columns through - * __get() and reduces to an empty array, so converting it would quietly drop every - * column. Such a row is rejected rather than emptied. - * * @return array * * @throws RuntimeException diff --git a/src/ResultSet/ArrayResultSet.php b/src/ResultSet/ArrayResultSet.php index 2f93eb4e..e1131452 100644 --- a/src/ResultSet/ArrayResultSet.php +++ b/src/ResultSet/ArrayResultSet.php @@ -6,8 +6,29 @@ use Override; +use function is_object; + class ArrayResultSet extends AbstractResultSet { + /** + * Iterator: get current item + * + * @return array|null + * + * @throws Exception\RuntimeException + */ + #[Override] + public function current(): ?array + { + $data = parent::current(); + + if (is_object($data)) { + return $this->rowToArray($data); + } + + return $data; + } + /** {@inheritDoc} */ #[Override] public function toArray(): array diff --git a/src/ResultSet/HydratingResultSet.php b/src/ResultSet/HydratingResultSet.php index ffd29230..66a17c80 100644 --- a/src/ResultSet/HydratingResultSet.php +++ b/src/ResultSet/HydratingResultSet.php @@ -119,9 +119,6 @@ public function toArray(): array /** * Hydrate one row onto a clone of the prototype. * - * A row that is neither an array nor reducible to one carries nothing to hydrate, - * which is how FETCH_BOUND arrives. - * * @throws RuntimeException */ private function hydrateRow(mixed $data): ?object diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index 2f17e34c..60b85aaa 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -40,7 +40,6 @@ public function current(): array|ArrayObject|null $data = parent::current(); if ($data instanceof ArrayObject) { - // already the shape this result set promises, so hand it back untouched return $data; } diff --git a/src/ResultSet/RowPrototypeResultSet.php b/src/ResultSet/RowPrototypeResultSet.php index 5fd541a9..e34e6d21 100644 --- a/src/ResultSet/RowPrototypeResultSet.php +++ b/src/ResultSet/RowPrototypeResultSet.php @@ -26,7 +26,6 @@ public function current(): ?RowPrototypeInterface $data = parent::current(); if ($data instanceof RowPrototypeInterface) { - // already the shape this result set promises, so hand it back untouched return $data; } @@ -38,8 +37,6 @@ public function current(): ?RowPrototypeInterface return (clone $this->getRowPrototype())->populate($data); } - // every array fills the prototype and every object is reduced to one, so - // anything reaching here is the exhausted-or-unusable row the parent nulls return null; } diff --git a/test/unit/ResultSet/ArrayResultSetTest.php b/test/unit/ResultSet/ArrayResultSetTest.php index 75b46a9c..6cd53564 100644 --- a/test/unit/ResultSet/ArrayResultSetTest.php +++ b/test/unit/ResultSet/ArrayResultSetTest.php @@ -4,16 +4,75 @@ namespace PhpDbTest\ResultSet; +use ArrayIterator; +use ArrayObject; +use PhpDb\ResultSet\AbstractResultSet; use PhpDb\ResultSet\ArrayResultSet; +use PhpDb\ResultSet\Exception\RuntimeException; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; +use stdClass; +#[CoversMethod(AbstractResultSet::class, 'rowToArray')] +#[CoversMethod(ArrayResultSet::class, 'current')] #[CoversMethod(ArrayResultSet::class, 'toArray')] #[Group('unit')] final class ArrayResultSetTest extends TestCase { + #[Test] + public function currentReducesAnObjectRowToAnArray(): void + { + $row = new stdClass(); + $row->id = 1; + $row->name = 'one'; + + $resultSet = new ArrayResultSet(); + $resultSet->initialize(new ArrayIterator([$row])); + + static::assertSame(['id' => 1, 'name' => 'one'], $resultSet->current()); + } + + #[Test] + public function currentReducesARowCarryingItsValuesAsElements(): void + { + $resultSet = new ArrayResultSet(); + $resultSet->initialize(new ArrayIterator([new ArrayObject(['id' => 1, 'name' => 'one'])])); + + static::assertSame(['id' => 1, 'name' => 'one'], $resultSet->current()); + } + + #[Test] + public function currentRejectsARowThatExposesNothing(): void + { + $resultSet = new ArrayResultSet(); + $resultSet->initialize(new ArrayIterator([new stdClass()])); + + self::expectException(RuntimeException::class); + self::expectExceptionMessage('exposes no properties'); + + $resultSet->current(); + } + + #[Test] + public function currentReturnsAnArrayRowUntouched(): void + { + $resultSet = new ArrayResultSet(); + $resultSet->initialize([['id' => 1, 'name' => 'one']]); + + static::assertSame(['id' => 1, 'name' => 'one'], $resultSet->current()); + } + + #[Test] + public function currentReturnsNullOnceTheRowsAreExhausted(): void + { + $resultSet = new ArrayResultSet(); + $resultSet->initialize(new ArrayIterator([])); + + static::assertNull($resultSet->current()); + } + #[Test] public function toArrayReturnsRowsAsProvided(): void { From bf86e8ea5188e149bc5b9a67bc13db5d4c4e6088 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 30 Sep 2026 11:52:42 +1000 Subject: [PATCH 5/5] Honour ResultSetReturnType::Prototype in ResultSet::current() The case was added in f3942077 with no consumer. Nothing read it, so a caller selecting it silently got Array behaviour: the constructor accepted it, getReturnType() reported it, current() checked only for ArrayObject and let everything else fall through to the raw row. Every observable signal said it worked. Prototype is the successor name for ArrayObject. It follows the rename already in flight elsewhere in the class, where TYPE_ARRAYOBJECT gives way to the enum and setArrayObjectPrototype() to setRowPrototype(); for ResultSet the ArrayObject and the row prototype are the same object. Both cases now fill the prototype. The decision is resolved once in the constructor rather than on each current() call, since the return type cannot change after construction. Both properties are readonly so that a later setter cannot leave the derived flag stale. Per-row cost drops to a bool read, below the enum comparison it replaces. The match is exhaustive. A fourth case with no decision here raises UnhandledMatchError at construction, before any row is touched, rather than falling through current() unnoticed. returnTypeProvider() covers every case and asserts its own completeness against ResultSetReturnType::cases(), so the same omission fails the suite too. Stacked on fix/resultset-object-rows, which current() depends on. --- src/ResultSet/ResultSet.php | 18 +++++++-- .../ResultSet/ResultSetIntegrationTest.php | 37 ++++++++++++++++++- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/ResultSet/ResultSet.php b/src/ResultSet/ResultSet.php index 60b85aaa..29437afa 100644 --- a/src/ResultSet/ResultSet.php +++ b/src/ResultSet/ResultSet.php @@ -17,7 +17,15 @@ class ResultSet extends AbstractResultSet implements ArrayObjectResultSetInterfa public const TYPE_ARRAYOBJECT = 'arrayobject'; public const TYPE_ARRAY = 'array'; - private ResultSetReturnType $returnType; + private readonly ResultSetReturnType $returnType; + + /** + * Whether the selected return type fills the row prototype, resolved once. + * + * The match is exhaustive so that a return type added without a decision here + * fails at construction rather than falling through current() unnoticed. + */ + private readonly bool $fillsRowPrototype; public function __construct( ResultSetReturnType|string $returnType = ResultSetReturnType::ArrayObject, @@ -26,7 +34,11 @@ public function __construct( ArrayObject::ARRAY_AS_PROPS, ), ) { - $this->returnType = is_string($returnType) ? ResultSetReturnType::from($returnType) : $returnType; + $this->returnType = is_string($returnType) ? ResultSetReturnType::from($returnType) : $returnType; + $this->fillsRowPrototype = match ($this->returnType) { + ResultSetReturnType::ArrayObject, ResultSetReturnType::Prototype => true, + ResultSetReturnType::Array => false, + }; } /** @@ -47,7 +59,7 @@ public function current(): array|ArrayObject|null $data = $this->rowToArray($data); } - if (ResultSetReturnType::ArrayObject === $this->returnType && is_array($data)) { + if ($this->fillsRowPrototype && is_array($data)) { $ao = clone $this->getRowPrototype(); $ao->exchangeArray($data); diff --git a/test/unit/ResultSet/ResultSetIntegrationTest.php b/test/unit/ResultSet/ResultSetIntegrationTest.php index 15189c73..6bc39324 100644 --- a/test/unit/ResultSet/ResultSetIntegrationTest.php +++ b/test/unit/ResultSet/ResultSetIntegrationTest.php @@ -24,6 +24,8 @@ use stdClass; use TypeError; +use function count; +use function get_debug_type; use function is_array; use function random_int; use function var_export; @@ -44,7 +46,6 @@ final class ResultSetIntegrationTest extends TestCase { protected ResultSet $resultSet; - /** @psalm-return array */ public static function invalidReturnTypes(): array { return [ @@ -57,6 +58,30 @@ public static function invalidReturnTypes(): array ]; } + /** @psalm-return array */ + /** + * Every case in ResultSetReturnType and the type a row arrives as, so a case + * cannot be added without deciding what current() does with it. + * + * @return array + */ + public static function returnTypeProvider(): array + { + $cases = [ + 'ArrayObject fills the row prototype' => [ResultSetReturnType::ArrayObject, ArrayObject::class], + 'Prototype fills the row prototype' => [ResultSetReturnType::Prototype, ArrayObject::class], + 'Array hands back the raw row' => [ResultSetReturnType::Array, 'array'], + ]; + + self::assertSame( + count(ResultSetReturnType::cases()), + count($cases), + 'every ResultSetReturnType case needs a row type asserted here', + ); + + return $cases; + } + /** * @throws Exception * @throws \Exception @@ -217,6 +242,16 @@ public function dataSourceIsNullByDefault(): void static::assertNull($this->resultSet->getDataSource()); } + #[Test] + #[DataProvider('returnTypeProvider')] + public function everyReturnTypeYieldsItsRowType(ResultSetReturnType $returnType, string $expected): void + { + $resultSet = new ResultSet($returnType); + $resultSet->initialize([['id' => 1, 'name' => 'one']]); + + static::assertSame($expected, get_debug_type($resultSet->current())); + } + #[Test] public function fieldCountIsZeroWithNoDataSourcePresent(): void {