Skip to content

Commit a491b72

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Do not record certainty-No conditionals for expressions that cannot be undefined
When a branch merge finds an expression tracked only in the other branch, createConditionalExpressions() recorded "if guard then the expression has certainty No" for every guard. For a variable that means it is undefined in our branch. Any other expression (a property fetch, a method call, a static property) is merely untracked there - the conditional carries no information, and once its guard matched later it discarded a narrowing made after the merge, so `$item->key !== null && !$resolve` read the declared int|null again. Set-difference guards (d6831e9) made more guards qualify and so exposed this; only variables are now recorded as undefined targets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DEjx9cAdSfGZDQdTWKaX64
1 parent f064084 commit a491b72

4 files changed

Lines changed: 182 additions & 56 deletions

File tree

‎src/Analyser/ScopeOps.php‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,16 @@ public static function intersectConditionalExpressions(array $ourConditionalExpr
358358
return $newConditionalExpressions;
359359
}
360360

361+
/**
362+
* Only a variable can be undefined. Any other expression missing from a
363+
* branch is just untracked there - a certainty-No conditional for it would,
364+
* once its guard matches, discard a narrowing made after the merge.
365+
*/
366+
private static function canBeUndefined(Expr $expr): bool
367+
{
368+
return $expr instanceof Variable && is_string($expr->name);
369+
}
370+
361371
/**
362372
* Mirrors the former MutatingScope::createConditionalExpressions().
363373
*
@@ -391,7 +401,7 @@ public static function createConditionalExpressions(
391401
if (!array_key_exists($exprString, $newVariableTypes)) {
392402
if (
393403
array_key_exists($exprString, $mergedExpressionTypes)
394-
&& !$mergedExpressionTypes[$exprString]->getExpr() instanceof VirtualNode
404+
&& self::canBeUndefined($mergedExpressionTypes[$exprString]->getExpr())
395405
) {
396406
$hasUndefinedTarget = true;
397407
}
@@ -579,7 +589,7 @@ public static function createConditionalExpressions(
579589
if (array_key_exists($exprString, $ourExpressionTypes)) {
580590
continue;
581591
}
582-
if ($mergedExprTypeHolder->getExpr() instanceof VirtualNode) {
592+
if (!self::canBeUndefined($mergedExprTypeHolder->getExpr())) {
583593
continue;
584594
}
585595

‎tests/PHPStan/Analyser/nsrt/conditional-untracked-expression-target.php‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
<?php
2+
3+
namespace ConditionalUntrackedExpressionTarget;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
final class Item
8+
{
9+
10+
public ?int $key = null;
11+
12+
public static ?string $name = null;
13+
14+
public function getKey(): ?int
15+
{
16+
return $this->key;
17+
}
18+
19+
}
20+
21+
function g(int $i): void
22+
{
23+
}
24+
25+
function propertyFetch(Item $item): void
26+
{
27+
if (rand(0, 1)) {
28+
$a = 1;
29+
} else {
30+
$a = 2;
31+
if ($item->key === null) {
32+
return;
33+
}
34+
}
35+
36+
if ($item->key !== null) {
37+
if ($a === 1) {
38+
assertType('int', $item->key);
39+
}
40+
if ($a === 2) {
41+
assertType('int', $item->key);
42+
}
43+
}
44+
if ($a === 2) {
45+
assertType('int', $item->key);
46+
}
47+
if ($a === 1) {
48+
assertType('int|null', $item->key);
49+
}
50+
}
51+
52+
function narrowedByTheSameCondition(Item $item, bool $resolve): void
53+
{
54+
if ($item->key !== null && $resolve) {
55+
g($item->key);
56+
}
57+
if ($item->key !== null && !$resolve) {
58+
assertType('int', $item->key);
59+
}
60+
}
61+
62+
function otherExpressions(Item $item, array $arr): void
63+
{
64+
if (rand(0, 1)) {
65+
$a = 1;
66+
} else {
67+
$a = 2;
68+
if ($item->getKey() === null || Item::$name === null || !isset($arr['k'])) {
69+
return;
70+
}
71+
}
72+
73+
if ($item->getKey() !== null && $a === 1) {
74+
assertType('int', $item->getKey());
75+
}
76+
if (Item::$name !== null && $a === 1) {
77+
assertType('string', Item::$name);
78+
}
79+
if (isset($arr['k']) && $a === 1) {
80+
assertType('mixed~null', $arr['k']);
81+
}
82+
}
83+
84+
function variableStaysUndefined(?int $key): void
85+
{
86+
if (rand(0, 1)) {
87+
$a = 1;
88+
} else {
89+
$a = 2;
90+
$x = $key;
91+
}
92+
93+
if ($a === 1) {
94+
assertType('*ERROR*', $x);
95+
}
96+
}

‎turbo-ext/src/ScopeOps.cpp‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,18 @@ class ScopeOps
445445
return zv::Val(std::move(result));
446446
}
447447

448+
/* Mirrors ScopeOps::canBeUndefined(). Returns false with an exception set
449+
* when the node class info cannot be resolved. */
450+
static bool canBeUndefined(zend_object *expr, bool *out)
451+
{
452+
pt_node_class_info *info = pt_get_node_class_info(expr->ce);
453+
if (UNEXPECTED(info == NULL)) return false;
454+
*out = info->is_variable
455+
&& info->name_offset >= 0
456+
&& zv::ObjRef(expr).propAtOffset((uint32_t) info->name_offset).deref().isString();
457+
return true;
458+
}
459+
448460
/*
449461
* Mirrors ScopeOps::createConditionalExpressions(). The isSuperTypeOf /
450462
* isConstantArray results are cached per guard, exactly like the twin's
@@ -483,7 +495,9 @@ class ScopeOps
483495
if (mergedSlot != NULL) {
484496
zv::Ref mergedHolder = zv::Ref(mergedSlot).deref();
485497
if (UNEXPECTED(!pt_check_holder(mergedHolder.raw()))) return zv::Val();
486-
if (!instanceof_function(holderExpr(mergedHolder)->ce, virtualNodeCe)) {
498+
bool undefinable;
499+
if (UNEXPECTED(!canBeUndefined(holderExpr(mergedHolder), &undefinable))) return zv::Val();
500+
if (undefinable) {
487501
hasUndefinedTarget = true;
488502
}
489503
}
@@ -735,7 +749,9 @@ class ScopeOps
735749
if (pt_ht_exists(ours.table(), key, idx)) continue;
736750
zv::Ref mergedHolder = zv::Ref(mergedSlot).deref();
737751
if (UNEXPECTED(!pt_check_holder(mergedHolder.raw()))) return zv::Val();
738-
if (instanceof_function(holderExpr(mergedHolder)->ce, virtualNodeCe)) continue;
752+
bool undefinable;
753+
if (UNEXPECTED(!canBeUndefined(holderExpr(mergedHolder), &undefinable))) return zv::Val();
754+
if (!undefinable) continue;
739755

740756
for (auto guardEntry : zv::TableRef(typeGuards.table())) {
741757
zv::Val noHolder = createNoErrorHolder(zv::ObjRef(mergedHolder.asObject()).propAt(PT_ETH_PROP_EXPR).raw());

‎turbo-ext/src/generated/ScopeOps.h‎

Lines changed: 56 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -62,29 +62,30 @@ inline constexpr char strings[] =
6262
"ourConditionalExpressions\0" /* 663 */
6363
"theirConditionalExpressions\0" /* 689 */
6464
"intersectConditionalExpressions\0" /* 717 */
65-
"createConditionalExpressions\0" /* 749 */
66-
"exprStringToInvalidate\0" /* 778 */
67-
"invalidateMethodsOnExpression\0" /* 801 */
68-
"keyMayHideSubExpressions\0" /* 831 */
69-
"expr\0" /* 856 */
70-
"getIntertwinedRefRootVariableName\0" /* 861 */
71-
"expressionToInvalidate\0" /* 895 */
72-
"requireMoreCharacters\0" /* 918 */
73-
"invalidatingClass\0" /* 940 */
74-
"PHPStan\\Reflection\\ClassReflection\0" /* 958 */
75-
"keepPropertyFetches\0" /* 993 */
76-
"false\0" /* 1013 */
77-
"invalidateExpressionEntries\0" /* 1019 */
78-
"PHPStan\\Analyser\\Scope\0" /* 1047 */
79-
"PhpParser\\Node\0" /* 1070 */
80-
"expressionToInvalidateClass\0" /* 1085 */
81-
"containsExpressionToInvalidate\0" /* 1113 */
82-
"exprToInvalidate\0" /* 1144 */
83-
"null\0" /* 1161 */
84-
"shouldInvalidateExpression\0" /* 1166 */
85-
"isPropertyFetchChainOn\0" /* 1193 */
86-
"specifiedExpressions\0" /* 1216 */
87-
"matchConditionalExpressions"; /* 1237 */
65+
"expr\0" /* 749 */
66+
"canBeUndefined\0" /* 754 */
67+
"createConditionalExpressions\0" /* 769 */
68+
"exprStringToInvalidate\0" /* 798 */
69+
"invalidateMethodsOnExpression\0" /* 821 */
70+
"keyMayHideSubExpressions\0" /* 851 */
71+
"getIntertwinedRefRootVariableName\0" /* 876 */
72+
"expressionToInvalidate\0" /* 910 */
73+
"requireMoreCharacters\0" /* 933 */
74+
"invalidatingClass\0" /* 955 */
75+
"PHPStan\\Reflection\\ClassReflection\0" /* 973 */
76+
"keepPropertyFetches\0" /* 1008 */
77+
"false\0" /* 1028 */
78+
"invalidateExpressionEntries\0" /* 1034 */
79+
"PHPStan\\Analyser\\Scope\0" /* 1062 */
80+
"PhpParser\\Node\0" /* 1085 */
81+
"expressionToInvalidateClass\0" /* 1100 */
82+
"containsExpressionToInvalidate\0" /* 1128 */
83+
"exprToInvalidate\0" /* 1159 */
84+
"null\0" /* 1176 */
85+
"shouldInvalidateExpression\0" /* 1181 */
86+
"isPropertyFetchChainOn\0" /* 1208 */
87+
"specifiedExpressions\0" /* 1231 */
88+
"matchConditionalExpressions"; /* 1252 */
8889
inline constexpr reg::PackedArg args[] = {
8990
reg::packed(0, 0, 5), /* nodeKey $node */
9091
reg::packed(25, 0, 37), /* nodeKey $exprPrinter */
@@ -127,54 +128,56 @@ inline constexpr reg::PackedArg args[] = {
127128
reg::packed(663, MAY_BE_ARRAY), /* intersectConditionalExpressions $ourConditionalExpressions */
128129
reg::packed(689, MAY_BE_ARRAY), /* intersectConditionalExpressions $theirConditionalExpressions */
129130
reg::packed(70, MAY_BE_ARRAY), /* intersectConditionalExpressions return */
131+
reg::packed(749, 0, 5), /* canBeUndefined $expr */
132+
reg::packed(70, MAY_BE_BOOL), /* canBeUndefined return */
130133
reg::packed(292, MAY_BE_ARRAY), /* createConditionalExpressions $conditionalExpressions */
131134
reg::packed(559, MAY_BE_ARRAY), /* createConditionalExpressions $ourExpressionTypes */
132135
reg::packed(578, MAY_BE_ARRAY), /* createConditionalExpressions $theirExpressionTypes */
133136
reg::packed(537, MAY_BE_ARRAY), /* createConditionalExpressions $mergedExpressionTypes */
134137
reg::packed(499, MAY_BE_ARRAY), /* createConditionalExpressions $differingKeys */
135138
reg::packed(70, MAY_BE_ARRAY), /* createConditionalExpressions return */
136139
reg::packed(25, 0, 37), /* invalidateMethodsOnExpression $exprPrinter */
137-
reg::packed(778, MAY_BE_STRING), /* invalidateMethodsOnExpression $exprStringToInvalidate */
140+
reg::packed(798, MAY_BE_STRING), /* invalidateMethodsOnExpression $exprStringToInvalidate */
138141
reg::packed(254, MAY_BE_ARRAY), /* invalidateMethodsOnExpression $expressionTypes */
139142
reg::packed(270, MAY_BE_ARRAY), /* invalidateMethodsOnExpression $nativeExpressionTypes */
140143
reg::packed(70, MAY_BE_NULL | MAY_BE_ARRAY), /* invalidateMethodsOnExpression return */
141144
reg::packed(155, MAY_BE_STRING), /* keyMayHideSubExpressions $exprString */
142145
reg::packed(70, MAY_BE_BOOL), /* keyMayHideSubExpressions return */
143-
reg::packed(856, 0, 5), /* getIntertwinedRefRootVariableName $expr */
146+
reg::packed(749, 0, 5), /* getIntertwinedRefRootVariableName $expr */
144147
reg::packed(70, MAY_BE_NULL | MAY_BE_STRING), /* getIntertwinedRefRootVariableName return */
145148
reg::packed(79, 0, 85), /* invalidateExpressionEntries $scope */
146149
reg::packed(25, 0, 37), /* invalidateExpressionEntries $exprPrinter */
147-
reg::packed(778, MAY_BE_STRING), /* invalidateExpressionEntries $exprStringToInvalidate */
148-
reg::packed(895, 0, 5), /* invalidateExpressionEntries $expressionToInvalidate */
149-
reg::packed(918, MAY_BE_BOOL), /* invalidateExpressionEntries $requireMoreCharacters */
150-
reg::packed(940, MAY_BE_NULL, 958), /* invalidateExpressionEntries $invalidatingClass */
150+
reg::packed(798, MAY_BE_STRING), /* invalidateExpressionEntries $exprStringToInvalidate */
151+
reg::packed(910, 0, 5), /* invalidateExpressionEntries $expressionToInvalidate */
152+
reg::packed(933, MAY_BE_BOOL), /* invalidateExpressionEntries $requireMoreCharacters */
153+
reg::packed(955, MAY_BE_NULL, 973), /* invalidateExpressionEntries $invalidatingClass */
151154
reg::packed(254, MAY_BE_ARRAY), /* invalidateExpressionEntries $expressionTypes */
152155
reg::packed(270, MAY_BE_ARRAY), /* invalidateExpressionEntries $nativeExpressionTypes */
153156
reg::packed(292, MAY_BE_ARRAY), /* invalidateExpressionEntries $conditionalExpressions */
154-
reg::packed(993, MAY_BE_BOOL, reg::NoString, false, false, 1013), /* invalidateExpressionEntries $keepPropertyFetches */
157+
reg::packed(1008, MAY_BE_BOOL, reg::NoString, false, false, 1028), /* invalidateExpressionEntries $keepPropertyFetches */
155158
reg::packed(70, MAY_BE_NULL | MAY_BE_ARRAY), /* invalidateExpressionEntries return */
156-
reg::packed(79, 0, 1047), /* containsExpressionToInvalidate $scope */
159+
reg::packed(79, 0, 1062), /* containsExpressionToInvalidate $scope */
157160
reg::packed(25, 0, 37), /* containsExpressionToInvalidate $exprPrinter */
158-
reg::packed(0, 0, 1070), /* containsExpressionToInvalidate $node */
159-
reg::packed(1085, MAY_BE_STRING), /* containsExpressionToInvalidate $expressionToInvalidateClass */
160-
reg::packed(778, MAY_BE_STRING), /* containsExpressionToInvalidate $exprStringToInvalidate */
161+
reg::packed(0, 0, 1085), /* containsExpressionToInvalidate $node */
162+
reg::packed(1100, MAY_BE_STRING), /* containsExpressionToInvalidate $expressionToInvalidateClass */
163+
reg::packed(798, MAY_BE_STRING), /* containsExpressionToInvalidate $exprStringToInvalidate */
161164
reg::packed(70, MAY_BE_BOOL), /* containsExpressionToInvalidate return */
162165
reg::packed(79, 0, 85), /* shouldInvalidateExpression $scope */
163166
reg::packed(25, 0, 37), /* shouldInvalidateExpression $exprPrinter */
164-
reg::packed(778, MAY_BE_STRING), /* shouldInvalidateExpression $exprStringToInvalidate */
165-
reg::packed(1144, 0, 5), /* shouldInvalidateExpression $exprToInvalidate */
166-
reg::packed(856, 0, 5), /* shouldInvalidateExpression $expr */
167+
reg::packed(798, MAY_BE_STRING), /* shouldInvalidateExpression $exprStringToInvalidate */
168+
reg::packed(1159, 0, 5), /* shouldInvalidateExpression $exprToInvalidate */
169+
reg::packed(749, 0, 5), /* shouldInvalidateExpression $expr */
167170
reg::packed(155, MAY_BE_STRING), /* shouldInvalidateExpression $exprString */
168-
reg::packed(918, MAY_BE_BOOL, reg::NoString, false, false, 1013), /* shouldInvalidateExpression $requireMoreCharacters */
169-
reg::packed(940, MAY_BE_NULL, 958, false, false, 1161), /* shouldInvalidateExpression $invalidatingClass */
170-
reg::packed(993, MAY_BE_BOOL, reg::NoString, false, false, 1013), /* shouldInvalidateExpression $keepPropertyFetches */
171+
reg::packed(933, MAY_BE_BOOL, reg::NoString, false, false, 1028), /* shouldInvalidateExpression $requireMoreCharacters */
172+
reg::packed(955, MAY_BE_NULL, 973, false, false, 1176), /* shouldInvalidateExpression $invalidatingClass */
173+
reg::packed(1008, MAY_BE_BOOL, reg::NoString, false, false, 1028), /* shouldInvalidateExpression $keepPropertyFetches */
171174
reg::packed(70, MAY_BE_BOOL), /* shouldInvalidateExpression return */
172-
reg::packed(856, 0, 5), /* isPropertyFetchChainOn $expr */
173-
reg::packed(778, MAY_BE_STRING), /* isPropertyFetchChainOn $exprStringToInvalidate */
175+
reg::packed(749, 0, 5), /* isPropertyFetchChainOn $expr */
176+
reg::packed(798, MAY_BE_STRING), /* isPropertyFetchChainOn $exprStringToInvalidate */
174177
reg::packed(25, 0, 37), /* isPropertyFetchChainOn $exprPrinter */
175178
reg::packed(70, MAY_BE_BOOL), /* isPropertyFetchChainOn return */
176179
reg::packed(292, MAY_BE_ARRAY), /* matchConditionalExpressions $conditionalExpressions */
177-
reg::packed(1216, MAY_BE_ARRAY), /* matchConditionalExpressions $specifiedExpressions */
180+
reg::packed(1231, MAY_BE_ARRAY), /* matchConditionalExpressions $specifiedExpressions */
178181
reg::packed(70, MAY_BE_ARRAY), /* matchConditionalExpressions return */
179182
};
180183
using Sig = reg::Sig<strings, args>;
@@ -191,15 +194,16 @@ inline constexpr sigtab::Sig scopeWith = { { 441 /* scopeWith */, 9, 18, 9, 27,
191194
inline constexpr sigtab::Sig mergeVariableHolders = { { 516 /* mergeVariableHolders */, 2, 28, 3, 31, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
192195
inline constexpr sigtab::Sig finishMerge = { { 651 /* finishMerge */, 5, 32, 5, 37, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
193196
inline constexpr sigtab::Sig intersectConditionalExpressions = { { 717 /* intersectConditionalExpressions */, 2, 38, 2, 40, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
194-
inline constexpr sigtab::Sig createConditionalExpressions = { { 749 /* createConditionalExpressions */, 5, 41, 5, 46, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
195-
inline constexpr sigtab::Sig invalidateMethodsOnExpression = { { 801 /* invalidateMethodsOnExpression */, 4, 47, 4, 51, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
196-
inline constexpr sigtab::Sig keyMayHideSubExpressions = { { 831 /* keyMayHideSubExpressions */, 1, 52, 1, 53, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
197-
inline constexpr sigtab::Sig getIntertwinedRefRootVariableName = { { 861 /* getIntertwinedRefRootVariableName */, 1, 54, 1, 55, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
198-
inline constexpr sigtab::Sig invalidateExpressionEntries = { { 1019 /* invalidateExpressionEntries */, 9, 56, 10, 66, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
199-
inline constexpr sigtab::Sig containsExpressionToInvalidate = { { 1113 /* containsExpressionToInvalidate */, 5, 67, 5, 72, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
200-
inline constexpr sigtab::Sig shouldInvalidateExpression = { { 1166 /* shouldInvalidateExpression */, 6, 73, 9, 82, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
201-
inline constexpr sigtab::Sig isPropertyFetchChainOn = { { 1193 /* isPropertyFetchChainOn */, 3, 83, 3, 86, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
202-
inline constexpr sigtab::Sig matchConditionalExpressions = { { 1237 /* matchConditionalExpressions */, 2, 87, 2, 89, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
197+
inline constexpr sigtab::Sig canBeUndefined = { { 754 /* canBeUndefined */, 1, 41, 1, 42, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
198+
inline constexpr sigtab::Sig createConditionalExpressions = { { 769 /* createConditionalExpressions */, 5, 43, 5, 48, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
199+
inline constexpr sigtab::Sig invalidateMethodsOnExpression = { { 821 /* invalidateMethodsOnExpression */, 4, 49, 4, 53, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
200+
inline constexpr sigtab::Sig keyMayHideSubExpressions = { { 851 /* keyMayHideSubExpressions */, 1, 54, 1, 55, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
201+
inline constexpr sigtab::Sig getIntertwinedRefRootVariableName = { { 876 /* getIntertwinedRefRootVariableName */, 1, 56, 1, 57, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
202+
inline constexpr sigtab::Sig invalidateExpressionEntries = { { 1034 /* invalidateExpressionEntries */, 9, 58, 10, 68, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
203+
inline constexpr sigtab::Sig containsExpressionToInvalidate = { { 1128 /* containsExpressionToInvalidate */, 5, 69, 5, 74, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
204+
inline constexpr sigtab::Sig shouldInvalidateExpression = { { 1181 /* shouldInvalidateExpression */, 6, 75, 9, 84, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
205+
inline constexpr sigtab::Sig isPropertyFetchChainOn = { { 1208 /* isPropertyFetchChainOn */, 3, 85, 3, 88, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
206+
inline constexpr sigtab::Sig matchConditionalExpressions = { { 1252 /* matchConditionalExpressions */, 2, 89, 2, 91, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } };
203207
} // namespace sig
204208

205209
} // namespace ptdecl::ScopeOps

0 commit comments

Comments
 (0)