diff --git a/src/Analyser/StmtHandler/ForeachHandler.php b/src/Analyser/StmtHandler/ForeachHandler.php index 2ba5323bf7b..1adb5ad7b76 100644 --- a/src/Analyser/StmtHandler/ForeachHandler.php +++ b/src/Analyser/StmtHandler/ForeachHandler.php @@ -2,6 +2,7 @@ namespace PHPStan\Analyser\StmtHandler; +use Iterator; use IteratorAggregate; use PhpParser\Node; use PhpParser\Node\Expr; @@ -9,8 +10,10 @@ 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, diff --git a/stubs/iterable.stub b/stubs/iterable.stub index b2982ba1522..5b1702bfec2 100644 --- a/stubs/iterable.stub +++ b/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(); diff --git a/tests/PHPStan/Analyser/nsrt/bug-3674.php b/tests/PHPStan/Analyser/nsrt/bug-3674.php new file mode 100644 index 00000000000..10b0edd831b --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-3674.php @@ -0,0 +1,45 @@ + $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 $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()); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-7519.php b/tests/PHPStan/Analyser/nsrt/bug-7519.php index 1fd556f0e3a..b8201139bbf 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-7519.php +++ b/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()); } diff --git a/turbo-ext/src/ForeachHandler.cpp b/turbo-ext/src/ForeachHandler.cpp index 93d6fd6990c..0901144888b 100644 --- a/turbo-ext/src/ForeachHandler.cpp +++ b/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");