From d809a9118b7e1fcefa222d4db47c0354a4b10b96 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 5 Sep 2026 10:11:03 +0200 Subject: [PATCH 1/3] Preserve one-sided conditional holders whose consequent already holds in the other branch An if-merge intersects conditional expression holders by exact key, so a holder recorded only in one branch is dropped even when the other branch's flat state for the holder's target already satisfies the consequent - a subtype with at-least-as-strong certainty. In that case the consequent holds on both paths whenever the guard matches, so firing the holder later is sound and the holder can survive the merge alongside the vacuous-guard rescue. Closes https://github.com/phpstan/phpstan/issues/9628 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA --- src/Analyser/MutatingScope.php | 23 +++++++++++++++++++++- tests/PHPStan/Analyser/nsrt/bug-9628.php | 25 ++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-9628.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 96f4301f6df..357a90d85e8 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -4232,6 +4232,14 @@ private function withoutPreciseClassConstantFetches( } /** + * Rescues one-sided conditional holders across an if-merge. + * + * A holder missing from the intersection survives when either some guard's + * recorded type is impossible in the other branch (the holder is vacuously + * true there), or the other branch's flat state for the holder's target + * already satisfies the holder's consequent (subtype with at-least-as-strong + * certainty) - the consequent then holds on both paths under the guard. + * * @param array $currentConditionalExpressions * @param array $sourceConditionalExpressions * @param array $otherExpressionTypes @@ -4264,9 +4272,22 @@ private function preserveVacuousConditionalExpressions( if ($otherType->isSuperTypeOf($guardType)->no()) { $currentConditionalExpressions[$exprString][$key] = $holder; - break; + continue 2; } } + + if ($typeHolder->getCertainty()->no() || !array_key_exists($exprString, $otherExpressionTypes)) { + continue; + } + + $otherTargetHolder = $otherExpressionTypes[$exprString]; + $otherTargetCertainty = $otherTargetHolder->getCertainty(); + if ( + ($otherTargetCertainty->yes() || $otherTargetCertainty->equals($typeHolder->getCertainty())) + && $typeHolder->getType()->isSuperTypeOf($otherTargetHolder->getType())->yes() + ) { + $currentConditionalExpressions[$exprString][$key] = $holder; + } } } diff --git a/tests/PHPStan/Analyser/nsrt/bug-9628.php b/tests/PHPStan/Analyser/nsrt/bug-9628.php new file mode 100644 index 00000000000..ab6af019c47 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-9628.php @@ -0,0 +1,25 @@ + Date: Sat, 5 Sep 2026 10:16:49 +0200 Subject: [PATCH 2/3] Union one-sided conditional holders that share a target and guard set across merges Loop convergence merges the body-end scope with the loop-entry scope, and both sides often carry a Yes-certainty holder for the same target under the same guard - just with different consequent types (the pre-loop assignment's type on one side, the body reassignment's type on the other). The exact-key intersection dropped both, losing the guard-implies-defined fact and producing "might not be defined" next to "if condition is always true". Whichever side a merged path came from, the guard matching later implies one of the recorded consequents, so a holder with the union of the consequent types under their shared certainty holds on every merged path. Closes https://github.com/phpstan/phpstan/issues/6830 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA --- src/Analyser/MutatingScope.php | 79 +++++++++++++++++++ tests/PHPStan/Analyser/nsrt/bug-6830.php | 29 +++++++ .../PHPStan/Rules/Variables/data/bug-6830.php | 19 +++++ 3 files changed, 127 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-6830.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 357a90d85e8..808c67096a8 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -4140,6 +4140,11 @@ public function mergeWith(?self $otherScope, bool $preserveVacuousConditionals = $ourExpressionTypes, ); } + $conditionalExpressions = $this->mergeSameGuardConditionalExpressions( + $conditionalExpressions, + $this->conditionalExpressions, + $otherScope->conditionalExpressions, + ); $conditionalExpressions = ScopeOps::createConditionalExpressions( $conditionalExpressions, $ourExpressionTypes, @@ -4294,6 +4299,80 @@ private function preserveVacuousConditionalExpressions( return $currentConditionalExpressions; } + /** + * Merges one-sided holders that share a target and an identical guard set: + * whichever branch a merged path came from, the guard matching later implies + * one of the recorded consequents, so the union of the consequent types + * (under their shared certainty) holds on every merged path. + * + * @param array $currentConditionalExpressions + * @param array $ourConditionalExpressions + * @param array $theirConditionalExpressions + * @return array + */ + private function mergeSameGuardConditionalExpressions( + array $currentConditionalExpressions, + array $ourConditionalExpressions, + array $theirConditionalExpressions, + ): array + { + foreach ($ourConditionalExpressions as $exprString => $ourHolders) { + if (!array_key_exists($exprString, $theirConditionalExpressions)) { + continue; + } + + $theirHolders = $theirConditionalExpressions[$exprString]; + foreach ($ourHolders as $ourKey => $ourHolder) { + if (isset($currentConditionalExpressions[$exprString][$ourKey])) { + continue; + } + + $ourTypeHolder = $ourHolder->getTypeHolder(); + if ($ourTypeHolder->getCertainty()->no()) { + continue; + } + + foreach ($theirHolders as $theirKey => $theirHolder) { + if (isset($currentConditionalExpressions[$exprString][$theirKey])) { + continue; + } + + $theirTypeHolder = $theirHolder->getTypeHolder(); + if (!$theirTypeHolder->getCertainty()->equals($ourTypeHolder->getCertainty())) { + continue; + } + + $ourGuards = $ourHolder->getConditionExpressionTypeHolders(); + $theirGuards = $theirHolder->getConditionExpressionTypeHolders(); + if (count($ourGuards) !== count($theirGuards)) { + continue; + } + + foreach ($ourGuards as $guardExprString => $ourGuardHolder) { + if ( + !array_key_exists($guardExprString, $theirGuards) + || !$ourGuardHolder->equals($theirGuards[$guardExprString]) + ) { + continue 2; + } + } + + $unionHolder = new ConditionalExpressionHolder( + $ourGuards, + new ExpressionTypeHolder( + $ourTypeHolder->getExpr(), + TypeCombinator::union($ourTypeHolder->getType(), $theirTypeHolder->getType()), + $ourTypeHolder->getCertainty(), + ), + ); + $currentConditionalExpressions[$exprString][$unionHolder->getKey()] = $unionHolder; + } + } + } + + return $currentConditionalExpressions; + } + /** * @param array $newConditionalExpressions * @param array $existingConditionalExpressions diff --git a/tests/PHPStan/Analyser/nsrt/bug-6830.php b/tests/PHPStan/Analyser/nsrt/bug-6830.php new file mode 100644 index 00000000000..0652ac4442c --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-6830.php @@ -0,0 +1,29 @@ + Date: Sat, 5 Sep 2026 22:16:08 +0200 Subject: [PATCH 3/3] Do not preserve conditional holders for unresolved expressions An ErrorType consequent (e.g. an access to an undefined property) is a subtype of everything, so it looked "already satisfied" and its holder was preserved across the merge. Applying it tracked the expression and hid the underlying error - a false negative on undefined-property access. Skip holders whose consequent or other-branch state is an ErrorType, and use early exit in the preservation loop. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA --- src/Analyser/MutatingScope.php | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 808c67096a8..be2a439aab0 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -4287,12 +4287,24 @@ private function preserveVacuousConditionalExpressions( $otherTargetHolder = $otherExpressionTypes[$exprString]; $otherTargetCertainty = $otherTargetHolder->getCertainty(); - if ( - ($otherTargetCertainty->yes() || $otherTargetCertainty->equals($typeHolder->getCertainty())) - && $typeHolder->getType()->isSuperTypeOf($otherTargetHolder->getType())->yes() - ) { - $currentConditionalExpressions[$exprString][$key] = $holder; + if (!$otherTargetCertainty->yes() && !$otherTargetCertainty->equals($typeHolder->getCertainty())) { + continue; + } + + // An unresolved expression (e.g. an access to an undefined property) is held + // as ErrorType. ErrorType is a subtype of everything, so a consequent or an + // other-branch state of ErrorType would look "already satisfied" and preserve + // a holder that tracks the expression and hides the underlying error. Such a + // holder carries no real type relationship, so skip it. + if ($typeHolder->getType() instanceof ErrorType || $otherTargetHolder->getType() instanceof ErrorType) { + continue; } + + if (!$typeHolder->getType()->isSuperTypeOf($otherTargetHolder->getType())->yes()) { + continue; + } + + $currentConditionalExpressions[$exprString][$key] = $holder; } }