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) {