Skip to content

Commit 5857b59

Browse files
phpstan-botclaude
andcommitted
Add comment explaining Variable exception and add variable-equivalent test cases
Address review comment by VincentLanglet asking why the guard is restricted to non-Variable expressions. The Variable exception is needed because: - Non-Variable expressions (array dim fetches, property fetches) are always queryable; absence from a branch means "not narrowed", not "doesn't exist" - Variables are different: absence means genuinely "not defined", and conditional expressions for newly-defined variables are essential for variable certainty tracking (e.g. bug-14411-regression where $order must be recognised as always-defined when two conditions cover all cases) - Pre-defined variables (present in both branches) are already handled by the existing check that tests array_key_exists($exprString, $theirExpressionTypes) Added variable-equivalent test cases to both the rule test data and NSRT test to demonstrate that pre-defined variables work correctly with the existing guard-overlap check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 32c94b9 commit 5857b59

3 files changed

Lines changed: 52 additions & 0 deletions

File tree

src/Analyser/MutatingScope.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3649,6 +3649,18 @@ private function createConditionalExpressions(
36493649
) {
36503650
continue;
36513651
}
3652+
// Skip non-Variable expressions absent from the other branch when
3653+
// the guard overlaps with the other branch's guard type.
3654+
// Non-Variable expressions (array dim fetches, property fetches)
3655+
// are always queryable, so absence from a branch means "not
3656+
// narrowed", not "doesn't exist". Creating a conditional
3657+
// expression would incorrectly narrow the type when the guard
3658+
// fires from the branch that never referenced the expression.
3659+
// Variables are exempt: their absence means genuinely "not
3660+
// defined", and conditional expressions for newly-defined
3661+
// variables are needed for variable certainty tracking (e.g.
3662+
// bug-14411-regression). Pre-defined variables (present in both
3663+
// branches) are handled by the check above.
36523664
if (
36533665
!array_key_exists($exprString, $theirExpressionTypes)
36543666
&& !$holder->getExpr() instanceof Variable

tests/PHPStan/Analyser/nsrt/bug-14469.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,19 @@ function t(array $R, bool $var1, object $user): void {
1717
assertType('mixed', $R['aa']);
1818
}
1919
}
20+
21+
/** Variable equivalent: pre-defined variable stays mixed inside if ($aa) */
22+
function variableEquivalent(mixed $input, bool $var1, object $user): void {
23+
$aa = null;
24+
$bb = $input;
25+
26+
if ($var1) {
27+
$aa = $user->id === 10 ? 2 : null;
28+
} elseif ($bb) {
29+
$aa = $bb;
30+
}
31+
32+
if ($aa) {
33+
assertType('mixed', $bb);
34+
}
35+
}

tests/PHPStan/Rules/Comparison/data/bug-14469.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,27 @@ function multipleElseif(array $R, bool $var1, bool $var2, object $user): void {
7171
}
7272
}
7373
}
74+
75+
/**
76+
* Variable equivalent: pre-defined variable used in elseif condition.
77+
* Same pattern as the array dim fetch case but $bb is a Variable defined
78+
* before the if/elseif, so it's present in both branches' expression types.
79+
* The existing guard-overlap check (lines that test array_key_exists($exprString,
80+
* $theirExpressionTypes)) handles this case correctly.
81+
*/
82+
function variableEquivalent(bool $var1, object $user, mixed $input): void {
83+
$aa = null;
84+
$bb = $input;
85+
86+
if ($var1) {
87+
$aa = $user->id === 10 ? 2 : null;
88+
} elseif ($bb) {
89+
$aa = $bb;
90+
}
91+
92+
if ($aa) {
93+
if (!$bb) {
94+
return;
95+
}
96+
}
97+
}

0 commit comments

Comments
 (0)