Repository navigation
Re-price conditional holder expressions whose stored result was walked on a branch narrowing that expression - #6828
Open
phpstan-bot wants to merge 1 commit into
Conversation
…d 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>
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
Since 2.3.0, storing
$a['k'] === null || is_int($a['k'])in a variable and branching on it narrowed$a['k']toint, droppingnull. The same condition written inline, or on a plain variable, was fine. This change makes the conditional expression holders recorded for the stored boolean use the type of$a['k']at the assignment position, not the type memoized while walking the||right operand.Changes
src/Analyser/ExpressionResult.php:answersOnScope()returnsfalsewhen the result's evaluation position (beforeScope) tracked a narrowing of the expression itself that the asking scope does not have.src/Analyser/ExprHandler/AssignHandler.php:currentTypeForConditionalHolder()reads the stored result only whenanswersOnScope()holds; otherwise it falls back toreadScopeStateOrSyntheticType().turbo-ext/src/ExpressionResult.cpp,turbo-ext/src/AssignHandler.cpp: native mirrors of both changes.$ok = $a['k'] !== null && !is_int($a['k'])$ok = $a['k'] === null || is_int($a['k']) ? 1 : 0$a['k']['j']is_string($a['k']) || is_int($a['k'])$a['k'] === 1 || is_string($a['k'])Root cause
currentTypeForConditionalHolder()takes the "current" type of each narrowed expression from the walk's storedExpressionResult. For$a['k'], the stored result found is the one from the||right operand. That operand was evaluated on the falsey scope of$a['k'] === null, so its type ismixed~null.answersOnScope()/getTypeOnScope()only compared the variables the expression reads ($a, unchanged), so they accepted the memoizedmixed~null. Intersecting it with the sure typeint|nullproducedint. The fix makesanswersOnScope()also reject the memoized answer when the evaluation position narrowed the expression itself, so the holder re-prices it on the post-assignment scope.Test
tests/PHPStan/Analyser/nsrt/bug-15479.phpcontains the issue's four functions verbatim (withassertType), plus the analogous cases listed above. It fails without the fix and passes with it, with and without the turbo extension loaded.Fixes phpstan/phpstan#15479
🤖 Generated with Claude Code