Skip to content

Commit a153f57

Browse files
phpstan-botclaude
authored and
ondrejmirtes
committed
Re-price conditional holder expressions whose stored result was walked on a branch narrowing that expression
- ExpressionResult::answersOnScope() now returns false when the result's own evaluation position (beforeScope) tracked a narrowing of this very expression that the asking scope does not carry - e.g. the `$a['k']` in `$a['k'] === null || is_int($a['k'])` was walked where `$a['k']` is `mixed~null`, so its memoized type is not valid after the `||`. - AssignHandler::currentTypeForConditionalHolder() only reads the stored result when it answersOnScope(), otherwise it reads scope state / prices the expression on demand. Previously a stored boolean such as `$ok = $a['k'] === null || is_int($a['k'])` produced a conditional holder `$a['k']: int` instead of `int|null`. - Same change mirrored in turbo-ext/src/ExpressionResult.cpp and turbo-ext/src/AssignHandler.cpp. - Also fixed by the same change (covered in the test): the falsey branch of `$ok = $a['k'] !== null && !is_int($a['k'])`, ternary-assigned conditions, nested offsets `$a['k']['j']`, `is_string(..) || is_int(..)` and `$a['k'] === 1 || is_string($a['k'])`. - Probed and already fine: plain variables and instance property fetches. Static property fetches are not projected through stored booleans at all (unchanged before/after, not a regression). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 4ef9ab7 commit a153f57

5 files changed

Lines changed: 111 additions & 3 deletions

File tree

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2170,8 +2170,10 @@ private function currentTypeForConditionalHolder(NodeScopeResolver $nodeScopeRes
21702170
// them from the walk's own storage (the scope's storage stack misses it
21712171
// on loop-convergence passes); synthetic terms narrowing extensions built
21722172
// (@phpstan-assert property fetches etc.) answer from scope state or a walk
2173+
// - unless the walk read them on a narrowed branch (the right operand of
2174+
// `$a['k'] === null || is_int($a['k'])`), then they are re-priced here
21732175
$storedResult = $storage->findExpressionResult($expr);
2174-
if ($storedResult !== null) {
2176+
if ($storedResult !== null && $storedResult->answersOnScope($scope, $scope->nativeTypesPromoted)) {
21752177
return $storedResult->getTypeOnScope($scope, $scope->nativeTypesPromoted);
21762178
}
21772179

‎src/Analyser/ExpressionResult.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,19 @@ public function answersOnScope(MutatingScope $scope, bool $useNativeTypes): bool
699699
$useNativeTypes = $useNativeTypes || $scope->nativeTypesPromoted;
700700
$readScope = $useNativeTypes ? $scope->doNotTreatPhpDocTypesAsCertain() : $scope;
701701

702-
return $this->isScopeAuthoritative($readScope) || $this->askScopeVariableStateMatches($scope, $useNativeTypes);
702+
if ($this->isScopeAuthoritative($readScope)) {
703+
return true;
704+
}
705+
706+
// the walk position narrowed this very expression (e.g. the right
707+
// operand of `$a['k'] === null || is_int($a['k'])`) - a position not
708+
// carrying that narrowing must not see it through the memoized type
709+
$positionScope = $useNativeTypes ? $this->beforeScope->doNotTreatPhpDocTypesAsCertain() : $this->beforeScope;
710+
if ($this->isScopeAuthoritative($positionScope)) {
711+
return false;
712+
}
713+
714+
return $this->askScopeVariableStateMatches($scope, $useNativeTypes);
703715
}
704716

705717
/**

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?php // lint >= 8.0
2+
3+
namespace Bug15479;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
/** @param array<string, mixed> $a */
8+
function f(array $a): void {
9+
$ok = array_key_exists('k', $a) && ($a['k'] === null || is_int($a['k']));
10+
if ($ok) { assertType('int|null', $a['k']); }
11+
}
12+
13+
/** @param array<string, mixed> $a */
14+
function g(array $a): void {
15+
$ok = ($a['k'] === null || is_int($a['k']));
16+
if ($ok) { assertType('int|null', $a['k']); }
17+
}
18+
19+
/** @param array<string, mixed> $a */
20+
function h(array $a): void {
21+
if (array_key_exists('k', $a) && ($a['k'] === null || is_int($a['k']))) { assertType('int|null', $a['k']); }
22+
}
23+
24+
function i(mixed $v): void {
25+
$ok = $v === null || is_int($v);
26+
if ($ok) { assertType('int|null', $v); }
27+
}
28+
29+
/** @param array<string, mixed> $a */
30+
function negated(array $a): void {
31+
$ok = ($a['k'] !== null && !is_int($a['k']));
32+
if (!$ok) {
33+
assertType('int|null', $a['k']);
34+
} else {
35+
assertType('mixed~(int|null)', $a['k']);
36+
}
37+
}
38+
39+
/** @param array<string, mixed> $a */
40+
function ternary(array $a): void {
41+
$ok = $a['k'] === null || is_int($a['k']) ? 1 : 0;
42+
if ($ok === 1) { assertType('int|null', $a['k']); }
43+
}
44+
45+
/** @param array<string, array<string, mixed>> $a */
46+
function nested(array $a): void {
47+
$ok = ($a['k']['j'] === null || is_int($a['k']['j']));
48+
if ($ok) { assertType('int|null', $a['k']['j']); }
49+
}
50+
51+
/** @param array<string, mixed> $a */
52+
function typeChecks(array $a): void {
53+
$ok = (is_string($a['k']) || is_int($a['k']));
54+
if ($ok) { assertType('int|string', $a['k']); }
55+
}
56+
57+
/** @param array<string, mixed> $a */
58+
function constantOrType(array $a): void {
59+
$ok = ($a['k'] === 1 || is_string($a['k']));
60+
if ($ok) { assertType('1|string', $a['k']); }
61+
}
62+
63+
/** @param array<string, mixed> $a */
64+
function elseBranch(array $a): void {
65+
$ok = ($a['k'] === null || is_string($a['k']));
66+
if ($ok) {
67+
assertType('string|null', $a['k']);
68+
} else {
69+
assertType('mixed~(string|null)', $a['k']);
70+
}
71+
}

‎turbo-ext/src/AssignHandler.cpp‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2489,8 +2489,16 @@ class AssignHandler
24892489
// them from the walk's own storage (the scope's storage stack misses it
24902490
// on loop-convergence passes); synthetic terms narrowing extensions built
24912491
// (@phpstan-assert property fetches etc.) answer from scope state or a walk
2492+
// - unless the walk read them on a narrowed branch (the right operand of
2493+
// `$a['k'] === null || is_int($a['k'])`), then they are re-priced here
24922494
AH_VAL(storedResult, pt_expression_result_storage_find(storage, expr));
2493-
if (!storedResult.isNull()) return ahTypeOnScope(storedResult.raw(), scope);
2495+
if (!storedResult.isNull()) {
2496+
bool promoted;
2497+
AH_OK(ahPromoted(scope, promoted));
2498+
bool answers;
2499+
AH_OK(pt_expression_result_answers_on_scope(storedResult.raw(), scope, promoted, answers));
2500+
if (answers) return ahTypeOnScope(storedResult.raw(), scope);
2501+
}
24942502

24952503
return nsrReadScopeStateOrSyntheticType(nsr, expr, scope);
24962504
}

‎turbo-ext/src/ExpressionResult.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,21 @@ class ExpressionResult
600600

601601
zend_long authoritative = isScopeAuthoritative(readScope);
602602
if (authoritative != 0) return authoritative;
603+
604+
// the walk position narrowed this very expression (e.g. the right
605+
// operand of `$a['k'] === null || is_int($a['k'])`) - a position not
606+
// carrying that narrowing must not see it through the memoized type
607+
zv::Val positionScopeHolder;
608+
zval *positionScope = slot(slots::beforeScope);
609+
if (useNativeTypes) {
610+
positionScopeHolder = scopeNativeView(positionScope);
611+
if (UNEXPECTED(positionScopeHolder.isUndef())) return -1;
612+
positionScope = positionScopeHolder.raw();
613+
}
614+
zend_long positionAuthoritative = isScopeAuthoritative(positionScope);
615+
if (positionAuthoritative < 0) return -1;
616+
if (positionAuthoritative == 1) return 0;
617+
603618
return askScopeVariableStateMatches(scope, useNativeTypes, false);
604619
}
605620

0 commit comments

Comments
 (0)