Fix phpstan/phpstan#6830: Variable inside loop might not be defined.#5130
Closed
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Closed
Fix phpstan/phpstan#6830: Variable inside loop might not be defined.#5130phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
- Fixed intersectConditionalExpressions() in MutatingScope to merge conditional expression holders with same conditions but different result types - Previously, when a variable was conditionally defined before a loop and used inside the loop under the same condition, the conditional expression was lost during scope merging because the result type changed across iterations - Added fallback logic: when exact key matching fails, match holders by their condition expressions and merge result types via TypeCombinator::union() - Extended regression test in tests/PHPStan/Rules/Variables/data/bug-6830.php
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
When a variable is conditionally defined before a loop and used inside the loop under the same condition, PHPStan incorrectly reports "Variable $x might not be defined." This happens because the conditional expression tracking ("if $do is true then $x is defined") is lost during loop scope merging when the variable's type changes across iterations.
Changes
MutatingScope::intersectConditionalExpressions()insrc/Analyser/MutatingScope.phpto add a fallback merging path when exact key matching fails$do=true) but different result types (e.g.,int(9999)vsint(123)), they are now merged by taking the union of result types and the minimum certaintytests/PHPStan/Rules/Variables/data/bug-6830.phpwith the original issue's reproduction case (variable defined before loop, used inside loop)Root cause
intersectConditionalExpressions()usesConditionalExpressionHolder::getKey()for matching, which includes the result type in the key string (e.g.,$do=true => 9999 (Yes)). When a variable's type changes across loop iterations (e.g.,$xgoes from9999to123), the keys differ and the intersection drops the conditional expression entirely. This means the knowledge that "$do being true implies $x is defined" is lost, causing the false positive.The fix adds a fallback: when exact key matching fails, it matches holders by their condition expressions only, then merges the result types using
TypeCombinator::union()and takes the minimum certainty viaTrinaryLogic::maxMin().Test
Extended the existing
tests/PHPStan/Rules/Variables/data/bug-6830.phpwith two additional test functions:test2(): reproduces the original issue — variable$xdefined conditionally before a foreach loop, reassigned inside the loop under the same conditiontest3(): simpler variant — variable$xdefined conditionally before a foreach loop, read inside the loop under the same conditionBoth expect no errors (previously
test2reported "Variable $x might not be defined" on line 28).Fixes phpstan/phpstan#6830