Repository navigation
Make offsets written in every iteration of a non-unrolled constant-array foreach required after the loop - #6928
Open
phpstan-bot wants to merge 1 commit into
Conversation
…ray `foreach` required after the loop - Add ForeachIterationOffsetWritesHelper. After a foreach over a sealed constant array with no optional keys that was not unrolled (more than FOREACH_UNROLL_LIMIT entries), it makes every iterated key or value a required offset of `$target`. This applies when each back-edge scope (end of body and every `continue`) still tracks the unmodified loop variable and `$target[$loopVar]`. It also requires the body to touch `$target` only through offsets: no reassignment, unset, by-ref passing, closure use, variable-variables, extract or include. Offset writes never remove keys, so every iterated offset exists after the loop. - Supported write forms: `=`, compound assignments including `??=`, `=&`, nested `$t[$v][...] =` / `$t[$v][] =`, and list destructuring. - Analogous case: `$this->prop[$loopVar] = ...` gets the same treatment, but only when the body has no method/function calls, `new`, property hooks, include/eval or yield, because those can reset the property without the scope seeing it. - Wired into ForeachHandler and its turbo-ext mirror (ForeachHandler.cpp calls the PHP helper through a new class-ref entry). - Probed and left unchanged: loops with `break`, conditional writes, `continue` before the write, unsetting another key, reassigning the target or the loop variable, and passing the target by reference all still keep the keys optional.
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.
Summary
A
foreachover a constant array with more than 16 entries is not unrolled. So after$result[$value] = ...in the body, every key of$resultwas optional, and since 2.2.0 reading one of them reports "Offset … might not exist". With this change, the keys written in every iteration are required after the loop, the same as with 16 or fewer entries.Changes
src/Analyser/StmtHandler/ForeachIterationOffsetWritesHelper.php. On the non-unrolled path with nobreak, it makes the iterated keys or values required offsets of the written target.src/Analyser/StmtHandler/ForeachHandler.php: calls the helper with the back-edge scopes (end of body pluscontinuescopes) and the body's impure points.turbo-ext/src/ForeachHandler.cpp,turbo-ext/src/support.{h,cpp}: the native mirror makes the same call through a newforeachIterationOffsetWritesHelperclass ref.??=and other compound assignments,=&,$t[$v][] =,$t[$v]['x'] =,[$t[$v]] = ...$this->prop[$v] = ...targets, only when the body has no calls, which could reset the propertyRoot cause
The general loop analysis merges all iterations, so the loop variable is a union of every iterated value. Writing
$t[$union]can only add optional keys, and the information about which iteration wrote which key is lost. Only unrolling (capped at 16 keys) kept it.The fix proves key presence directly instead of raising the limit:
$t[$loopVar]set (checked in the back-edge scopes, with the loop variable unmodified);$tonly through offset writes, which never remove keys (checked syntactically);Test
tests/PHPStan/Analyser/nsrt/bug-15657.phpcovers:$thisproperty targets;continuebefore the write, conditional write,break, unset of another key, reassignment, by-ref passing, reassigned loop variable, method calls resetting a property.tests/PHPStan/Rules/Arrays/data/bug-15657.php: the issue's reproducer reports no "might not exist" error.Fixes phpstan/phpstan#15657