diff --git a/src/Analyser/LoopWrittenVariableNames.php b/src/Analyser/LoopWrittenVariableNames.php new file mode 100644 index 0000000000..5951abf64d --- /dev/null +++ b/src/Analyser/LoopWrittenVariableNames.php @@ -0,0 +1,226 @@ +|null null when the loop can write variables whose names are not known + */ + public static function collect(Node $loop, ?VariableFlow $passFlow): ?array + { + $names = self::getSyntacticNames($loop); + if ($names === null) { + return null; + } + + $flows = [$passFlow]; + while ($flows !== []) { + $flow = array_pop($flows); + if ($flow instanceof VariableAccessFlow) { + if (in_array($flow->kind, [VariableFlow::WRITE, VariableFlow::DEFINE, VariableFlow::DISCARD, VariableFlow::ESCAPE], true)) { + $names[$flow->name] = true; + } + continue; + } + if ($flow instanceof VariableSequenceFlow) { + foreach ($flow->children as $child) { + $flows[] = $child; + } + continue; + } + if (!$flow instanceof VariableControlFlow) { + continue; + } + + foreach ($flow->children as $child) { + $flows[] = $child; + } + foreach ($flow->catches as [, $catchFlow]) { + $flows[] = $catchFlow; + } + foreach ($flow->cases as [$caseCondition, $caseBody]) { + $flows[] = $caseCondition; + $flows[] = $caseBody; + } + foreach ($flow->bindings as $write) { + $names[$write->getVariableName()] = true; + } + foreach ($flow->ownWrites as $write) { + $names[$write->getVariableName()] = true; + } + } + + return $names; + } + + /** + * @return array|null + */ + private static function getSyntacticNames(Node $loop): ?array + { + $cached = $loop->getAttribute(self::SYNTACTIC_NAMES_ATTRIBUTE); + if (is_array($cached)) { + return $cached; + } + if ($cached === false) { + return null; + } + + $names = self::findSyntacticNames($loop); + $loop->setAttribute(self::SYNTACTIC_NAMES_ATTRIBUTE, $names ?? false); + + return $names; + } + + /** + * @return array|null + */ + private static function findSyntacticNames(Node $loop): ?array + { + $names = []; + $nodes = [$loop]; + while ($nodes !== []) { + $node = array_pop($nodes); + if ($node instanceof Stmt\Function_ || $node instanceof Stmt\ClassLike) { + continue; + } + if ( + ($node instanceof Expr\Variable && !is_string($node->name)) + || $node instanceof Expr\Include_ + || $node instanceof Expr\Eval_ + || ( + $node instanceof Expr\FuncCall + && $node->name instanceof Node\Name + && in_array($node->name->toLowerString(), ['extract', 'parse_str'], true) + ) + ) { + return null; + } + + $targets = []; + if ($node instanceof Expr\Assign || $node instanceof Expr\AssignOp) { + $targets[] = $node->var; + } elseif ($node instanceof Expr\AssignRef) { + $targets[] = $node->var; + $targets[] = $node->expr; + } elseif ($node instanceof Expr\PreInc || $node instanceof Expr\PreDec || $node instanceof Expr\PostInc || $node instanceof Expr\PostDec) { + $targets[] = $node->var; + } elseif ($node instanceof Stmt\Foreach_) { + if ($node->byRef) { + $targets[] = $node->expr; + } + if ($node->keyVar !== null) { + $targets[] = $node->keyVar; + } + $targets[] = $node->valueVar; + } elseif ($node instanceof Stmt\Catch_) { + if ($node->var !== null) { + $targets[] = $node->var; + } + } elseif ($node instanceof Stmt\Static_) { + foreach ($node->vars as $staticVar) { + $targets[] = $staticVar->var; + } + } elseif ($node instanceof Stmt\Global_ || $node instanceof Stmt\Unset_) { + foreach ($node->vars as $var) { + $targets[] = $var; + } + } elseif ($node instanceof Expr\Closure) { + foreach ($node->uses as $use) { + if (!$use->byRef) { + continue; + } + $targets[] = $use->var; + } + } + + foreach ($targets as $target) { + $targetNames = self::getTargetNames($target); + if ($targetNames === null) { + return null; + } + foreach ($targetNames as $targetName) { + $names[$targetName] = true; + } + } + + foreach ($node->getSubNodeNames() as $subNodeName) { + $subNode = $node->$subNodeName; + if ($subNode instanceof Node) { + $nodes[] = $subNode; + continue; + } + if (!is_array($subNode)) { + continue; + } + foreach ($subNode as $subNodeItem) { + if (!$subNodeItem instanceof Node) { + continue; + } + $nodes[] = $subNodeItem; + } + } + } + + return $names; + } + + /** + * @return list|null + */ + private static function getTargetNames(Expr $target): ?array + { + while ($target instanceof Expr\ArrayDimFetch || $target instanceof Expr\PropertyFetch || $target instanceof Expr\NullsafePropertyFetch) { + $target = $target->var; + } + if ($target instanceof Expr\Variable) { + return is_string($target->name) ? [$target->name] : null; + } + if (!$target instanceof Expr\List_ && !$target instanceof Expr\Array_) { + return []; + } + + $names = []; + foreach ($target->items as $item) { + if ($item === null) { + continue; + } + $itemNames = self::getTargetNames($item->value); + if ($itemNames === null) { + return null; + } + foreach ($itemNames as $itemName) { + $names[] = $itemName; + } + } + + return $names; + } + +} diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 13fd2e8f77..cdb27461f2 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -4773,20 +4773,47 @@ public function processAlwaysIterableForeachScopeWithoutPollute(self $finalScope ); } - public function generalizeWith(self $otherScope): self + /** + * @param array|null $writableVariableNames variables the loop can write, null when unknown + */ + public function generalizeWith(self $otherScope, ?array $writableVariableNames = null): self { - return $this->generalizeWithVariableState($otherScope)->addTemplateArgumentConstraints($otherScope->getTemplateArgumentConstraints()); + return $this->generalizeWithVariableState($otherScope, $writableVariableNames)->addTemplateArgumentConstraints($otherScope->getTemplateArgumentConstraints()); } - private function generalizeWithVariableState(self $otherScope): self - { + /** + * @param array|null $writableVariableNames + */ + private function generalizeWithVariableState(self $otherScope, ?array $writableVariableNames): self + { + if ($writableVariableNames !== null) { + // a reference created before the loop lets the loop write a variable it does not name + foreach ([$this->expressionTypes, $otherScope->expressionTypes] as $expressionTypes) { + foreach ($expressionTypes as $expressionTypeHolder) { + $intertwinedExpr = $expressionTypeHolder->getExpr(); + if (!$intertwinedExpr instanceof IntertwinedVariableByReferenceWithExpr) { + continue; + } + $writableVariableNames[$intertwinedExpr->getVariableName()] = true; + foreach ([$intertwinedExpr->getExpr(), $intertwinedExpr->getAssignedExpr()] as $aliasedExpr) { + $aliasedVariableName = ScopeOps::getIntertwinedRefRootVariableName($aliasedExpr); + if ($aliasedVariableName === null) { + continue; + } + $writableVariableNames[$aliasedVariableName] = true; + } + } + } + } $variableTypeHolders = $this->generalizeVariableTypeHolders( $this->expressionTypes, $otherScope->expressionTypes, + $writableVariableNames, ); $nativeTypes = $this->generalizeVariableTypeHolders( $this->nativeExpressionTypes, $otherScope->nativeExpressionTypes, + $writableVariableNames, ); return $this->scopeFactory->create( @@ -4814,11 +4841,13 @@ private function generalizeWithVariableState(self $otherScope): self /** * @param array $variableTypeHolders * @param array $otherVariableTypeHolders + * @param array|null $writableVariableNames * @return array */ private function generalizeVariableTypeHolders( array $variableTypeHolders, array $otherVariableTypeHolders, + ?array $writableVariableNames, ): array { uksort($variableTypeHolders, static fn (string $exprA, string $exprB): int => strlen($exprA) <=> strlen($exprB)); @@ -4838,7 +4867,18 @@ private function generalizeVariableTypeHolders( continue; } - $generalizedType = $this->generalizeType($variableTypeHolder->getType(), $otherVariableTypeHolders[$variableExprString]->getType(), 0); + $variableExpr = $variableTypeHolder->getExpr(); + if ( + $writableVariableNames !== null + && $variableExpr instanceof Variable + && is_string($variableExpr->name) + && !isset($writableVariableNames[$variableExpr->name]) + ) { + // the loop does not write this variable, its types differ between passes only by narrowing + $generalizedType = TypeCombinator::union($variableTypeHolder->getType(), $otherVariableTypeHolders[$variableExprString]->getType()); + } else { + $generalizedType = $this->generalizeType($variableTypeHolder->getType(), $otherVariableTypeHolders[$variableExprString]->getType(), 0); + } if ( !$generalizedType->equals($variableTypeHolder->getType()) ) { diff --git a/src/Analyser/StmtHandler/DoWhileHandler.php b/src/Analyser/StmtHandler/DoWhileHandler.php index 4124c17bcb..ae63fdc64e 100644 --- a/src/Analyser/StmtHandler/DoWhileHandler.php +++ b/src/Analyser/StmtHandler/DoWhileHandler.php @@ -8,6 +8,7 @@ use PHPStan\Analyser\ExpressionContext; use PHPStan\Analyser\ExpressionResultStorage; use PHPStan\Analyser\InternalStatementResult; +use PHPStan\Analyser\LoopWrittenVariableNames; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\NoopNodeCallback; @@ -83,22 +84,24 @@ public function processStmt( $replayPassStorage = $storage; $replayPassResult = $bodyScopeResult; } - if ($backEdgeScope !== null) { - $bodyScope = $nodeScopeResolver->processExprNode($stmt, $stmt->cond, $backEdgeScope, $storage, new NoopNodeCallback(), ExpressionContext::createDeep(resolveTemplateArguments: false))->getTruthyScope(); + if ($backEdgeScope === null) { + $bodyScope = $prevScope; + break; } + $passCondResult = $nodeScopeResolver->processExprNode($stmt, $stmt->cond, $backEdgeScope, $storage, new NoopNodeCallback(), ExpressionContext::createDeep(resolveTemplateArguments: false)); + $bodyScope = $passCondResult->getTruthyScope(); } finally { $scope->popExpressionResultStorage(); } - if ($backEdgeScope === null) { - $bodyScope = $prevScope; - break; - } if ($bodyScope->equals($prevScope)) { break; } if ($count >= NodeScopeResolver::GENERALIZE_AFTER_ITERATION) { - $bodyScope = $prevScope->generalizeWith($bodyScope); + $bodyScope = $prevScope->generalizeWith( + $bodyScope, + LoopWrittenVariableNames::collect($stmt, VariableFlow::sequence($bodyScopeResult->getVariableFlow(), $passCondResult->getVariableFlow())), + ); } $count++; } while ($count < NodeScopeResolver::LOOP_SCOPE_ITERATIONS); diff --git a/src/Analyser/StmtHandler/ForHandler.php b/src/Analyser/StmtHandler/ForHandler.php index 45bdb70b06..445afa016b 100644 --- a/src/Analyser/StmtHandler/ForHandler.php +++ b/src/Analyser/StmtHandler/ForHandler.php @@ -16,6 +16,7 @@ use PHPStan\Analyser\ExpressionContext; use PHPStan\Analyser\ExpressionResultStorage; use PHPStan\Analyser\InternalStatementResult; +use PHPStan\Analyser\LoopWrittenVariableNames; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\NoopNodeCallback; @@ -195,10 +196,14 @@ public function processStmt( $prevEntryScope = $bodyScope; $scope->pushExpressionResultStorage($storage); try { + $passFlows = []; if ($lastCondExpr !== null) { - $bodyScope = $nodeScopeResolver->processExprNode($stmt, $lastCondExpr, $bodyScope, $storage, new NoopNodeCallback(), ExpressionContext::createDeep(resolveTemplateArguments: false))->getTruthyScope(); + $passCondResult = $nodeScopeResolver->processExprNode($stmt, $lastCondExpr, $bodyScope, $storage, new NoopNodeCallback(), ExpressionContext::createDeep(resolveTemplateArguments: false)); + $bodyScope = $passCondResult->getTruthyScope(); + $passFlows[] = $passCondResult->getVariableFlow(); } $bodyScopeResult = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, new NoopNodeCallback(), $context->enterDeep()->withoutTemplateArgumentResolution())->filterOutLoopExitPoints(); + $passFlows[] = $bodyScopeResult->getVariableFlow(); $backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope(); if ($backEdgeScope === null) { $bodyScope = $prevScope; @@ -209,6 +214,7 @@ public function processStmt( foreach ($stmt->loop as $loopExpr) { $exprResult = $nodeScopeResolver->processExprNode($stmt, $loopExpr, $bodyScope, $storage, new NoopNodeCallback(), ExpressionContext::createTopLevel(resolveTemplateArguments: false)); $bodyScope = $exprResult->getScope(); + $passFlows[] = $exprResult->getVariableFlow(); $hasYield = $hasYield || $exprResult->hasYield(); $throwPoints = array_merge($throwPoints, $exprResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $exprResult->getImpurePoints()); @@ -222,7 +228,7 @@ public function processStmt( } if ($count >= NodeScopeResolver::GENERALIZE_AFTER_ITERATION) { - $bodyScope = $prevScope->generalizeWith($bodyScope); + $bodyScope = $prevScope->generalizeWith($bodyScope, LoopWrittenVariableNames::collect($stmt, VariableFlow::sequence(...$passFlows))); } $count++; } while ($count < NodeScopeResolver::LOOP_SCOPE_ITERATIONS); diff --git a/src/Analyser/StmtHandler/ForeachHandler.php b/src/Analyser/StmtHandler/ForeachHandler.php index 6271e4cd4e..9d8921de38 100644 --- a/src/Analyser/StmtHandler/ForeachHandler.php +++ b/src/Analyser/StmtHandler/ForeachHandler.php @@ -26,6 +26,7 @@ use PHPStan\Analyser\ExprHandler\Helper\IdenticalNarrowingHelper; use PHPStan\Analyser\InternalStatementResult; use PHPStan\Analyser\InternalThrowPoint; +use PHPStan\Analyser\LoopWrittenVariableNames; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\NoopNodeCallback; @@ -254,7 +255,7 @@ static function () use ($condResult, $emptyArrayType): Type { } if ($count >= NodeScopeResolver::GENERALIZE_AFTER_ITERATION) { - $bodyScope = $prevScope->generalizeWith($bodyScope); + $bodyScope = $prevScope->generalizeWith($bodyScope, LoopWrittenVariableNames::collect($stmt, $bodyScopeResult->getVariableFlow())); } $count++; } while ($count < NodeScopeResolver::LOOP_SCOPE_ITERATIONS); @@ -844,7 +845,7 @@ private function tryProcessUnrolledConstantArrayForeach( break; } if ($count >= NodeScopeResolver::GENERALIZE_AFTER_ITERATION) { - $loopScope = $prevLoopScope->generalizeWith($loopScope); + $loopScope = $prevLoopScope->generalizeWith($loopScope, LoopWrittenVariableNames::collect($stmt, $iterBodyScopeResult->getVariableFlow())); } $count++; } while ($count < NodeScopeResolver::LOOP_SCOPE_ITERATIONS); diff --git a/src/Analyser/StmtHandler/WhileHandler.php b/src/Analyser/StmtHandler/WhileHandler.php index 469b2c833f..9aa8437b9f 100644 --- a/src/Analyser/StmtHandler/WhileHandler.php +++ b/src/Analyser/StmtHandler/WhileHandler.php @@ -8,6 +8,7 @@ use PHPStan\Analyser\ExpressionContext; use PHPStan\Analyser\ExpressionResultStorage; use PHPStan\Analyser\InternalStatementResult; +use PHPStan\Analyser\LoopWrittenVariableNames; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\NoopNodeCallback; @@ -122,7 +123,10 @@ public function processStmt( } if ($count >= NodeScopeResolver::GENERALIZE_AFTER_ITERATION) { - $bodyScope = $prevScope->generalizeWith($bodyScope); + $bodyScope = $prevScope->generalizeWith( + $bodyScope, + LoopWrittenVariableNames::collect($stmt, VariableFlow::sequence($passCondResult->getVariableFlow(), $bodyScopeResult->getVariableFlow())), + ); } $count++; } while ($count < NodeScopeResolver::LOOP_SCOPE_ITERATIONS); diff --git a/tests/PHPStan/Analyser/nsrt/bug-12666.php b/tests/PHPStan/Analyser/nsrt/bug-12666.php new file mode 100644 index 0000000000..93beb4bab8 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-12666.php @@ -0,0 +1,160 @@ + $x0 + * @param int<1,max> $xn + */ +function add(int $x0, int $xn): float +{ + $xi = $x0; + + assertType('int<1, max>', $xn); + + while ($xi < $xn) { + $xi += 1; + } + + assertType('int<1, max>', $xn); + + return $xi; +} + +/** + * @param int<1,max> $x0 + * @param int<1,max> $xn + */ +function addFrac(int $x0, int $xn): float +{ + $xi = $x0; + + assertType('int<1, max>', $xn); + + while ($xi < $xn) { + $xi += 0.1; + } + + assertType('int<1, max>', $xn); + + return $xi; +} + +/** + * @param int<1,max> $x0 + * @param int<1,max> $xn + */ +function sub(int $x0, int $xn): float +{ + $xi = $x0; + + assertType('int<1, max>', $xn); + + while ($xi < $xn) { + $xi -= 1; + } + + assertType('int<1, max>', $xn); + + return $xi; +} + +/** + * @param int<1,max> $x0 + * @param int<1,max> $xn + */ +function subFrac(int $x0, int $xn): float +{ + $xi = $x0; + + assertType('int<1, max>', $xn); + + while ($xi < $xn) { + $xi -= 0.1; + } + + assertType('int<1, max>', $xn); + + return $xi; +} + +/** + * @param int<1,max> $x0 + * @param int<1,max> $xn + */ +function mul(int $x0, int $xn): float +{ + $xi = $x0; + + assertType('int<1, max>', $xn); + + while ($xi < $xn) { + $xi *= 2; + } + + assertType('int<1, max>', $xn); + + return $xi; +} + +/** + * @param int<1,max> $x0 + * @param int<1,max> $xn + */ +function mulFrac(int $x0, int $xn): float +{ + $xi = $x0; + + assertType('int<1, max>', $xn); + + while ($xi < $xn) { + $xi *= 1.1; + } + + assertType('int<1, max>', $xn); + + return $xi; +} + +/** + * @param int<1,max> $x0 + * @param int<1,max> $xn + */ +function div(int $x0, int $xn): float +{ + $xi = $x0; + + assertType('int<1, max>', $xn); + + while ($xi < $xn) { + $xi /= 2; + } + + assertType('int<1, max>', $xn); + + return $xi; +} + +/** + * @param int<1,max> $x0 + * @param int<1,max> $xn + */ +function divFrac(int $x0, int $xn): float +{ + $xi = $x0; + + assertType('int<1, max>', $xn); + + while ($xi < $xn) { + $xi /= 1.1; + } + + assertType('int<1, max>', $xn); + + return $xi; +} diff --git a/tests/PHPStan/Analyser/nsrt/loop-generalize-written-variables.php b/tests/PHPStan/Analyser/nsrt/loop-generalize-written-variables.php new file mode 100644 index 0000000000..b14995a586 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/loop-generalize-written-variables.php @@ -0,0 +1,134 @@ + $x0 + * @param int<1,max> $xn + */ +function doWhileNarrowedOnFirstPass(int $x0, int $xn): void +{ + $xi = $x0; + do { + $previous = $xi; + $xi += 0.1; + } while ($previous < $xn); + assertType('int<1, max>', $xn); +} + +/** + * @param int<1,max> $x0 + * @param int<1,max> $xn + */ +function forLoop(int $x0, int $xn): void +{ + for ($xi = $x0; $xi < $xn; $xi += 0.1) { + assertType('int<1, max>', $xn); + } + assertType('int<1, max>', $xn); +} + +/** + * @param int<1,max> $x0 + * @param int<1,max> $xn + * @param list $items + */ +function foreachLoop(int $x0, int $xn, array $items): void +{ + $xi = $x0; + foreach ($items as $item) { + if ($xi >= $xn) { + break; + } + $xi += 0.1; + } + assertType('int<1, max>', $xn); +} + +/** + * @param array{1, ...} $items + * @param int<1,max> $x0 + * @param int<1,max> $xn + */ +function unsealedConstantArrayForeach(array $items, int $x0, int $xn): void +{ + $xi = $x0; + $previous = $x0; + foreach ($items as $item) { + if ($previous >= $xn) { + break; + } + $previous = $xi; + $xi += 0.1; + } + assertType('int<1, max>', $xn); +} + +/** + * @param int<1,max> $x0 + * @param int<1,max> $xn + */ +function passedByValueInBody(int $x0, int $xn): void +{ + $xi = $x0; + while ($xi < $xn) { + takesInt($xn); + $xi += 0.1; + } + assertType('int<1, max>', $xn); +} + +/** + * @param int<0,max> $i + */ +function incrementedVariableIsWidened(int $i): void +{ + while ($i < 10) { + $i++; + } + assertType('int<10, max>', $i); +} + +function destructuredVariableIsWidened(): void +{ + $xn = 1; + while (rand(0, 1)) { + [$xn] = [$xn + 1]; + } + assertType('int<1, max>', $xn); +} + +function variableWrittenThroughReferenceIsWidened(): void +{ + $xn = 1; + $ref = &$xn; + while (rand(0, 1)) { + $ref = $xn + 1; + } + assertType('int<1, max>', $xn); +} + +function variablePassedByReferenceIsWidened(): void +{ + $arr = []; + $i = 0; + while ($i < 5) { + array_push($arr, $i); + $i++; + } + assertType('non-empty-list>', $arr); +} + +function variablePassedByReferenceInForUpdateIsWidened(): void +{ + $arr = []; + for ($i = 0; $i < 5; $i++, array_push($arr, $i)) { + } + assertType('non-empty-list>', $arr); +}