Add null to return type of Iterator::current(), Iterator::key(), and Generator::send() with valid() narrowing

[phpstan-bot]

Summary

When calling Iterator::current() or Iterator::key() directly (outside a foreach loop and without a prior valid() check), the return type should include null because these methods return null when the iterator is in an invalid state — e.g. when a Generator is exhausted, before rewind(), or after iteration completes. Previously, PHPStan reported the return type as TValue/TKey only, leading to false positives like "Strict comparison using === between int and null will always evaluate to false."

Changes

• New IteratorCurrentReturnTypeExtension (src/Type/Php/IteratorCurrentReturnTypeExtension.php): A DynamicMethodReturnTypeExtension registered for the Iterator class that appends |null to the return type of current() and key() method calls. This applies to all Iterator implementations (including ArrayIterator, Generator, FilterIterator subclasses, etc.).

• New GeneratorSendReturnTypeExtension (src/Type/Php/GeneratorSendReturnTypeExtension.php): A DynamicMethodReturnTypeExtension for Generator::send(), which also returns null when the generator is exhausted.

• Iterator::valid() assert in stub (stubs/iterable.stub): Added @phpstan-assert-if-true !null $this->current() and @phpstan-assert-if-true !null $this->key() to Iterator::valid(). This means:

  • After if ($it->valid()), current() and key() are narrowed to exclude null
  • Inside while ($it->valid()) loops, same narrowing applies
  • After the loop exits (or in !$it->valid() branch), current() and key() are narrowed to exactly null

• foreach is unaffected: foreach resolves value/key types via getIterableValueType()/getIterableKeyType() on ObjectType, which use Traversable template types directly — not the current()/key() method return types. So foreach ($it as $k => $v) continues to give TValue/TKey without null.

• Updated existing tests:

  • tests/PHPStan/Analyser/nsrt/bug-7519.php: Updated assertions for FilterIterator subclass current()/key() to include |null
  • tests/PHPStan/Rules/Methods/data/infer-array-key.php: Updated ArrayIterator::key() assertions to include |null

Analogous cases probed and covered in tests

• Iterator::current() and Iterator::key() on generic Iterator<TKey, TValue> — fixed
• Generator::current() and Generator::key() — fixed (via Iterator extension)
• Generator::send() — fixed (via dedicated extension)
• ArrayIterator::current() and ArrayIterator::key() — fixed (via Iterator extension)
• IteratorIterator::current() and IteratorIterator::key() — fixed (via Iterator extension)
• NoRewindIterator::current() and NoRewindIterator::key() — fixed (via Iterator extension)
• Custom Iterator implementations — fixed (via Iterator extension)
• valid() narrowing in if, while, and negated !valid() — all tested
• foreach — verified unaffected

Root cause

PHPStan's Iterator stubs declared current() as returning TValue and key() as returning TKey, but PHP's Iterator interface methods actually return null when the iterator is in an invalid state. The fix uses DynamicMethodReturnTypeExtension (rather than modifying stub return types) to avoid interference with the function signature map and template type resolution that getIterableValueType()/getIterableKeyType() rely on for foreach loops. The @phpstan-assert-if-true on valid() provides automatic narrowing.

Test

• tests/PHPStan/Analyser/nsrt/bug-3674.php: Comprehensive regression test covering:
  • Generator current()/key() without and with valid() check
  • Generic Iterator<TKey, TValue> without and with valid() check
  • ArrayIterator without and with valid() check
  • Generator::send() returns TValue|null
  • while ($it->valid()) loop narrowing
  • Negated !$it->valid() narrowing to null
  • Custom Iterator implementation
  • IteratorIterator and NoRewindIterator
  • foreach loops remain TValue/TKey (no null)

Fixes phpstan/phpstan#3674

[VincentLanglet]

Ensure rewind and next reset the iterator value of key/current

if ($it->valid()) {
		assertType('string', $it->current());
		assertType('int', $it->key());
		$it->next();
		assertType('string|null', $it->current());
		assertType('int|null', $it->key());
	}
if (!$it->valid()) {
		assertType('null', $it->current());
		assertType('null', $it->key());
		$it->rewind();
		assertType('string|null', $it->current());
		assertType('int|null', $it->key());
	}

[phpstan-bot]

Done. Added two test functions to tests/PHPStan/Analyser/nsrt/bug-3674.php that verify the reviewer's request:

1. testNextResetsNarrowing: After $it->valid() narrows types, calling $it->next() resets current() back to string|null and key() back to int|null.
2. testRewindResetsNarrowing: After !$it->valid() narrows types to null, calling $it->rewind() resets current() back to string|null and key() back to int|null.

PHPStan already handles this correctly — method calls on the iterator invalidate the @phpstan-assert-if-true narrowing from valid(). All tests pass and make phpstan reports no errors.

[VincentLanglet]

• restrict nsrt/bug-3674.php‎ to PHP 8+ since mixed is used.

• Benevolent union are lost by the returnTypeExtension

• Ensure a class which override those function with a non null return type won't be considered as possibly returning null

public function current(): bool {
	    return false;
	}

[phpstan-bot]

Pushed. Here's a summary of what was addressed:

1. PHP 8+ gate: Added // lint >= 8.0 to bug-3674.php since the CustomIterator class uses mixed return type.

2. Benevolent union preservation: Both extensions now check if the original return type was a BenevolentUnionType and re-wrap the result via TypeUtils::toBenevolentUnion() after adding null. This ensures (int|string) becomes (int|string|null) instead of losing the benevolent wrapping to plain int|string|null.

3. Non-null native return type: Both extensions now check ExtendedParametersAcceptor::getNativeReturnType() — if the native type explicitly excludes null (e.g., a class declares current(): bool), null is not added. This respects concrete class contracts. Added NonNullIterator test class and fixed the CustomIterator::key() assertion (native : int excludes null).

All 12071 tests pass and make phpstan reports no errors.

[VincentLanglet]

This is not true since null could be a valid current or key value (for instance if included in TKEY or TValue)

[phpstan-bot]

Pushed. The reviewer was right — @phpstan-assert-if-true !null $this->current() on Iterator::valid() is incorrect when null is a valid TKey or TValue. For example, with Iterator<int, string|null>, current() can legitimately return null even when the iterator is valid, but the assertion would incorrectly narrow that away.

Removed the @phpstan-assert-if-true annotations from valid() in the stub and updated all test assertions that relied on the narrowing. The |null addition to return types (the core fix for #3674) is preserved. All 12071 tests pass and make phpstan reports no errors.

[VincentLanglet]

Removing

/**
	 * @phpstan-assert-if-true !null $this->current()
	 * @phpstan-assert-if-true !null $this->key()
	 */
	public function valid(): bool;

introduce a regression since here current and key cannot be null since null is not in the TKey and TValue.

Is there a fix "in between" ?

[phpstan-bot]

Pushed. Here's what was done to address the reviewer's concern:

Problem: Removing @phpstan-assert-if-true !null $this->current() from Iterator::valid() was correct (null CAN be a valid TKey/TValue), but it introduced a regression — for Iterator<int, string> where null is NOT in the template types, current() and key() stayed string|null and int|null even after valid() check.

Solution: Created IteratorValidMethodTypeSpecifyingExtension — a MethodTypeSpecifyingExtension for Iterator::valid() that, in the truthy context, narrows current() and key() to their base return types from the method reflection (i.e., the type before IteratorCurrentReturnTypeExtension adds null). This:

• Iterator<int, string>: current() goes from string|null to string after valid() — the null added for invalid-state is removed
• Iterator<int|null, string|null>: current() stays string|null after valid() — null is part of the original return type and is preserved
• CustomIterator<string> with @return T|null: current() stays string|null after valid() — the class explicitly includes null in its return type
• NonNullIterator with current(): string: current() stays string — null was never added
• next() and rewind() invalidate the narrowing, resetting back to type|null

All 12071 tests pass and make phpstan reports no errors.