diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 507987a2a5..1f0b7a4cb0 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4,6 +4,7 @@ use ArrayAccess; use Closure; +use IteratorAggregate; use Override; use PhpParser\Comment\Doc; use PhpParser\Modifiers; @@ -1437,8 +1438,9 @@ public function processStmtNode( $throwPoints = array_merge($throwPoints, $finalScopeResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $finalScopeResult->getImpurePoints()); } - if (!(new ObjectType(Traversable::class))->isSuperTypeOf($scope->getType($stmt->expr))->no()) { - $throwPoints[] = InternalThrowPoint::createImplicit($scope, $stmt->expr); + $traversableThrowPoint = $this->getTraversableForeachThrowPoint($scope, $stmt->expr); + if ($traversableThrowPoint !== null) { + $throwPoints[] = $traversableThrowPoint; } if ($context->isTopLevel() && $stmt->byRef) { $finalScope = $finalScope->assignExpression(new ForeachValueByRefExpr($stmt->valueVar), new MixedType(), new MixedType()); @@ -4031,6 +4033,37 @@ private function tryProcessUnrolledConstantArrayForeach( return ['bodyScope' => $bodyScope, 'endScope' => $endScope]; } + private function getTraversableForeachThrowPoint(MutatingScope $scope, Expr $iteratee): ?InternalThrowPoint + { + $exprType = $scope->getType($iteratee); + $traversableType = new ObjectType(Traversable::class); + + if ($traversableType->isSuperTypeOf($exprType)->no()) { + return null; + } + + $traversablePart = TypeCombinator::intersect($exprType, $traversableType); + $iteratorAggregateType = new ObjectType(IteratorAggregate::class); + + if ($iteratorAggregateType->isSuperTypeOf($traversablePart)->yes() + && $traversablePart->hasMethod('getIterator')->yes()) { + $method = $traversablePart->getMethod('getIterator', $scope); + $throwType = $method->getThrowType(); + if ($throwType !== null) { + if ($throwType->isVoid()->yes()) { + return null; + } + return InternalThrowPoint::createExplicit($scope, $throwType, $iteratee, true); + } + + if (!$this->implicitThrows) { + return null; + } + } + + return InternalThrowPoint::createImplicit($scope, $iteratee); + } + /** * @param callable(Node $node, Scope $scope): void $nodeCallback */ diff --git a/tests/PHPStan/Analyser/nsrt/bug-6833.php b/tests/PHPStan/Analyser/nsrt/bug-6833.php new file mode 100644 index 0000000000..3affcd9c6e --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-6833.php @@ -0,0 +1,208 @@ += 8.0 + +declare(strict_types=1); + +namespace Bug6833; + +use PHPStan\TrinaryLogic; +use function PHPStan\Testing\assertVariableCertainty; + +class File +{ + public function __construct(private int $id) {} + public function getId(): int { return $this->id; } +} + +/** + * @implements \IteratorAggregate + */ +class FileCollection implements \IteratorAggregate +{ + /** @var File[] */ + private array $files = []; + + public function add(File $file): void + { + $this->files[] = $file; + } + + /** @throws void */ + public function getIterator(): \Iterator + { + return new \ArrayIterator($this->files); + } +} + +function testThrowsVoidOnGetIterator(FileCollection $files): void +{ + try { + foreach ($files as $file) { + echo $file->getId(); + } + } catch (\Throwable) { + assertVariableCertainty(TrinaryLogic::createYes(), $file); + echo 'Invalid file:' . $file->getId(); + } +} + +/** + * @implements \IteratorAggregate + */ +class FileCollectionWithoutThrowsVoid implements \IteratorAggregate +{ + /** @var File[] */ + private array $files = []; + + public function getIterator(): \Iterator + { + return new \ArrayIterator($this->files); + } +} + +function testWithoutThrowsVoid(FileCollectionWithoutThrowsVoid $files): void +{ + try { + foreach ($files as $file) { + echo $file->getId(); + } + } catch (\Throwable) { + assertVariableCertainty(TrinaryLogic::createMaybe(), $file); + echo $file->getId(); // error - getIterator() could throw + } +} + +/** + * @implements \IteratorAggregate + */ +class FileCollectionExplicitThrows implements \IteratorAggregate +{ + /** @var File[] */ + private array $files = []; + + /** @throws \RuntimeException */ + public function getIterator(): \Iterator + { + return new \ArrayIterator($this->files); + } +} + +function testExplicitThrowsMatchingCatch(FileCollectionExplicitThrows $files): void +{ + try { + foreach ($files as $file) { + echo $file->getId(); + } + } catch (\Throwable) { + assertVariableCertainty(TrinaryLogic::createMaybe(), $file); + echo $file->getId(); // error - getIterator() can throw RuntimeException + } +} + +function testExplicitThrowsNonMatchingCatch(FileCollectionExplicitThrows $files): void +{ + try { + foreach ($files as $file) { + if ($file->getId() < 0) { + throw new \LogicException('negative'); + } + } + } catch (\LogicException) { + echo $file->getId(); // no error - RuntimeException doesn't match LogicException catch + } +} + +/** @param File[] $files */ +function testArrayForeach(array $files): void +{ + try { + foreach ($files as $file) { + echo $file->getId(); + } + } catch (\Throwable) { + assertVariableCertainty(TrinaryLogic::createYes(), $file); + echo $file->getId(); // no error - arrays don't call getIterator() + } +} + +function testThrowsVoidFinallyScope(FileCollection $files): void +{ + try { + foreach ($files as $file) { + doSomething(); + } + } finally { + assertVariableCertainty(TrinaryLogic::createMaybe(), $file); + } +} + +/** @param File[]|FileCollection $files */ +function testArrayOrThrowsVoid(array|FileCollection $files): void +{ + try { + foreach ($files as $file) { + echo $file->getId(); + } + } catch (\Throwable) { + assertVariableCertainty(TrinaryLogic::createYes(), $file); + echo $file->getId(); // no error - array doesn't throw, getIterator() has @throws void + } +} + +/** @param File[]|FileCollectionExplicitThrows $files */ +function testArrayOrExplicitThrows(array|FileCollectionExplicitThrows $files): void +{ + try { + foreach ($files as $file) { + echo $file->getId(); + } + } catch (\Throwable) { + assertVariableCertainty(TrinaryLogic::createMaybe(), $file); + echo $file->getId(); // error - getIterator() can throw RuntimeException + } +} + +/** @param File[]|FileCollectionWithoutThrowsVoid $files */ +function testArrayOrNoAnnotation(array|FileCollectionWithoutThrowsVoid $files): void +{ + try { + foreach ($files as $file) { + echo $file->getId(); + } + } catch (\Throwable) { + assertVariableCertainty(TrinaryLogic::createMaybe(), $file); + echo $file->getId(); // error - getIterator() could throw + } +} + +function doSomething(): void {} + +/** + * @implements \IteratorAggregate + */ +class MaybeThrowingCollection implements \IteratorAggregate +{ + /** @var File[] */ + private array $files = []; + + public function add(File $file): void + { + $this->files[] = $file; + } + + public function getIterator(): \Iterator + { + return new \ArrayIterator($this->files); + } +} + +function testUnionThrowsMatchingCatch(FileCollectionExplicitThrows|MaybeThrowingCollection $files): void +{ + try { + foreach ($files as $file) { + echo $file->getId(); + } + } catch (\Throwable) { + assertVariableCertainty(TrinaryLogic::createMaybe(), $file); + echo $file->getId(); // error - getIterator() can throw RuntimeException + } +} diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index b6cc5f03bc..67f77fa6bf 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -1571,4 +1571,59 @@ public function testBug10729(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testBug6833(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = true; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-6833.php'], [ + [ + 'Variable $file might not be defined.', + 69, + ], + [ + 'Variable $file might not be defined.', + 70, + ], + [ + 'Variable $file might not be defined.', + 96, + ], + [ + 'Variable $file might not be defined.', + 97, + ], + [ + 'Variable $file might not be defined.', + 134, + ], + [ + 'Variable $file might not be defined.', + 159, + ], + [ + 'Variable $file might not be defined.', + 160, + ], + [ + 'Variable $file might not be defined.', + 172, + ], + [ + 'Variable $file might not be defined.', + 173, + ], + [ + 'Variable $file might not be defined.', + 205, + ], + [ + 'Variable $file might not be defined.', + 206, + ], + ]); + } + }