From ac553f8bc5a61991b75cd02605ab2d8d18422cb1 Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Sat, 24 Oct 2026 20:37:57 +0000 Subject: [PATCH] Carry template argument constraints of `if`/`elseif`/`else` branches ruled out by the condition onto the `if` result scope - IfHandler dropped the whole statement result of a branch whose condition is always false (or an elseif/else after an always-true condition), including the template argument constraints collected in it, e.g. the send from the declared return type. The branch is still walked and its rules still see its calls, so with unresolvedTemplateArguments their template arguments stayed unresolved(*NEVER*) and return.type / generator.valueType were reported. - The dropped branches' scopes now contribute their constraints to the final scope, like TernaryHandler, ForHandler and TryCatchHandler already do for parts that do not flow into the result. - Ported the same change to the turbo-ext IfHandler.cpp mirror. - Probed while/for/foreach/switch/match/ternary/&&/dead catch/unreachable statement after return: already correct. --- src/Analyser/StmtHandler/IfHandler.php | 13 +++ .../Rules/Generators/YieldTypeRuleTest.php | 7 ++ .../Rules/Generators/data/bug-15476.php | 56 ++++++++++ .../Rules/Methods/ReturnTypeRuleTest.php | 6 ++ .../PHPStan/Rules/Methods/data/bug-15476.php | 102 ++++++++++++++++++ turbo-ext/src/IfHandler.cpp | 27 +++++ 6 files changed, 211 insertions(+) create mode 100644 tests/PHPStan/Rules/Generators/data/bug-15476.php create mode 100644 tests/PHPStan/Rules/Methods/data/bug-15476.php diff --git a/src/Analyser/StmtHandler/IfHandler.php b/src/Analyser/StmtHandler/IfHandler.php index 102a6599db0..23cc1d49afe 100644 --- a/src/Analyser/StmtHandler/IfHandler.php +++ b/src/Analyser/StmtHandler/IfHandler.php @@ -54,6 +54,7 @@ public function processStmt( $entryScope = $scope; $flowBranches = []; $elseFlow = null; + $deadBranchScopes = []; $condResult = $nodeScopeResolver->processExprNode($stmt, $stmt->cond, $scope, $storage, $nodeCallback, ExpressionContext::createDeep($context->shouldResolveTemplateArguments())); $dependencies[] = $condResult->getDependencies(); $nodeScopeResolver->callNodeCallback($nodeCallback, $stmt, $entryScope, $storage); @@ -84,6 +85,8 @@ public function processStmt( $endStatements[] = new InternalEndStatementResult($stmt, $branchScopeStatementResult); } $hasYield = $branchScopeStatementResult->hasYield() || $hasYield; + } else { + $deadBranchScopes[] = $branchScopeStatementResult->getScope(); } $scope = $condResult->getFalseyScope(); @@ -119,6 +122,8 @@ public function processStmt( $endStatements[] = new InternalEndStatementResult($elseif, $branchScopeStatementResult); } $hasYield = $hasYield || $branchScopeStatementResult->hasYield(); + } else { + $deadBranchScopes[] = $branchScopeStatementResult->getScope(); } if ( @@ -156,6 +161,8 @@ public function processStmt( $endStatements[] = new InternalEndStatementResult($stmt->else, $branchScopeStatementResult); } $hasYield = $hasYield || $branchScopeStatementResult->hasYield(); + } else { + $deadBranchScopes[] = $branchScopeStatementResult->getScope(); } } @@ -163,6 +170,12 @@ public function processStmt( $finalScope = $scope; } + // a branch the condition rules out is still walked and its rules still + // see its calls - their template arguments are resolved from it too + foreach ($deadBranchScopes as $deadBranchScope) { + $finalScope = $finalScope->addTemplateArgumentConstraints($deadBranchScope->getTemplateArgumentConstraints()); + } + if ($stmt->else === null && !$ifAlwaysTrue && !$lastElseIfConditionIsTrue) { $endStatements[] = new InternalEndStatementResult($stmt, new InternalStatementResult($finalScope, hasYield: $hasYield, isAlwaysTerminating: $alwaysTerminating, exitPoints: $exitPoints, throwPoints: $throwPoints, impurePoints: $impurePoints)); } diff --git a/tests/PHPStan/Rules/Generators/YieldTypeRuleTest.php b/tests/PHPStan/Rules/Generators/YieldTypeRuleTest.php index 555ae5f5baf..bc1f39c878d 100644 --- a/tests/PHPStan/Rules/Generators/YieldTypeRuleTest.php +++ b/tests/PHPStan/Rules/Generators/YieldTypeRuleTest.php @@ -5,6 +5,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleLevelHelper; use PHPStan\Testing\RuleTestCase; +use PHPUnit\Framework\Attributes\RequiresPhp; /** * @extends RuleTestCase @@ -87,4 +88,10 @@ public function testYieldOversizedSelfRejection(): void $this->analyse([__DIR__ . '/data/bug-yield-oversized-self-rejection.php'], []); } + #[RequiresPhp('>= 8.0.0')] + public function testBug15476(): void + { + $this->analyse([__DIR__ . '/data/bug-15476.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Generators/data/bug-15476.php b/tests/PHPStan/Rules/Generators/data/bug-15476.php new file mode 100644 index 00000000000..c9ac1a7aa6c --- /dev/null +++ b/tests/PHPStan/Rules/Generators/data/bug-15476.php @@ -0,0 +1,56 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug15476Generators; + +/** + * @template TKey of array-key + * @template TValue + */ +final class Box +{ +} + +interface Factory +{ + /** + * @template TKey of array-key + * @template TValue + * @param iterable $value + * @return Box + */ + public function box(iterable $value = []): Box; +} + +final class Item +{ +} + +final class Example +{ + public function __construct(private Factory $factory) + { + } + + /** @return \Generator> */ + public function deadBranch(Item $item): \Generator + { + if ($item === null) { + yield $this->factory->box(); + } + + yield $this->factory->box([$item]); + } + + /** @return \Generator> */ + public function deadElse(Item $item): \Generator + { + if ($item !== null) { + yield $this->factory->box([$item]); + } else { + yield $this->factory->box(); + } + } + +} diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index 260fde1475f..1bd63a75502 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -1419,4 +1419,10 @@ public function testBug13325(): void $this->analyse([__DIR__ . '/data/bug-13325.php'], []); } + #[RequiresPhp('>= 8.0.0')] + public function testBug15476(): void + { + $this->analyse([__DIR__ . '/data/bug-15476.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-15476.php b/tests/PHPStan/Rules/Methods/data/bug-15476.php new file mode 100644 index 00000000000..95dbbbcea19 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-15476.php @@ -0,0 +1,102 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug15476; + +/** + * @template TKey of array-key + * @template TValue + */ +final class Box +{ +} + +interface Factory +{ + /** + * @template TKey of array-key + * @template TValue + * @param iterable $value + * @return Box + */ + public function box(iterable $value = []): Box; +} + +final class Item +{ +} + +final class Example +{ + public function __construct(private Factory $factory) + { + } + + /** @return Box */ + public function deadBranch(Item $item): Box + { + if ($item === null) { + return $this->factory->box(); + } + + return $this->factory->box([$item]); + } + + /** @return Box */ + public function liveBranch(?Item $item): Box + { + if ($item === null) { + return $this->factory->box(); + } + + return $this->factory->box([$item]); + } + + /** @return Box */ + public function deadElseif(Item $item, bool $b): Box + { + if ($b) { + return $this->factory->box([$item]); + } elseif ($item === null) { + return $this->factory->box(); + } + + return $this->factory->box([$item]); + } + + /** @return Box */ + public function deadElse(Item $item): Box + { + if ($item !== null) { + return $this->factory->box([$item]); + } else { + return $this->factory->box(); + } + } + + /** @return Box */ + public function deadElseAfterAlwaysTrueElseif(Item $item, bool $b): Box + { + if ($b) { + return $this->factory->box([$item]); + } elseif ($item !== null) { + return $this->factory->box([$item]); + } else { + return $this->factory->box(); + } + } + + /** @return Box */ + public function deadNestedBranch(Item $item, bool $b): Box + { + if ($b) { + if ($item === null) { + return $this->factory->box(); + } + } + + return $this->factory->box([$item]); + } + +} diff --git a/turbo-ext/src/IfHandler.cpp b/turbo-ext/src/IfHandler.cpp index f703bb5086e..a98cd8d7109 100644 --- a/turbo-ext/src/IfHandler.cpp +++ b/turbo-ext/src/IfHandler.cpp @@ -105,6 +105,16 @@ void pushFlowBranch(zv::Arr &flowBranches, zv::Val conditionFlow, zval *branchFl flowBranches.push(zv::Ref(branchFlow)); } +/* $deadBranchScopes[] = $branchScopeStatementResult->getScope(); false = pending exception */ +[[nodiscard]] bool pushDeadBranchScope(zv::Arr &deadBranchScopes, zval *branchResult) +{ + zv::Val hold; + zval *branchScope = pt_internal_statement_result_scope(branchResult, hold); + if (UNEXPECTED(branchScope == NULL)) return false; + deadBranchScopes.push(zv::Ref(branchScope)); + return true; +} + } // namespace namespace phpstanturbo { @@ -137,6 +147,7 @@ class IfHandler zv::Arr dependencies = zv::Arr::empty(); zval *entryScope = scope; zv::Arr flowBranches = zv::Arr::empty(); + zv::Arr deadBranchScopes = zv::Arr::empty(); zv::Val elseFlow = zv::Val::null(); zval *cond = ptsh::readNodeProperty(pt_ih_cond_site, stmt, PT_LC("cond")); @@ -200,6 +211,8 @@ class IfHandler bool branchHasYield; if (UNEXPECTED(!pt_internal_statement_result_has_yield(branchResult.raw(), branchHasYield))) return zv::Val(); hasYield = branchHasYield || hasYield; + } else if (UNEXPECTED(!pushDeadBranchScope(deadBranchScopes, branchResult.raw()))) { + return zv::Val(); } } @@ -258,6 +271,8 @@ class IfHandler pushFlowBranch(flowBranches, std::move(conditionFlow), branchFlow); if (!ifAlwaysTrue && !lastElseIfConditionIsTrue && elseIfConditionType.isTrue != PT_TRI_NO) { if (UNEXPECTED(!mergeBranch(branchResult.raw(), exitPoints, throwPoints, impurePoints, finalScope, alwaysTerminating, endStatements, elseif, pt_ih_elseif_stmts_site, hasYield))) return zv::Val(); + } else if (UNEXPECTED(!pushDeadBranchScope(deadBranchScopes, branchResult.raw()))) { + return zv::Val(); } if (elseIfConditionType.isTrue == PT_TRI_YES) { @@ -309,6 +324,8 @@ class IfHandler if (UNEXPECTED(elseNode == NULL)) return zv::Val(); elseHold = zv::Val::copyOf(zv::Ref(elseNode)); if (UNEXPECTED(!mergeBranch(branchResult.raw(), exitPoints, throwPoints, impurePoints, finalScope, alwaysTerminating, endStatements, elseHold.raw(), pt_ih_else_stmts_site, hasYield))) return zv::Val(); + } else if (UNEXPECTED(!pushDeadBranchScope(deadBranchScopes, branchResult.raw()))) { + return zv::Val(); } } @@ -316,6 +333,16 @@ class IfHandler finalScope = zv::Val::copyOf(zv::Ref(scopeValue.raw())); } + // a branch the condition rules out is still walked and its rules still + // see its calls - their template arguments are resolved from it too + for (auto entry : zv::TableRef(deadBranchScopes.table())) { + zv::Val constraints = pt_mutating_scope_get_template_argument_constraints(Z_OBJ_P(entry.value().raw())); + if (UNEXPECTED(constraints.isUndef())) return zv::Val(); + zv::Val constrained = pt_mutating_scope_add_template_argument_constraints(Z_OBJ_P(finalScope.raw()), constraints.raw()); + if (UNEXPECTED(constrained.isUndef())) return zv::Val(); + finalScope = std::move(constrained); + } + elseNode = ptsh::readNodeProperty(pt_ih_else_site, stmt, PT_LC("else")); if (UNEXPECTED(elseNode == NULL)) return zv::Val(); if (Z_TYPE_P(elseNode) == IS_NULL && !ifAlwaysTrue && !lastElseIfConditionIsTrue) {