diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 868f77f1f6..dbceccc51a 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -809,7 +809,7 @@ private function processStmtNode( $finalScope, $finalScopeResult->hasYield() || $condResult->hasYield(), $isIterableAtLeastOnce->yes() && $finalScopeResult->isAlwaysTerminating(), - [] + $finalScopeResult->getExitPointsForOuterLoop() ); } elseif ($stmt instanceof While_) { $condResult = $this->processExprNode($stmt->cond, $scope, static function (): void { @@ -875,7 +875,7 @@ private function processStmtNode( $finalScope, $finalScopeResult->hasYield() || $condResult->hasYield(), $isAlwaysTerminating, - [] + $finalScopeResult->getExitPointsForOuterLoop() ); } elseif ($stmt instanceof Do_) { $finalScope = null; @@ -940,7 +940,7 @@ private function processStmtNode( $finalScope, $bodyScopeResult->hasYield() || $hasYield, $alwaysTerminating, - [] + $bodyScopeResult->getExitPointsForOuterLoop() ); } elseif ($stmt instanceof For_) { $initScope = $scope; @@ -1014,7 +1014,7 @@ private function processStmtNode( $finalScope, $finalScopeResult->hasYield() || $hasYield, false/* $finalScopeResult->isAlwaysTerminating() && $isAlwaysIterable*/, - [] + $finalScopeResult->getExitPointsForOuterLoop() ); } elseif ($stmt instanceof Switch_) { $condResult = $this->processExprNode($stmt->cond, $scope, $nodeCallback, ExpressionContext::createDeep()); @@ -1025,6 +1025,7 @@ private function processStmtNode( $hasDefaultCase = false; $alwaysTerminating = true; $hasYield = $condResult->hasYield(); + $exitPointsForOuterLoop = []; foreach ($stmt->cases as $caseNode) { if ($caseNode->cond !== null) { $condExpr = new BinaryOp\Equal($stmt->cond, $caseNode->cond); @@ -1047,6 +1048,7 @@ private function processStmtNode( foreach ($branchScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) { $finalScope = $continueExitPoint->getScope()->mergeWith($finalScope); } + $exitPointsForOuterLoop = array_merge($exitPointsForOuterLoop, $branchFinalScopeResult->getExitPointsForOuterLoop()); if ($branchScopeResult->isAlwaysTerminating()) { $alwaysTerminating = $alwaysTerminating && $branchFinalScopeResult->isAlwaysTerminating(); $prevScope = null; @@ -1074,7 +1076,7 @@ private function processStmtNode( $finalScope = $scope->mergeWith($finalScope); } - return new StatementResult($finalScope, $hasYield, $alwaysTerminating, []); + return new StatementResult($finalScope, $hasYield, $alwaysTerminating, $exitPointsForOuterLoop); } elseif ($stmt instanceof TryCatch) { $branchScopeResult = $this->processStmtNodes($stmt, $stmt->stmts, $scope, $nodeCallback); $branchScope = $branchScopeResult->getScope(); diff --git a/src/Analyser/StatementResult.php b/src/Analyser/StatementResult.php index e589fb9434..8e6f3c1d6c 100644 --- a/src/Analyser/StatementResult.php +++ b/src/Analyser/StatementResult.php @@ -2,6 +2,7 @@ namespace PHPStan\Analyser; +use PhpParser\Node\Scalar\LNumber; use PhpParser\Node\Stmt; class StatementResult @@ -58,12 +59,20 @@ public function filterOutLoopExitPoints(): self foreach ($this->exitPoints as $exitPoint) { $statement = $exitPoint->getStatement(); - if ( - $statement instanceof Stmt\Break_ - || $statement instanceof Stmt\Continue_ - ) { + if (!$statement instanceof Stmt\Break_ && !$statement instanceof Stmt\Continue_) { + continue; + } + + $num = $statement->num; + if (!$num instanceof LNumber) { return new self($this->scope, $this->hasYield, false, $this->exitPoints); } + + if ($num->value !== 1) { + continue; + } + + return new self($this->scope, $this->hasYield, false, $this->exitPoints); } return $this; @@ -78,14 +87,31 @@ public function getExitPoints(): array } /** - * @param string $stmtClass + * @param class-string|class-string $stmtClass * @return StatementExitPoint[] */ public function getExitPointsByType(string $stmtClass): array { $exitPoints = []; foreach ($this->exitPoints as $exitPoint) { - if (!$exitPoint->getStatement() instanceof $stmtClass) { + $statement = $exitPoint->getStatement(); + if (!$statement instanceof $stmtClass) { + continue; + } + + $value = $statement->num; + if ($value === null) { + $exitPoints[] = $exitPoint; + continue; + } + + if (!$value instanceof LNumber) { + $exitPoints[] = $exitPoint; + continue; + } + + $value = $value->value; + if ($value !== 1) { continue; } @@ -95,4 +121,42 @@ public function getExitPointsByType(string $stmtClass): array return $exitPoints; } + /** + * @return StatementExitPoint[] + */ + public function getExitPointsForOuterLoop(): array + { + $exitPoints = []; + foreach ($this->exitPoints as $exitPoint) { + $statement = $exitPoint->getStatement(); + if (!$statement instanceof Stmt\Continue_ && !$statement instanceof Stmt\Break_) { + continue; + } + if ($statement->num === null) { + continue; + } + if (!$statement->num instanceof LNumber) { + continue; + } + $value = $statement->num->value; + if ($value === 1) { + continue; + } + + $newNode = null; + if ($value > 2) { + $newNode = new LNumber($value - 1); + } + if ($statement instanceof Stmt\Continue_) { + $newStatement = new Stmt\Continue_($newNode); + } else { + $newStatement = new Stmt\Break_($newNode); + } + + $exitPoints[] = new StatementExitPoint($newStatement, $exitPoint->getScope()); + } + + return $exitPoints; + } + } diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index 0ace2ff0c1..eebb2f7066 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -10777,6 +10777,31 @@ public function dataBug3777(): array return $this->gatherAssertTypes(__DIR__ . '/../Rules/Properties/data/bug-3777.php'); } + public function dataBug2549(): array + { + return $this->gatherAssertTypes(__DIR__ . '/data/bug-2549.php'); + } + + public function dataBug1945(): array + { + return $this->gatherAssertTypes(__DIR__ . '/data/bug-1945.php'); + } + + public function dataBug2003(): array + { + return $this->gatherAssertTypes(__DIR__ . '/data/bug-2003.php'); + } + + public function dataBug651(): array + { + return $this->gatherAssertTypes(__DIR__ . '/data/bug-651.php'); + } + + public function dataBug1283(): array + { + return $this->gatherAssertTypes(__DIR__ . '/data/bug-1283.php'); + } + /** * @param string $file * @return array @@ -10994,6 +11019,11 @@ private function gatherAssertTypes(string $file): array * @dataProvider dataBug4504 * @dataProvider dataBug4436 * @dataProvider dataBug3777 + * @dataProvider dataBug2549 + * @dataProvider dataBug1945 + * @dataProvider dataBug2003 + * @dataProvider dataBug651 + * @dataProvider dataBug1283 * @param string $assertType * @param string $file * @param mixed ...$args diff --git a/tests/PHPStan/Analyser/data/bug-1283.php b/tests/PHPStan/Analyser/data/bug-1283.php new file mode 100644 index 0000000000..f124e47c98 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-1283.php @@ -0,0 +1,27 @@ + 1, ?1 => 3)', $allowedElements); + assertVariableCertainty(TrinaryLogic::createYes(), $allowedElements); + } +}; diff --git a/tests/PHPStan/Analyser/data/bug-1945.php b/tests/PHPStan/Analyser/data/bug-1945.php new file mode 100644 index 0000000000..17b29940b7 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-1945.php @@ -0,0 +1,106 @@ +analyse([__DIR__ . '/data/bug-4076.php'], []); } + public function testBug4535(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-4535.php'], []); + } + + public function testBug4346(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-4346.php'], []); + } + + public function testBug2913(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-2913.php'], []); + } + } diff --git a/tests/PHPStan/Rules/DeadCode/data/bug-2913.php b/tests/PHPStan/Rules/DeadCode/data/bug-2913.php new file mode 100644 index 0000000000..de3055094f --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/data/bug-2913.php @@ -0,0 +1,19 @@ +