Skip to content

Commit 9698573

Browse files
phpstan-botclaude
andcommitted
Only preserve vacuous conditional expressions during if/else branch merging
The preserveVacuousConditionalExpressions logic was being applied to all mergeWith calls, including loop merging. In loop contexts, this incorrectly preserved per-iteration branch correlations (e.g., "if $operators is array{Operator}, then $operands is array{}") across iterations, causing types to be over-narrowed (int → 0, non-empty-list<int> → array{int}). Fix: add a $preserveVacuousConditionals parameter to mergeWith() (default false) and only pass true from the if/elseif/else branch merge sites in NodeScopeResolver. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent d62fe52 commit 9698573

3 files changed

Lines changed: 19 additions & 17 deletions

File tree

src/Analyser/MutatingScope.php

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3362,7 +3362,7 @@ public function isInFirstLevelStatement(): bool
33623362
return $this->inFirstLevelStatement;
33633363
}
33643364

3365-
public function mergeWith(?self $otherScope): self
3365+
public function mergeWith(?self $otherScope, bool $preserveVacuousConditionals = false): self
33663366
{
33673367
if ($otherScope === null || $this === $otherScope) {
33683368
return $this;
@@ -3372,16 +3372,18 @@ public function mergeWith(?self $otherScope): self
33723372

33733373
$mergedExpressionTypes = $this->mergeVariableHolders($ourExpressionTypes, $theirExpressionTypes);
33743374
$conditionalExpressions = $this->intersectConditionalExpressions($otherScope->conditionalExpressions);
3375-
$conditionalExpressions = $this->preserveVacuousConditionalExpressions(
3376-
$conditionalExpressions,
3377-
$this->conditionalExpressions,
3378-
$theirExpressionTypes,
3379-
);
3380-
$conditionalExpressions = $this->preserveVacuousConditionalExpressions(
3381-
$conditionalExpressions,
3382-
$otherScope->conditionalExpressions,
3383-
$ourExpressionTypes,
3384-
);
3375+
if ($preserveVacuousConditionals) {
3376+
$conditionalExpressions = $this->preserveVacuousConditionalExpressions(
3377+
$conditionalExpressions,
3378+
$this->conditionalExpressions,
3379+
$theirExpressionTypes,
3380+
);
3381+
$conditionalExpressions = $this->preserveVacuousConditionalExpressions(
3382+
$conditionalExpressions,
3383+
$otherScope->conditionalExpressions,
3384+
$ourExpressionTypes,
3385+
);
3386+
}
33853387
$conditionalExpressions = $this->createConditionalExpressions(
33863388
$conditionalExpressions,
33873389
$ourExpressionTypes,

src/Analyser/NodeScopeResolver.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,7 +1146,7 @@ public function processStmtNode(
11461146
$throwPoints = array_merge($throwPoints, $branchScopeStatementResult->getThrowPoints());
11471147
$impurePoints = array_merge($impurePoints, $branchScopeStatementResult->getImpurePoints());
11481148
$branchScope = $branchScopeStatementResult->getScope();
1149-
$finalScope = $branchScopeStatementResult->isAlwaysTerminating() ? $finalScope : $branchScope->mergeWith($finalScope);
1149+
$finalScope = $branchScopeStatementResult->isAlwaysTerminating() ? $finalScope : $branchScope->mergeWith($finalScope, true);
11501150
$alwaysTerminating = $alwaysTerminating && $branchScopeStatementResult->isAlwaysTerminating();
11511151
if (count($branchScopeStatementResult->getEndStatements()) > 0) {
11521152
$endStatements = array_merge($endStatements, $branchScopeStatementResult->getEndStatements());
@@ -1170,7 +1170,7 @@ public function processStmtNode(
11701170

11711171
if ($stmt->else === null) {
11721172
if (!$ifAlwaysTrue && !$lastElseIfConditionIsTrue) {
1173-
$finalScope = $scope->mergeWith($finalScope);
1173+
$finalScope = $scope->mergeWith($finalScope, true);
11741174
$alwaysTerminating = false;
11751175
}
11761176
} else {
@@ -1182,7 +1182,7 @@ public function processStmtNode(
11821182
$throwPoints = array_merge($throwPoints, $branchScopeStatementResult->getThrowPoints());
11831183
$impurePoints = array_merge($impurePoints, $branchScopeStatementResult->getImpurePoints());
11841184
$branchScope = $branchScopeStatementResult->getScope();
1185-
$finalScope = $branchScopeStatementResult->isAlwaysTerminating() ? $finalScope : $branchScope->mergeWith($finalScope);
1185+
$finalScope = $branchScopeStatementResult->isAlwaysTerminating() ? $finalScope : $branchScope->mergeWith($finalScope, true);
11861186
$alwaysTerminating = $alwaysTerminating && $branchScopeStatementResult->isAlwaysTerminating();
11871187
if (count($branchScopeStatementResult->getEndStatements()) > 0) {
11881188
$endStatements = array_merge($endStatements, $branchScopeStatementResult->getEndStatements());

tests/PHPStan/Analyser/nsrt/bug-13385b.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,13 @@ public function calculate(array $children): int {
3131
$value = $op->calculate($left, $right);
3232

3333
assertType(Operator::class, $op);
34-
assertType('0', $left); // could be int
35-
assertType('0', $right); // could be int
34+
assertType('int', $left);
35+
assertType('int', $right);
3636
assertType('int', $value);
3737

3838
$operands[] = $value;
3939

40-
assertType('array{int}', $operands); // could be non-empty-list<int>
40+
assertType('non-empty-list<int>', $operands);
4141
}
4242

4343
$operators[] = $child;

0 commit comments

Comments
 (0)