Skip to content

Commit e3cb3c8

Browse files
rvanvelzenondrejmirtes
rvanvelzen
authored and
ondrejmirtes
committed
Make Iterator::current() and ::key() nullable
1 parent 091c5a7 commit e3cb3c8

5 files changed

Lines changed: 136 additions & 6 deletions

File tree

‎src/Analyser/StmtHandler/ForeachHandler.php‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,18 @@
22

33
namespace PHPStan\Analyser\StmtHandler;
44

5+
use Iterator;
56
use IteratorAggregate;
67
use PhpParser\Node;
78
use PhpParser\Node\Expr;
89
use PhpParser\Node\Expr\Array_;
910
use PhpParser\Node\Expr\ArrayDimFetch;
1011
use PhpParser\Node\Expr\Assign;
1112
use PhpParser\Node\Expr\BinaryOp;
13+
use PhpParser\Node\Expr\ConstFetch;
1214
use PhpParser\Node\Expr\FuncCall;
1315
use PhpParser\Node\Expr\List_;
16+
use PhpParser\Node\Expr\MethodCall;
1417
use PhpParser\Node\Expr\Variable;
1518
use PhpParser\Node\Name;
1619
use PhpParser\Node\Stmt;
@@ -127,6 +130,13 @@ public function processStmt(
127130

128131
$foreachIterateeType = $condResult->getType();
129132
$foreachNativeIterateeType = $condResult->getNativeType();
133+
$iteratorValidExpr = null;
134+
if ((new ObjectType(Iterator::class))->isSuperTypeOf($foreachIterateeType)->yes()) {
135+
$iteratorValidExpr = new BinaryOp\Identical(
136+
new MethodCall($stmt->expr, 'valid'),
137+
new ConstFetch(new Name\FullyQualified('true')),
138+
);
139+
}
130140

131141
if ($stmt->keyVar instanceof Variable) {
132142
$keyTypeExpr = new NativeTypeExpr(
@@ -199,6 +209,9 @@ static function () use ($condResult, $emptyArrayType): Type {
199209
$storage = $originalStorage->duplicate();
200210

201211
$originalScope = $nonEmptyIterateeScope;
212+
if ($iteratorValidExpr !== null) {
213+
$originalScope = $originalScope->filterByTruthyValue($iteratorValidExpr);
214+
}
202215
// $originalScope may narrow the iteratee to a non-empty array - a genuinely
203216
// different scope than its own. The narrowing is tracked by the scope
204217
// (getTypeOnScope's authoritative read), so the iteratee only needs
@@ -233,6 +246,9 @@ static function () use ($condResult, $emptyArrayType): Type {
233246
do {
234247
$prevScope = $bodyScope;
235248
$bodyScope = $bodyScope->mergeWith($nonEmptyIterateeScope);
249+
if ($iteratorValidExpr !== null) {
250+
$bodyScope = $bodyScope->filterByTruthyValue($iteratorValidExpr);
251+
}
236252
if ($prevEntryScope !== null && $bodyScope->equals($prevEntryScope)) {
237253
// walking is deterministic in the entry scope - an unchanged entry
238254
// reproduces the previous pass's exit, so the verification walk is skipped
@@ -283,6 +299,9 @@ static function () use ($condResult, $emptyArrayType): Type {
283299
}
284300

285301
$bodyScope = $bodyScope->mergeWith($nonEmptyIterateeScope);
302+
if ($iteratorValidExpr !== null) {
303+
$bodyScope = $bodyScope->filterByTruthyValue($iteratorValidExpr);
304+
}
286305
$finalEntryScope = $bodyScope;
287306
$storage = $originalStorage;
288307
$bodyScope = $this->enterForeach($nodeScopeResolver, $bodyScope, $storage, $originalScope, $stmt, $foreachIterateeType, $foreachNativeIterateeType, $nodeCallback);
@@ -494,6 +513,10 @@ static function () use ($condResult, $emptyArrayType): Type {
494513
$finalScope = $finalScope->assignExpression(new ForeachValueByRefExpr($stmt->valueVar), new MixedType(), new MixedType());
495514
}
496515

516+
if ($iteratorValidExpr !== null) {
517+
$finalScope = $finalScope->filterByFalseyValue($iteratorValidExpr);
518+
}
519+
497520
$bindingFlow = VariableFlow::sequence(
498521
$stmt->keyVar !== null ? VariableFlowBuilder::targetRead($stmt->keyVar, $storage, false) : null,
499522
$stmt->keyVar !== null ? VariableFlowBuilder::targetWrite($stmt->keyVar, VariableWrite::KIND_FOREACH_KEY, $finalScope, $storage) : null,

‎stubs/iterable.stub‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,21 @@ interface Iterator extends Traversable
3434
{
3535

3636
/**
37-
* @return TValue
37+
* @phpstan-assert-if-true =TValue $this->current()
38+
* @phpstan-assert-if-false =null $this->current()
39+
*
40+
* @phpstan-assert-if-true =TKey $this->key()
41+
* @phpstan-assert-if-false =null $this->key()
42+
*/
43+
public function valid(): bool;
44+
45+
/**
46+
* @return TValue|null
3847
*/
3948
public function current();
4049

4150
/**
42-
* @return TKey
51+
* @return TKey|null
4352
*/
4453
public function key();
4554

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug3674;
4+
5+
use Iterator;
6+
use function PHPStan\Testing\assertType;
7+
8+
/**
9+
* @param Iterator<int> $it
10+
*/
11+
function foo(Iterator $it): void {
12+
assertType('int|null', $it->current());
13+
14+
if ($it->valid()) {
15+
assertType('int', $it->current());
16+
17+
$it->rewind();
18+
19+
assertType('int|null', $it->current());
20+
21+
if ($it->valid()) {
22+
assertType('int', $it->current());
23+
} else {
24+
assertType('null', $it->current());
25+
}
26+
} else {
27+
assertType('null', $it->current());
28+
}
29+
}
30+
31+
/**
32+
* @param Iterator<int> $it
33+
*/
34+
function bar(Iterator $it): void {
35+
assertType('bool', $it->valid());
36+
assertType('int|null', $it->current());
37+
38+
foreach ($it as $v) {
39+
assertType('true', $it->valid());
40+
assertType('int', $it->current());
41+
}
42+
43+
assertType('false', $it->valid());
44+
assertType('null', $it->current());
45+
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ function doFoo() {
4141

4242
$iterator = new FooFilterIterator($generator());
4343

44-
assertType('array{}|bool|stdClass', $iterator->key());
45-
assertType('array{}|bool|stdClass', $iterator->current());
44+
assertType('array{}|bool|stdClass|null', $iterator->key());
45+
assertType('array{}|bool|stdClass|null', $iterator->current());
4646

4747
$generator = static function (): Generator {
4848
yield true => true;
@@ -51,6 +51,6 @@ function doFoo() {
5151

5252
$iterator = new FooFilterIterator($generator());
5353

54-
assertType('bool', $iterator->key());
55-
assertType('bool', $iterator->current());
54+
assertType('bool|null', $iterator->key());
55+
assertType('bool|null', $iterator->current());
5656
}

‎turbo-ext/src/ForeachHandler.cpp‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ zend_string *pt_feh_identical_narrowing_helper_class = nullptr;
6969
zend_string *pt_feh_empty_or_object_narrowing_helper_class = nullptr;
7070
zend_string *pt_feh_traversable_class = nullptr;
7171
zend_string *pt_feh_iterator_aggregate_class = nullptr;
72+
zend_string *pt_feh_iterator_class = nullptr;
73+
zend_string *pt_feh_valid = nullptr;
7274
zend_string *pt_feh_get_iterator = nullptr;
7375

7476
/* $node->getAttributes() / ->setAttributes($attributes) */
@@ -374,6 +376,8 @@ class ForeachHandler
374376
if (UNEXPECTED(foreachIterateeType.isUndef())) return zv::Val();
375377
zv::Val foreachNativeIterateeType = pt_expression_result_get_native_type(condResult.raw());
376378
if (UNEXPECTED(foreachNativeIterateeType.isUndef())) return zv::Val();
379+
zv::Val iteratorValidExpr = newIteratorValidExpr(iteratee, foreachIterateeType.raw());
380+
if (UNEXPECTED(iteratorValidExpr.isUndef())) return zv::Val();
377381

378382
if (parts.keyVarIsVariable) {
379383
zv::Val keyTypeExpr = newIterableTypeExpr(originalScope.raw(), foreachIterateeType.raw(), foreachNativeIterateeType.raw(), true);
@@ -458,6 +462,10 @@ class ForeachHandler
458462
if (UNEXPECTED(storage.isUndef())) return zv::Val();
459463

460464
originalScope = zv::Val::copyOf(nonEmptyIterateeScope.ref());
465+
if (!iteratorValidExpr.isNull()) {
466+
originalScope = pt_mutating_scope_filter_by_value(Z_OBJ_P(originalScope.raw()), Z_OBJ_P(iteratorValidExpr.raw()), true);
467+
if (UNEXPECTED(originalScope.isUndef())) return zv::Val();
468+
}
461469
// $originalScope may narrow the iteratee to a non-empty array - a genuinely
462470
// different scope than its own. The narrowing is tracked by the scope
463471
// (getTypeOnScope's authoritative read), so the iteratee only needs
@@ -506,6 +514,10 @@ class ForeachHandler
506514
zv::Val prevScope = zv::Val::copyOf(bodyScope.ref());
507515
bodyScope = pt_mutating_scope_merge_with(Z_OBJ_P(bodyScope.raw()), nonEmptyIterateeScope.raw());
508516
if (UNEXPECTED(bodyScope.isUndef())) return zv::Val();
517+
if (!iteratorValidExpr.isNull()) {
518+
bodyScope = pt_mutating_scope_filter_by_value(Z_OBJ_P(bodyScope.raw()), Z_OBJ_P(iteratorValidExpr.raw()), true);
519+
if (UNEXPECTED(bodyScope.isUndef())) return zv::Val();
520+
}
509521
if (!prevEntryScope.isNull()) {
510522
bool equal;
511523
if (UNEXPECTED(!ptlh::scopesEqual(bodyScope.raw(), prevEntryScope.raw(), equal))) return zv::Val();
@@ -579,6 +591,10 @@ class ForeachHandler
579591

580592
bodyScope = pt_mutating_scope_merge_with(Z_OBJ_P(bodyScope.raw()), nonEmptyIterateeScope.raw());
581593
if (UNEXPECTED(bodyScope.isUndef())) return zv::Val();
594+
if (!iteratorValidExpr.isNull()) {
595+
bodyScope = pt_mutating_scope_filter_by_value(Z_OBJ_P(bodyScope.raw()), Z_OBJ_P(iteratorValidExpr.raw()), true);
596+
if (UNEXPECTED(bodyScope.isUndef())) return zv::Val();
597+
}
582598
zv::Val finalEntryScope = zv::Val::copyOf(bodyScope.ref());
583599
zval *lastPass = lastRecordedPass(previousPass);
584600
zval *storage = originalStorage;
@@ -806,6 +822,12 @@ class ForeachHandler
806822
finalScope = std::move(assigned);
807823
}
808824

825+
if (!iteratorValidExpr.isNull()) {
826+
zv::Val filtered = pt_mutating_scope_filter_by_value(Z_OBJ_P(finalScope.raw()), Z_OBJ_P(iteratorValidExpr.raw()), false);
827+
if (UNEXPECTED(filtered.isUndef())) return zv::Val();
828+
finalScope = std::move(filtered);
829+
}
830+
809831
zv::Val bindingFlow;
810832
{
811833
zval flows[5];
@@ -1777,6 +1799,35 @@ class ForeachHandler
17771799
return pt_mutating_scope_assign_variable(Z_OBJ_P(scope), name, type, nativeType, pt_trinary_singleton(PT_TRI_YES));
17781800
}
17791801

1802+
/* processStmt()'s `$iteratorValidExpr`: `$iteratee->valid() === true` when
1803+
* the iterated type is always an Iterator, null otherwise, UNDEF = pending
1804+
* exception */
1805+
zv::Val newIteratorValidExpr(zval *iteratee, zval *iterateeType) const
1806+
{
1807+
zv::Val iteratorType;
1808+
{
1809+
zval out;
1810+
if (UNEXPECTED(!pt_object_type_new(&out, pt_feh_iterator_class))) return zv::Val();
1811+
iteratorType = zv::Val::adopt(out);
1812+
}
1813+
zend_long superType = isSuperTypeOf(iteratorType.raw(), iterateeType);
1814+
if (UNEXPECTED(superType < 0)) return zv::Val();
1815+
if (superType != PT_TRI_YES) return zv::Val::null();
1816+
1817+
zv::Args methodCallArgv{iteratee, pt_feh_valid};
1818+
zv::Val methodCall = pt_type_new(PT_CLASS_METHOD_CALL, 2, methodCallArgv);
1819+
if (UNEXPECTED(methodCall.isUndef())) return zv::Val();
1820+
zval name;
1821+
ZVAL_STRINGL(&name, "true", 4);
1822+
zv::Val nameValue = zv::Val::adopt(name);
1823+
zv::Val fullyQualified = pt_name_node_new(PT_CLASS_FULLY_QUALIFIED, nameValue.raw());
1824+
if (UNEXPECTED(fullyQualified.isUndef())) return zv::Val();
1825+
zv::Val constFetch = pt_type_new(PT_CLASS_CONST_FETCH, 1, fullyQualified.raw());
1826+
if (UNEXPECTED(constFetch.isUndef())) return zv::Val();
1827+
zv::Args identicalArgv{methodCall.raw(), constFetch.raw()};
1828+
return pt_type_new(PT_CLASS_IDENTICAL_EXPR, 2, identicalArgv);
1829+
}
1830+
17801831
/* Mirrors the private getTraversableForeachThrowPoint(); the throw point
17811832
* or null, UNDEF = pending exception */
17821833
zv::Val getTraversableForeachThrowPoint(zval *scope, zval *iteratee, zval *exprType) const
@@ -1994,6 +2045,8 @@ PT_MINIT_REGISTRATION(pt_register_foreach_handler)
19942045
pt_feh_empty_or_object_narrowing_helper_class = zend_string_init_interned(PT_LC("PHPStan\\Analyser\\ExprHandler\\Helper\\ForeachEmptyOrObjectNarrowingHelper"), 1);
19952046
pt_feh_traversable_class = zend_string_init_interned(PT_LC("Traversable"), 1);
19962047
pt_feh_iterator_aggregate_class = zend_string_init_interned(PT_LC("IteratorAggregate"), 1);
2048+
pt_feh_iterator_class = zend_string_init_interned(PT_LC("Iterator"), 1);
2049+
pt_feh_valid = zend_string_init_interned(PT_LC("valid"), 1);
19972050
pt_feh_get_iterator = zend_string_init_interned(PT_LC("getIterator"), 1);
19982051

19992052
reg::Class cls("PHPStan\\Analyser\\StmtHandler\\ForeachHandler");

0 commit comments

Comments
 (0)