Skip to content

Commit 48d75ab

Browse files
ondrejmirtesclaude
andcommitted
Drop the foreach value alias on dynamic-variable writes to the iteratee
The value-variable alias projected onto $array[$key] is only sound while the loop body cannot mutate the iteratee at a foreign key. foreachAliasDesyncingTarget flagged such writes only when their target base was a statically named Variable equal to the iteratee, so a dynamic-variable base slipped through: $$name[$key + 1] (with $name = 'data') resolves to the iteratee at runtime and desyncs the live element from the snapshot value variable, yet the alias survived and projected a narrowed int onto $data[$key] whose sound type is int|string. Make the detector conservatively complete - the alias is a pure precision optimization, so any write target that cannot be proven to be exactly the iteratee at the current key (or a distinctly named non-iteratee location) drops it. A dim fetch whose base is a Variable with a non-string (Expr) name - a variable-variable ($$name[...]) or a curly dynamic variable (${'data'}[...]) - now desyncs, as does a bare dynamic-variable write ($$name = ...) that may target the key or value variable, and a dynamic global ($$name). The #7508 same-key sub-offset write ($array[$key][0] = ...) stays kept. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA
1 parent 63d438b commit 48d75ab

2 files changed

Lines changed: 96 additions & 10 deletions

File tree

‎src/Analyser/StmtHandler/ForeachHandler.php‎

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,12 @@ private function foreachAliasDesyncingNode(Node $node, string $iterateeName, str
695695

696696
if ($node instanceof Stmt\Global_) {
697697
foreach ($node->vars as $var) {
698-
if ($var instanceof Variable && is_string($var->name) && $var->name === $iterateeName) {
698+
if (!$var instanceof Variable) {
699+
continue;
700+
}
701+
// `global $iteratee` rebinds the iteratee to the global; a dynamic
702+
// `global $$name` may rebind it too - drop the alias in both cases
703+
if (!is_string($var->name) || $var->name === $iterateeName) {
699704
return true;
700705
}
701706
}
@@ -741,6 +746,15 @@ private function foreachAliasDesyncingNode(Node $node, string $iterateeName, str
741746
return false;
742747
}
743748

749+
/**
750+
* Conservatively complete: the value alias is a pure precision optimization,
751+
* so any write target the analyzer cannot PROVE is exactly $iteratee[$key]
752+
* (or a sub-offset of it) or a distinctly named non-iteratee location drops
753+
* the alias. In particular a dynamic-variable base ($$name / ${expr}) may
754+
* resolve to the iteratee, key or value variable at runtime, so it always
755+
* desyncs; only a same-key iteratee write ($iteratee[$key]...) and writes to
756+
* provably distinct plain variables / non-variable storage are kept.
757+
*/
744758
private function foreachAliasDesyncingTarget(Expr $target, string $iterateeName, string $keyName): bool
745759
{
746760
if ($target instanceof List_ || $target instanceof Array_) {
@@ -756,20 +770,40 @@ private function foreachAliasDesyncingTarget(Expr $target, string $iterateeName,
756770
return false;
757771
}
758772

759-
// whole-iteratee reassignment ($iteratee = ...)
760-
if ($target instanceof Variable && is_string($target->name) && $target->name === $iterateeName) {
761-
return true;
773+
if ($target instanceof Variable) {
774+
// a dynamic $$name / ${expr} write may target the iteratee (or the key
775+
// or value variable) - we cannot prove otherwise, so drop the alias
776+
if (!is_string($target->name)) {
777+
return true;
778+
}
779+
780+
// whole-iteratee reassignment ($iteratee = ...); a plain write to a
781+
// differently named variable (including a static write to the key or
782+
// value variable, which is severed through containment) does not desync
783+
// across iterations
784+
return $target->name === $iterateeName;
762785
}
763786

764-
// a write into the iteratee is safe only when the outermost access on the
765-
// iteratee variable uses exactly the current key ($iteratee[$key]...); any
766-
// other offset, or an append ($iteratee[]), may hit a future iteration's
767-
// element
787+
// A write into an array offset is safe only when it provably targets the
788+
// iteratee at exactly the current key ($iteratee[$key]...) - keeping the
789+
// #7508 same-key sub-offset case - or provably targets a distinctly named
790+
// plain variable. Peel the ArrayDimFetch layers to the base variable; a
791+
// dynamic-variable base ($$name[...]) that may be the iteratee, or an
792+
// iteratee write at any other offset / an append ($iteratee[]), drops the
793+
// alias. A non-variable base (a property, etc.) cannot rebind the plain
794+
// iteratee variable and is kept.
768795
$node = $target;
769796
while ($node instanceof ArrayDimFetch) {
770797
$inner = $node->var;
771-
if ($inner instanceof Variable && is_string($inner->name) && $inner->name === $iterateeName) {
772-
return !($node->dim instanceof Variable && is_string($node->dim->name) && $node->dim->name === $keyName);
798+
if ($inner instanceof Variable) {
799+
if (!is_string($inner->name)) {
800+
return true;
801+
}
802+
if ($inner->name === $iterateeName) {
803+
return !($node->dim instanceof Variable && is_string($node->dim->name) && $node->dim->name === $keyName);
804+
}
805+
806+
return false;
773807
}
774808

775809
$node = $inner;

‎tests/PHPStan/Analyser/nsrt/foreach-value-alias-soundness.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,58 @@ function dynamicWrite(array $data): void
4242
}
4343
}
4444

45+
/**
46+
* A dynamic-variable dim write ($$name is $data) into the iteratee at a foreign
47+
* key desyncs the live element from the snapshot value variable, exactly like a
48+
* statically named $data[$key + 1] write - the dynamic base must not slip past
49+
* the alias detector.
50+
*
51+
* @param array<int|string> $data
52+
*/
53+
function dynamicDimWrite(array $data): void
54+
{
55+
foreach ($data as $key => $value) {
56+
$name = 'data';
57+
$$name[$key + 1] = 'str';
58+
if (is_int($value)) {
59+
assertType('int|string', $data[$key]);
60+
}
61+
}
62+
}
63+
64+
/**
65+
* A dynamic write to the key variable ($$name is $key) means $data[$key] no
66+
* longer refers to the snapshot value's element.
67+
*
68+
* @param array<int|string> $data
69+
*/
70+
function dynamicKeyWrite(array $data): void
71+
{
72+
foreach ($data as $key => $value) {
73+
$name = 'key';
74+
$$name = 99;
75+
if (is_int($value)) {
76+
assertType('int|string', $data[$key]);
77+
}
78+
}
79+
}
80+
81+
/**
82+
* A curly-brace dynamic variable (${'data'}) is a Variable with an Expr name in
83+
* the AST, so a dim write through it must be treated as a possible iteratee write.
84+
*
85+
* @param array<int|string> $data
86+
*/
87+
function curlyDynamicDimWrite(array $data): void
88+
{
89+
foreach ($data as $key => $value) {
90+
${'data'}[$key + 1] = 'str';
91+
if (is_int($value)) {
92+
assertType('int|string', $data[$key]);
93+
}
94+
}
95+
}
96+
4597
/**
4698
* @param array<int|string> $data
4799
* @param array<string, mixed> $vars

0 commit comments

Comments
 (0)