Expose the loop that overwrites a variable still in use - #6414
Merged
Merged
Conversation
phpstan-strict-rules reports a foreach/for that reuses a variable name. Since definedness became precise, it also fires on reusing a spent loop variable. The rule needs to know whether the loop takes over a variable that was assigned before it and is read after it - a liveness question. VariableWritesNode::getVariableOverwritingLoop() answers it for foreach key/value bindings and for-loop initial assignments: the statement's flow is wrapped, and a probe in the live set follows the variable backwards through the loop (the loop's own bindings and updates let it through, any other write kills it) and records the assignment before the loop that the binding replaces. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LYiPGg9cpsKLyK6X5BrJTT
Collaborator
|
You've opened the pull request against the latest branch 2.3.x. PHPStan 2.3 is not going to be released for months. If your code is relevant on 2.2.x and you want it to be released sooner, please rebase your pull request and change its target to 2.2.x. |
This was referenced Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Needed by phpstan/phpstan-strict-rules#332 (also phpstan/phpstan#9940).
OverwriteVariablesWithForeachRulefires on any foreach whose key/value variable is already defined. Since #6380 tracks definedness precisely, that includes reusing a spent loop variable after a flag-then-check (if (!$found) return;). The rule needs a liveness answer: does the loop take over a variable that was assigned before it and is read after it, with nothing but the loop's own writes in between?This adds
VariableWritesNode::getVariableOverwritingLoop(VariableWrite $write): Foreach_|For_|null, answering exactly that for foreach key/value bindings and for-loop initial assignments (list targets included).How it works:
ForeachHandler/ForHandlerwrap the statement's flow in a newVariableFlow::loopStatement()carrying the head bindings and the statement's own writes (bindings plus a for-loop's update, e.g.$i++).VariableLivenessResolver, when the bound variable is live after the statement, a probe key (a\0-prefixed key no read can produce) enters the live set. It travels backwards through the statement: own writes let it through, other whole-variable writes kill it. Surviving to the statement entry it is armed; the first earlier assignment, by-reference alias, or offset write of the variable then records the loop.unset()kills it without recording.Examples (see the fixture):
foreach ($a as $x) {} foreach ($b as $x) {}— silent$found = false; foreach (...) { ... break; } if (!$found) return; foreach ($a as $x) {}— silentforeach ($outer as $x) { foreach ($inner as $x) {} use($x); }— inner loop reportedfunction (string $x) { foreach ($a as $x) {} echo $x; }— reportedfor ($i = 0; ...) {} for ($i = 0; ...) {}— silent; reported only if$iis read after the second loopStrict-rules side follows once this is in the dev phar.
🤖 Generated with Claude Code
https://claude.ai/code/session_01LYiPGg9cpsKLyK6X5BrJTT