[simon-mundy]

Takes src/ResultSet from 90 Mago findings to 20, and fixes six defects found along the way. Every one of them followed from the same fact: a row reaching the result set need not be an array.

	Before	After
Lint	25	4
Analyzer	65	16
PHPUnit	1988 tests, 3901 assertions	2017 tests, 3932 assertions

mago lint, mago analyze and mago format --check are clean.

Object rows were being discarded

AbstractResultSet::current() ended with is_array($data) ? $data : null. The method declares array|object|null, so objects were meant through. That 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. No exception, no warning, no data.

FETCH_LAZY   before: [null, null]      after: [PDORow, PDORow]
FETCH_OBJ    before: [null, null]      after: [stdClass, stdClass]
FETCH_BOUND  before: [null, null]      after: [null, null]

FETCH_BOUND is unchanged on purpose: that mode carries its row in variables bound by reference and returns only true, so null is the right answer.

The rest of the defects

• initialize() measured the first element of an array rather than the array, so an array of objects raised an uncaught TypeError from count().
• 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.
• count() returned null while count($resultSet) returned 0, against the Countable contract. It returns int now, and the 0 is deliberately not memoised so a count() before initialize() cannot poison the cache.
• getObjectPrototype() declared ?object but delegates to a method that cannot return null.

The iterator contract

$dataSource narrows 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 assign.propertyType 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 rather than stored. The iteration methods reach the data source through an accessor that reports an uninitialised result set instead of calling a method on null.

prefer-self-return-type is retired

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. The return types are now plain self.

That matters beyond this component: prefer-self-return-type rewrote ResultSetInterface&ArrayObjectResultSetInterface to ResultSetInterface&self, which does not parse. It was the reason mago lint --fix was unsafe to run against src. It is now safe.

Sealing

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 exception classes are final, and EventFeatureTest doubles the interface it actually needs rather than a now-final class.

Tests

Two existing tests asserted the old behaviour and are corrected. One mocked an IteratorAggregate whose getIterator() returned itself and expected that to be stored, which is the bug written down as a contract. The other expected null rather than 0 from an uncountable data source.

ResultSetFetchModeTest is new. It 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.

Raised separately

The three modes that throw are adapter defects, not result set ones:

• Result::setFetchMode() cannot drive FETCH_CLASS or FETCH_INTO #198 Result::setFetchMode() takes only an int, so FETCH_CLASS and FETCH_INTO can never receive the class name or target object PDO needs.
• VALID_FETCH_MODES lists three entries that are not usable fetch modes #199 VALID_FETCH_MODES lists FETCH_FUNC, which PDO allows only in fetchAll(), and two bit flags for FETCH_CLASS that are meaningless alone.

The 20 that remain

Nine are the mixed family that follows from a row being mixed, five are complexity metrics, three are deferred to RowGateway along 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.

Worth noting that array<string, mixed> is not a safe shape for a row here. The fetch mode is caller selectable, so a row can be string keyed, integer keyed, both, an object, true, or false. The only truthful annotation is array<array-key, mixed>, which is identical to bare array, so those findings survive any honest fix.

Merge note

Branched from 0.6.x, so the regenerated baselines do not include #197. The two will conflict on lint-baseline.toml and analyzer-baseline.toml. Regenerate after rebasing rather than merging those files by hand.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[simon-mundy]

Superseded by #201, which ships the six defect fixes without the binary compatibility breaks.

Everything dropped is listed under "Deliberately not fixed" there: the Countable violation in count(), the sealing, the narrowed getDataSource() and $dataSource, getObjectPrototype(): ?object, the typed constants, and the interface re-parenting that retired prefer-self-return-type.

Closing rather than reworking, so this description stays as the record of what a major can pick up.