From ac071b45f282ae97f2b5e1af3186eb63134cbde5 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 15 Apr 2026 04:10:02 +0000 Subject: [PATCH] Do not let conditional expressions unset variables with proven Yes certainty in `filterBySpecifiedTypes` - In `MutatingScope::filterBySpecifiedTypes()`, when conditional expressions resolve to `no()` certainty for a variable, skip the unset if the variable already has `yes()` certainty in the current scope. - This prevents stale conditional expressions (created during earlier scope merges) from overriding definite variable existence proven by `isset()` or direct assignment. - The bug manifested when a `foreach` loop's zero-iterations scope merge triggered conditional expressions linking a variable's existence to the iterable being non-empty, even though `isset()` had already proven the variable was defined. - The same bug also affected `if` branches that narrowed the iterable type to empty. --- src/Analyser/MutatingScope.php | 6 ++ .../Variables/DefinedVariableRuleTest.php | 9 +++ .../Rules/Variables/data/bug-14353.php | 68 +++++++++++++++++++ 3 files changed, 83 insertions(+) create mode 100644 tests/PHPStan/Rules/Variables/data/bug-14353.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 122340cc27..29e958f3ba 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3263,6 +3263,12 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self foreach ($conditions as $conditionalExprString => $expressions) { $certainty = TrinaryLogic::lazyExtremeIdentity($expressions, static fn (ConditionalExpressionHolder $holder) => $holder->getTypeHolder()->getCertainty()); if ($certainty->no()) { + if ( + array_key_exists($conditionalExprString, $scope->expressionTypes) + && $scope->expressionTypes[$conditionalExprString]->getCertainty()->yes() + ) { + continue; + } unset($scope->expressionTypes[$conditionalExprString]); } else { if (array_key_exists($conditionalExprString, $scope->expressionTypes)) { diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index 6da8dfb7a5..af4622783d 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1531,4 +1531,13 @@ public function testBug6688(): void ]); } + public function testBug14353(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = true; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + $this->analyse([__DIR__ . '/data/bug-14353.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/bug-14353.php b/tests/PHPStan/Rules/Variables/data/bug-14353.php new file mode 100644 index 0000000000..ed25cb3b45 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-14353.php @@ -0,0 +1,68 @@ + */ +function get(): array +{ + return [1]; +} + +class Test +{ + public function test(): void + { + $reports = []; + + foreach (get() as $report) { + $reports[$report] = $report; + } + + if (isset($this->data)) { + foreach ($reports as $report_id => $report) { + $report_ids[$report_id] = 1; + } + } else { + foreach ($reports as $report_id => $report) { + $report_ids[$report_id] = 1; + } + } + + if (isset($report_ids)) { + var_dump($report_ids); + + foreach ($reports as $report) {} + + var_dump($report_ids); + } + } + + public function testWithIfBranch(): void + { + $reports = []; + + foreach (get() as $report) { + $reports[$report] = $report; + } + + if (isset($this->data)) { + foreach ($reports as $report_id => $report) { + $report_ids[$report_id] = 1; + } + } else { + foreach ($reports as $report_id => $report) { + $report_ids[$report_id] = 1; + } + } + + if (isset($report_ids)) { + var_dump($report_ids); + + if ($reports === []) { + echo 'empty'; + } + + var_dump($report_ids); + } + } +}