Skip to content

Fix phpstan/phpstan#6830: Variable inside loop might not be defined.#5185

Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-beqyr4f
Open

Fix phpstan/phpstan#6830: Variable inside loop might not be defined.#5185
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-beqyr4f

Conversation

@phpstan-bot
Copy link
Collaborator

Summary

When a variable is conditionally assigned before a foreach loop (e.g., if ($do) { $x = 9999; }) and then used inside the loop under the same condition (if ($do) { if ($x) { ... } }), PHPStan incorrectly reported "Variable $x might not be defined" even though the guarding condition guarantees the variable is defined. Additionally, PHPStan contradictorily reported "If condition is always true" on the same line.

Changes

  • Modified intersectConditionalExpressions in src/Analyser/MutatingScope.php to merge conditional expressions that have matching condition expressions but different result types, instead of dropping them entirely
  • Added helper method conditionExpressionHoldersMatch to compare condition expression type holders
  • Extended regression test in tests/PHPStan/Rules/Variables/data/bug-6830.php with the exact reproducing code from the issue

Root cause

During loop scope iteration in NodeScopeResolver, the body scope is merged with the original scope using mergeWith(). This calls intersectConditionalExpressions() which only keeps conditional expressions present in both scopes, matched by key. The key format includes the result type (e.g., $do=true => 9999 (Yes) vs $do=true => 123 (Yes)). After the first loop iteration, the variable's type changes (from 9999 to 123 due to reassignment), causing the keys to differ. The intersection then dropped the conditional expression entirely, losing the information that $x is definitely defined when $do is true.

The fix detects when both scopes have conditional expressions for the same variable with the same condition expressions (matching by condition holder equality) but different result types, and merges the result type holders using ExpressionTypeHolder::and() (which unions the types and ANDs the certainties).

Test

Extended tests/PHPStan/Rules/Variables/data/bug-6830.php with the test2 function that reproduces the exact issue: a variable assigned before a foreach loop under a condition, and used inside the loop under the same condition. The test expects no errors.

Fixes phpstan/phpstan#6830

- Fixed intersectConditionalExpressions in MutatingScope to merge conditional
  expressions with matching conditions but different result types, instead of
  dropping them entirely
- Added regression test case in tests/PHPStan/Rules/Variables/data/bug-6830.php
- Root cause: during loop scope merging, conditional expression keys include the
  result type, so different types across iterations caused the intersection to
  drop the conditional linking variable definedness to the guarding condition

Closes phpstan/phpstan#6830
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant