Make Iterator::key() and ::current() nullable

src/Analyser/StmtHandler/ForeachHandler.php

@@ -2,15 +2,18 @@
 
 namespace PHPStan\Analyser\StmtHandler;
 
+use Iterator;
 use IteratorAggregate;
 use PhpParser\Node;
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Array_;
 use PhpParser\Node\Expr\ArrayDimFetch;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\BinaryOp;
+use PhpParser\Node\Expr\ConstFetch;
 use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\List_;
+use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Name;
 use PhpParser\Node\Stmt;
@@ -127,6 +130,13 @@ public function processStmt(
 
 		$foreachIterateeType = $condResult->getType();
 		$foreachNativeIterateeType = $condResult->getNativeType();
+		$iteratorValidExpr = null;
+		if ((new ObjectType(Iterator::class))->isSuperTypeOf($foreachIterateeType)->yes()) {
+			$iteratorValidExpr = new BinaryOp\Identical(
+				new MethodCall($stmt->expr, 'valid'),
+				new ConstFetch(new Name\FullyQualified('true')),
+			);
+		}
 
 		if ($stmt->keyVar instanceof Variable) {
 			$keyTypeExpr = new NativeTypeExpr(
@@ -199,6 +209,9 @@ static function () use ($condResult, $emptyArrayType): Type {
 			$storage = $originalStorage->duplicate();
 
 			$originalScope = $nonEmptyIterateeScope;
+			if ($iteratorValidExpr !== null) {
+				$originalScope = $originalScope->filterByTruthyValue($iteratorValidExpr);
+			}
 			// $originalScope may narrow the iteratee to a non-empty array - a genuinely
 			// different scope than its own. The narrowing is tracked by the scope
 			// (getTypeOnScope's authoritative read), so the iteratee only needs
@@ -233,6 +246,9 @@ static function () use ($condResult, $emptyArrayType): Type {
 				do {
 					$prevScope = $bodyScope;
 					$bodyScope = $bodyScope->mergeWith($nonEmptyIterateeScope);
+					if ($iteratorValidExpr !== null) {
+						$bodyScope = $bodyScope->filterByTruthyValue($iteratorValidExpr);
+					}
 					if ($prevEntryScope !== null && $bodyScope->equals($prevEntryScope)) {
 						// walking is deterministic in the entry scope - an unchanged entry
 						// reproduces the previous pass's exit, so the verification walk is skipped
@@ -283,6 +299,9 @@ static function () use ($condResult, $emptyArrayType): Type {
 		}
 
 		$bodyScope = $bodyScope->mergeWith($nonEmptyIterateeScope);
+		if ($iteratorValidExpr !== null) {
+			$bodyScope = $bodyScope->filterByTruthyValue($iteratorValidExpr);
+		}
 		$finalEntryScope = $bodyScope;
 		$storage = $originalStorage;
 		$bodyScope = $this->enterForeach($nodeScopeResolver, $bodyScope, $storage, $originalScope, $stmt, $foreachIterateeType, $foreachNativeIterateeType, $nodeCallback);
@@ -494,6 +513,10 @@ static function () use ($condResult, $emptyArrayType): Type {
 			$finalScope = $finalScope->assignExpression(new ForeachValueByRefExpr($stmt->valueVar), new MixedType(), new MixedType());
 		}
 
+		if ($iteratorValidExpr !== null) {
+			$finalScope = $finalScope->filterByFalseyValue($iteratorValidExpr);
+		}
+
 		$bindingFlow = VariableFlow::sequence(
 			$stmt->keyVar !== null ? VariableFlowBuilder::targetRead($stmt->keyVar, $storage, false) : null,
 			$stmt->keyVar !== null ? VariableFlowBuilder::targetWrite($stmt->keyVar, VariableWrite::KIND_FOREACH_KEY, $finalScope, $storage) : null,

stubs/iterable.stub

@@ -34,12 +34,21 @@ interface Iterator extends Traversable
 {
 
 	/**
-	 * @return TValue
+	 * @phpstan-assert-if-true =TValue $this->current()
+	 * @phpstan-assert-if-false =null $this->current()
+	 *
+	 * @phpstan-assert-if-true =TKey $this->key()
+	 * @phpstan-assert-if-false =null $this->key()
+	 */
+	public function valid(): bool;
+
+	/**
+	 * @return TValue|null
 	 */
 	public function current();
 
 	/**
-	 * @return TKey
+	 * @return TKey|null
 	 */
 	public function key();
 

tests/PHPStan/Analyser/nsrt/bug-3674.php

@@ -0,0 +1,45 @@
+<?php declare(strict_types = 1);
+
+namespace Bug3674;
+
+use Iterator;
+use function PHPStan\Testing\assertType;
+
+/**
+ * @param Iterator<int> $it
+ */
+function foo(Iterator $it): void {
+	assertType('int|null', $it->current());
+
+	if ($it->valid()) {
+		assertType('int', $it->current());
+
+		$it->rewind();
+
+		assertType('int|null', $it->current());
+
+		if ($it->valid()) {
+			assertType('int', $it->current());
+		} else {
+			assertType('null', $it->current());
+		}
+	} else {
+		assertType('null', $it->current());
+	}
+}
+
+/**
+ * @param Iterator<int> $it
+ */
+function bar(Iterator $it): void {
+	assertType('bool', $it->valid());
+	assertType('int|null', $it->current());
+
+	foreach ($it as $v) {
+		assertType('true', $it->valid());
+		assertType('int', $it->current());
+	}
+
+	assertType('false', $it->valid());
+	assertType('null', $it->current());
+}

tests/PHPStan/Analyser/nsrt/bug-7519.php

@@ -41,8 +41,8 @@ function doFoo() {
 
 	$iterator = new FooFilterIterator($generator());
 
-	assertType('array{}|bool|stdClass', $iterator->key());
-	assertType('array{}|bool|stdClass', $iterator->current());
+	assertType('array{}|bool|stdClass|null', $iterator->key());
+	assertType('array{}|bool|stdClass|null', $iterator->current());
 
 	$generator = static function (): Generator {
 		yield true => true;
@@ -51,6 +51,6 @@ function doFoo() {
 
 	$iterator = new FooFilterIterator($generator());
 
-	assertType('bool', $iterator->key());
-	assertType('bool', $iterator->current());
+	assertType('bool|null', $iterator->key());
+	assertType('bool|null', $iterator->current());
 }

turbo-ext/src/ForeachHandler.cpp

@@ -69,6 +69,8 @@ zend_string *pt_feh_identical_narrowing_helper_class = nullptr;
 zend_string *pt_feh_empty_or_object_narrowing_helper_class = nullptr;
 zend_string *pt_feh_traversable_class = nullptr;
 zend_string *pt_feh_iterator_aggregate_class = nullptr;
+zend_string *pt_feh_iterator_class = nullptr;
+zend_string *pt_feh_valid = nullptr;
 zend_string *pt_feh_get_iterator = nullptr;
 
 /* $node->getAttributes() / ->setAttributes($attributes) */
@@ -374,6 +376,8 @@ class ForeachHandler
 		if (UNEXPECTED(foreachIterateeType.isUndef())) return zv::Val();
 		zv::Val foreachNativeIterateeType = pt_expression_result_get_native_type(condResult.raw());
 		if (UNEXPECTED(foreachNativeIterateeType.isUndef())) return zv::Val();
+		zv::Val iteratorValidExpr = newIteratorValidExpr(iteratee, foreachIterateeType.raw());
+		if (UNEXPECTED(iteratorValidExpr.isUndef())) return zv::Val();
 
 		if (parts.keyVarIsVariable) {
 			zv::Val keyTypeExpr = newIterableTypeExpr(originalScope.raw(), foreachIterateeType.raw(), foreachNativeIterateeType.raw(), true);
@@ -458,6 +462,10 @@ class ForeachHandler
 			if (UNEXPECTED(storage.isUndef())) return zv::Val();
 
 			originalScope = zv::Val::copyOf(nonEmptyIterateeScope.ref());
+			if (!iteratorValidExpr.isNull()) {
+				originalScope = pt_mutating_scope_filter_by_value(Z_OBJ_P(originalScope.raw()), Z_OBJ_P(iteratorValidExpr.raw()), true);
+				if (UNEXPECTED(originalScope.isUndef())) return zv::Val();
+			}
 			// $originalScope may narrow the iteratee to a non-empty array - a genuinely
 			// different scope than its own. The narrowing is tracked by the scope
 			// (getTypeOnScope's authoritative read), so the iteratee only needs
@@ -506,6 +514,10 @@ class ForeachHandler
 					zv::Val prevScope = zv::Val::copyOf(bodyScope.ref());
 					bodyScope = pt_mutating_scope_merge_with(Z_OBJ_P(bodyScope.raw()), nonEmptyIterateeScope.raw());
 					if (UNEXPECTED(bodyScope.isUndef())) return zv::Val();
+					if (!iteratorValidExpr.isNull()) {
+						bodyScope = pt_mutating_scope_filter_by_value(Z_OBJ_P(bodyScope.raw()), Z_OBJ_P(iteratorValidExpr.raw()), true);
+						if (UNEXPECTED(bodyScope.isUndef())) return zv::Val();
+					}
 					if (!prevEntryScope.isNull()) {
 						bool equal;
 						if (UNEXPECTED(!ptlh::scopesEqual(bodyScope.raw(), prevEntryScope.raw(), equal))) return zv::Val();
@@ -579,6 +591,10 @@ class ForeachHandler
 
 		bodyScope = pt_mutating_scope_merge_with(Z_OBJ_P(bodyScope.raw()), nonEmptyIterateeScope.raw());
 		if (UNEXPECTED(bodyScope.isUndef())) return zv::Val();
+		if (!iteratorValidExpr.isNull()) {
+			bodyScope = pt_mutating_scope_filter_by_value(Z_OBJ_P(bodyScope.raw()), Z_OBJ_P(iteratorValidExpr.raw()), true);
+			if (UNEXPECTED(bodyScope.isUndef())) return zv::Val();
+		}
 		zv::Val finalEntryScope = zv::Val::copyOf(bodyScope.ref());
 		zval *lastPass = lastRecordedPass(previousPass);
 		zval *storage = originalStorage;
@@ -806,6 +822,12 @@ class ForeachHandler
 			finalScope = std::move(assigned);
 		}
 
+		if (!iteratorValidExpr.isNull()) {
+			zv::Val filtered = pt_mutating_scope_filter_by_value(Z_OBJ_P(finalScope.raw()), Z_OBJ_P(iteratorValidExpr.raw()), false);
+			if (UNEXPECTED(filtered.isUndef())) return zv::Val();
+			finalScope = std::move(filtered);
+		}
+
 		zv::Val bindingFlow;
 		{
 			zval flows[5];
@@ -1777,6 +1799,35 @@ class ForeachHandler
 		return pt_mutating_scope_assign_variable(Z_OBJ_P(scope), name, type, nativeType, pt_trinary_singleton(PT_TRI_YES));
 	}
 
+	/* processStmt()'s `$iteratorValidExpr`: `$iteratee->valid() === true` when
+	 * the iterated type is always an Iterator, null otherwise, UNDEF = pending
+	 * exception */
+	zv::Val newIteratorValidExpr(zval *iteratee, zval *iterateeType) const
+	{
+		zv::Val iteratorType;
+		{
+			zval out;
+			if (UNEXPECTED(!pt_object_type_new(&out, pt_feh_iterator_class))) return zv::Val();
+			iteratorType = zv::Val::adopt(out);
+		}
+		zend_long superType = isSuperTypeOf(iteratorType.raw(), iterateeType);
+		if (UNEXPECTED(superType < 0)) return zv::Val();
+		if (superType != PT_TRI_YES) return zv::Val::null();
+
+		zv::Args methodCallArgv{iteratee, pt_feh_valid};
+		zv::Val methodCall = pt_type_new(PT_CLASS_METHOD_CALL, 2, methodCallArgv);
+		if (UNEXPECTED(methodCall.isUndef())) return zv::Val();
+		zval name;
+		ZVAL_STRINGL(&name, "true", 4);
+		zv::Val nameValue = zv::Val::adopt(name);
+		zv::Val fullyQualified = pt_name_node_new(PT_CLASS_FULLY_QUALIFIED, nameValue.raw());
+		if (UNEXPECTED(fullyQualified.isUndef())) return zv::Val();
+		zv::Val constFetch = pt_type_new(PT_CLASS_CONST_FETCH, 1, fullyQualified.raw());
+		if (UNEXPECTED(constFetch.isUndef())) return zv::Val();
+		zv::Args identicalArgv{methodCall.raw(), constFetch.raw()};
+		return pt_type_new(PT_CLASS_IDENTICAL_EXPR, 2, identicalArgv);
+	}
+
 	/* Mirrors the private getTraversableForeachThrowPoint(); the throw point
 	 * or null, UNDEF = pending exception */
 	zv::Val getTraversableForeachThrowPoint(zval *scope, zval *iteratee, zval *exprType) const
@@ -1994,6 +2045,8 @@ PT_MINIT_REGISTRATION(pt_register_foreach_handler)
 	pt_feh_empty_or_object_narrowing_helper_class = zend_string_init_interned(PT_LC("PHPStan\\Analyser\\ExprHandler\\Helper\\ForeachEmptyOrObjectNarrowingHelper"), 1);
 	pt_feh_traversable_class = zend_string_init_interned(PT_LC("Traversable"), 1);
 	pt_feh_iterator_aggregate_class = zend_string_init_interned(PT_LC("IteratorAggregate"), 1);
+	pt_feh_iterator_class = zend_string_init_interned(PT_LC("Iterator"), 1);
+	pt_feh_valid = zend_string_init_interned(PT_LC("valid"), 1);
 	pt_feh_get_iterator = zend_string_init_interned(PT_LC("getIterator"), 1);
 
 	reg::Class cls("PHPStan\\Analyser\\StmtHandler\\ForeachHandler");