Fix phpstan/phpstan#14269: Regression: maybe undefined variable#5208
Merged
ondrejmirtes merged 2 commits intophpstan:2.1.xfrom Mar 12, 2026
Merged
Conversation
…nments - Fixed intersectConditionalExpressions to keep matching conditional expression keys instead of dropping all keys when any single key mismatches - The previous all-or-nothing behavior caused conditional expressions (e.g. for variable certainty based on method return values) to be entirely dropped when an unrelated conditional expression key was added during loop scope merging - New regression test in tests/PHPStan/Analyser/nsrt/bug-14269.php Closes phpstan/phpstan#14269
ondrejmirtes
requested changes
Mar 12, 2026
Member
ondrejmirtes
left a comment
There was a problem hiding this comment.
Please add a regression test for the snippet https://phpstan.org/r/dc24e5cb-ca3c-45f5-a49e-7425604e7ac9 from phpstan/phpstan#4284.
I want to check "If condition is always true" on line 23 so IfConstantConditionRuleTest.
Collaborator
Author
|
I processed this review but have nothing to report. |
1 similar comment
Collaborator
Author
|
I processed this review but have nothing to report. |
ondrejmirtes
approved these changes
Mar 12, 2026
This was referenced Mar 12, 2026
Collaborator
Author
|
I processed this review but have nothing to report. |
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
2 participants
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
Fixes a regression introduced in #5125 (fix for phpstan/phpstan#14117) where a variable assigned conditionally before a
foreachloop was incorrectly reported as "might not be defined" inside the loop, even when the same condition was re-checked.Changes
intersectConditionalExpressions()insrc/Analyser/MutatingScope.phpto keep only the conditional expression keys that are present in both scopes, instead of dropping ALL keys for a variable when any single key is missing from the other scopetests/PHPStan/Analyser/nsrt/bug-14269.phpRoot cause
The fix for phpstan/phpstan#14117 added logic in
createConditionalExpressions()to keep variables with same type but different certainty in the candidate set. This correctly creates conditional expressions for the certainty difference, but as a side effect, it also creates spurious conditional expressions based on unrelated type guards (e.g.,$availableIntervalstype was used as a guard for$packageDurationInMinutescertainty).During subsequent loop iteration scope merges,
intersectConditionalExpressions()required ALL conditional expression keys for a variable to be present in both scopes. When one scope had the spurious key and the other didn't, the method dropped ALL conditional expressions for that variable — including the correct ones based on the actual condition (isMultiDay()). This caused the variable to lose its conditional certainty tracking, resulting in a false "might not be defined" error.The fix changes
intersectConditionalExpressions()to keep only the keys that ARE in both scopes (a true intersection), rather than the previous all-or-nothing behavior.Test
Added
tests/PHPStan/Analyser/nsrt/bug-14269.phpwhich reproduces the exact scenario from the issue: a variable assigned insideif (!$package->isMultiDay()), then checked inside aforeachloop under the same condition, with an unrelated variable assignment ($availableIntervals = $s->subtract()) that triggers the bug.Fixes phpstan/phpstan#14269
Fixes phpstan/phpstan#4284