From 32c94b969878f5d2ae88dbdd3713c9d31e07350a Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Tue, 14 Apr 2026 21:42:54 +0000 Subject: [PATCH 1/4] Do not create conditional expressions for non-Variable expressions absent from the other branch - In `MutatingScope::createConditionalExpressions()`, skip creating a conditional expression when the target expression (e.g. an array dim fetch like `$R['aa']`) is not tracked in the other branch, the expression is not a Variable, and the guard type overlaps with the other branch's guard type. - This prevents incorrect type narrowing where an array dim fetch or property fetch from an `elseif` condition leaks into a subsequent `if` block that checks a variable set in multiple branches. - The Variable exception preserves the existing behavior for variable certainty tracking (e.g. a variable defined in only one branch correctly gets Yes certainty via conditional expressions). - Tested with array dim fetch, nested array dim fetch, property fetch, and multiple elseif variants. --- src/Analyser/MutatingScope.php | 9 +++ tests/PHPStan/Analyser/nsrt/bug-14469.php | 19 +++++ .../BooleanNotConstantConditionRuleTest.php | 6 ++ .../Rules/Comparison/data/bug-14469.php | 73 +++++++++++++++++++ 4 files changed, 107 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-14469.php create mode 100644 tests/PHPStan/Rules/Comparison/data/bug-14469.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 122340cc27b..1046029b244 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3649,6 +3649,15 @@ private function createConditionalExpressions( ) { continue; } + if ( + !array_key_exists($exprString, $theirExpressionTypes) + && !$holder->getExpr() instanceof Variable + && array_key_exists($guardExprString, $theirExpressionTypes) + && $theirExpressionTypes[$guardExprString]->getCertainty()->yes() + && !$guardHolder->getType()->isSuperTypeOf($theirExpressionTypes[$guardExprString]->getType())->no() + ) { + continue; + } $conditionalExpression = new ConditionalExpressionHolder([$guardExprString => $guardHolder], $holder); $conditionalExpressions[$exprString][$conditionalExpression->getKey()] = $conditionalExpression; } diff --git a/tests/PHPStan/Analyser/nsrt/bug-14469.php b/tests/PHPStan/Analyser/nsrt/bug-14469.php new file mode 100644 index 00000000000..053c4112d3e --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14469.php @@ -0,0 +1,19 @@ +id === 10 ? 2 : null; + } elseif ($R['aa']) { + $aa = $R['aa']; + } + + if ($aa) { + assertType('mixed', $R['aa']); + } +} diff --git a/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php index 3cf3f2a8bbc..ee1d1ff12fe 100644 --- a/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/BooleanNotConstantConditionRuleTest.php @@ -236,4 +236,10 @@ public function testBug6702(): void $this->analyse([__DIR__ . '/data/bug-6702.php'], []); } + public function testBug14469(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-14469.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-14469.php b/tests/PHPStan/Rules/Comparison/data/bug-14469.php new file mode 100644 index 00000000000..49d1399e259 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-14469.php @@ -0,0 +1,73 @@ +id === 10 ? 2 : null; + } elseif ($R['aa']) { + $aa = $R['aa']; + } + + if ($aa) { + if (!$R['aa']) { + return []; + } + } + return $R; +} + +/** Property fetch variant */ +function propertyFetch(object $obj, bool $var1, object $user): void { + $aa = null; + + if ($var1) { + $aa = $user->id === 10 ? 2 : null; + } elseif ($obj->prop) { + $aa = $obj->prop; + } + + if ($aa) { + if (!$obj->prop) { + return; + } + } +} + +/** Nested array fetch variant */ +function nestedArrayFetch(array $R, bool $var1, object $user): void { + $aa = null; + + if ($var1) { + $aa = $user->id === 10 ? 2 : null; + } elseif ($R['a']['b']) { + $aa = $R['a']['b']; + } + + if ($aa) { + if (!$R['a']['b']) { + return; + } + } +} + +/** Multiple elseif branches */ +function multipleElseif(array $R, bool $var1, bool $var2, object $user): void { + $aa = null; + + if ($var1) { + $aa = $user->id === 10 ? 2 : null; + } elseif ($var2) { + $aa = 5; + } elseif ($R['aa']) { + $aa = $R['aa']; + } + + if ($aa) { + if (!$R['aa']) { + return; + } + } +} From 5857b59c2c9aea635a9c8cbaaa7141792f6765c3 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 15 Apr 2026 09:10:24 +0000 Subject: [PATCH 2/4] 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 --- src/Analyser/MutatingScope.php | 12 ++++++++++ tests/PHPStan/Analyser/nsrt/bug-14469.php | 16 +++++++++++++ .../Rules/Comparison/data/bug-14469.php | 24 +++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 1046029b244..5b87dfe13e9 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3649,6 +3649,18 @@ private function createConditionalExpressions( ) { continue; } + // Skip non-Variable expressions absent from the other branch when + // the guard overlaps with the other branch's guard type. + // Non-Variable expressions (array dim fetches, property fetches) + // are always queryable, so absence from a branch means "not + // narrowed", not "doesn't exist". Creating a conditional + // expression would incorrectly narrow the type when the guard + // fires from the branch that never referenced the expression. + // Variables are exempt: their absence means genuinely "not + // defined", and conditional expressions for newly-defined + // variables are needed for variable certainty tracking (e.g. + // bug-14411-regression). Pre-defined variables (present in both + // branches) are handled by the check above. if ( !array_key_exists($exprString, $theirExpressionTypes) && !$holder->getExpr() instanceof Variable diff --git a/tests/PHPStan/Analyser/nsrt/bug-14469.php b/tests/PHPStan/Analyser/nsrt/bug-14469.php index 053c4112d3e..98537e35565 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-14469.php +++ b/tests/PHPStan/Analyser/nsrt/bug-14469.php @@ -17,3 +17,19 @@ function t(array $R, bool $var1, object $user): void { assertType('mixed', $R['aa']); } } + +/** Variable equivalent: pre-defined variable stays mixed inside if ($aa) */ +function variableEquivalent(mixed $input, bool $var1, object $user): void { + $aa = null; + $bb = $input; + + if ($var1) { + $aa = $user->id === 10 ? 2 : null; + } elseif ($bb) { + $aa = $bb; + } + + if ($aa) { + assertType('mixed', $bb); + } +} diff --git a/tests/PHPStan/Rules/Comparison/data/bug-14469.php b/tests/PHPStan/Rules/Comparison/data/bug-14469.php index 49d1399e259..6d0b8a49b86 100644 --- a/tests/PHPStan/Rules/Comparison/data/bug-14469.php +++ b/tests/PHPStan/Rules/Comparison/data/bug-14469.php @@ -71,3 +71,27 @@ function multipleElseif(array $R, bool $var1, bool $var2, object $user): void { } } } + +/** + * Variable equivalent: pre-defined variable used in elseif condition. + * Same pattern as the array dim fetch case but $bb is a Variable defined + * before the if/elseif, so it's present in both branches' expression types. + * The existing guard-overlap check (lines that test array_key_exists($exprString, + * $theirExpressionTypes)) handles this case correctly. + */ +function variableEquivalent(bool $var1, object $user, mixed $input): void { + $aa = null; + $bb = $input; + + if ($var1) { + $aa = $user->id === 10 ? 2 : null; + } elseif ($bb) { + $aa = $bb; + } + + if ($aa) { + if (!$bb) { + return; + } + } +} From 96cbec6284e5fd54797cea950379f455c66f8be2 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 15 Apr 2026 09:31:13 +0000 Subject: [PATCH 3/4] Remove comments and use @param mixed for PHP 7.4 compatibility Co-Authored-By: Claude Opus 4.6 --- src/Analyser/MutatingScope.php | 12 ------------ tests/PHPStan/Analyser/nsrt/bug-14469.php | 7 +++++-- tests/PHPStan/Rules/Comparison/data/bug-14469.php | 8 ++------ 3 files changed, 7 insertions(+), 20 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 5b87dfe13e9..1046029b244 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3649,18 +3649,6 @@ private function createConditionalExpressions( ) { continue; } - // Skip non-Variable expressions absent from the other branch when - // the guard overlaps with the other branch's guard type. - // Non-Variable expressions (array dim fetches, property fetches) - // are always queryable, so absence from a branch means "not - // narrowed", not "doesn't exist". Creating a conditional - // expression would incorrectly narrow the type when the guard - // fires from the branch that never referenced the expression. - // Variables are exempt: their absence means genuinely "not - // defined", and conditional expressions for newly-defined - // variables are needed for variable certainty tracking (e.g. - // bug-14411-regression). Pre-defined variables (present in both - // branches) are handled by the check above. if ( !array_key_exists($exprString, $theirExpressionTypes) && !$holder->getExpr() instanceof Variable diff --git a/tests/PHPStan/Analyser/nsrt/bug-14469.php b/tests/PHPStan/Analyser/nsrt/bug-14469.php index 98537e35565..08eb174c59c 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-14469.php +++ b/tests/PHPStan/Analyser/nsrt/bug-14469.php @@ -18,8 +18,11 @@ function t(array $R, bool $var1, object $user): void { } } -/** Variable equivalent: pre-defined variable stays mixed inside if ($aa) */ -function variableEquivalent(mixed $input, bool $var1, object $user): void { +/** + * Variable equivalent: pre-defined variable stays mixed inside if ($aa) + * @param mixed $input + */ +function variableEquivalent($input, bool $var1, object $user): void { $aa = null; $bb = $input; diff --git a/tests/PHPStan/Rules/Comparison/data/bug-14469.php b/tests/PHPStan/Rules/Comparison/data/bug-14469.php index 6d0b8a49b86..0ac2cac904c 100644 --- a/tests/PHPStan/Rules/Comparison/data/bug-14469.php +++ b/tests/PHPStan/Rules/Comparison/data/bug-14469.php @@ -73,13 +73,9 @@ function multipleElseif(array $R, bool $var1, bool $var2, object $user): void { } /** - * Variable equivalent: pre-defined variable used in elseif condition. - * Same pattern as the array dim fetch case but $bb is a Variable defined - * before the if/elseif, so it's present in both branches' expression types. - * The existing guard-overlap check (lines that test array_key_exists($exprString, - * $theirExpressionTypes)) handles this case correctly. + * @param mixed $input */ -function variableEquivalent(bool $var1, object $user, mixed $input): void { +function variableEquivalent(bool $var1, object $user, $input): void { $aa = null; $bb = $input; From 8f158b1c80724e9826c2816b21d5079512f2b399 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Wed, 15 Apr 2026 09:32:50 +0000 Subject: [PATCH 4/4] Use @param mixed instead of native mixed type for PHP 7.4 compat, remove comments Co-Authored-By: Claude Opus 4.6 --- tests/PHPStan/Analyser/nsrt/bug-14469.php | 5 +---- tests/PHPStan/Rules/Comparison/data/bug-14469.php | 4 +--- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/PHPStan/Analyser/nsrt/bug-14469.php b/tests/PHPStan/Analyser/nsrt/bug-14469.php index 08eb174c59c..67bff59378f 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-14469.php +++ b/tests/PHPStan/Analyser/nsrt/bug-14469.php @@ -18,10 +18,7 @@ function t(array $R, bool $var1, object $user): void { } } -/** - * Variable equivalent: pre-defined variable stays mixed inside if ($aa) - * @param mixed $input - */ +/** @param mixed $input */ function variableEquivalent($input, bool $var1, object $user): void { $aa = null; $bb = $input; diff --git a/tests/PHPStan/Rules/Comparison/data/bug-14469.php b/tests/PHPStan/Rules/Comparison/data/bug-14469.php index 0ac2cac904c..66c6a98aa07 100644 --- a/tests/PHPStan/Rules/Comparison/data/bug-14469.php +++ b/tests/PHPStan/Rules/Comparison/data/bug-14469.php @@ -72,9 +72,7 @@ function multipleElseif(array $R, bool $var1, bool $var2, object $user): void { } } -/** - * @param mixed $input - */ +/** @param mixed $input */ function variableEquivalent(bool $var1, object $user, $input): void { $aa = null; $bb = $input;