From c3389dc559c904771349dc3d3208e949e83391f0 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Sun, 26 Jul 2026 10:34:06 +0900 Subject: [PATCH] Narrow foreach body to non-empty behind an opt-in toggle Inside a foreach body the iterated expression is provably non-empty, so it can be narrowed (list to non-empty-list, array to non-empty-array) at body entry. Before, this happened only when polluteScopeWithAlwaysIterableForeach was on, so with the flag off (as phpstan-strict-rules sets it) the narrowing disappeared even though the body is only entered when the iteratee is non-empty. The narrowing is gated behind the new feature toggle narrowForeachBodyNonEmpty, off by default. With it off the body scope is built exactly as before, so no behaviour changes. With it on, the body narrows regardless of the flag. Narrowing the body also feeds the loop-exit scope, which with the flag off must stay conservative: it must not conclude the loop always iterated. After the body pass the iterated expression's possibly-empty-ness is restored in the after-loop scope, keeping any element types the body refined. This containment is partial: definedness that flows through variables the body builds (rather than through the iterated expression itself) still reaches the after-loop scope. Because of that residual, the toggle is not enabled under bleedingEdge yet; it is opt-in until the leak is fully contained. Closes phpstan/phpstan#13312 --- conf/config.neon | 1 + conf/parametersSchema.neon | 1 + src/Analyser/NodeScopeResolver.php | 41 +++++++++++++++++-- src/Testing/RuleTestCase.php | 1 + src/Testing/TypeInferenceTestCase.php | 1 + tests/PHPStan/Analyser/AnalyserTest.php | 1 + .../Analyser/Bug13312NoPolluteTest.php | 36 ++++++++++++++++ tests/PHPStan/Analyser/Bug13312StableTest.php | 36 ++++++++++++++++ .../Fiber/FiberNodeScopeResolverRuleTest.php | 1 + .../Fiber/FiberNodeScopeResolverTest.php | 1 + .../Analyser/bug-13312-no-pollute.neon | 4 ++ tests/PHPStan/Analyser/bug-13312-stable.neon | 2 + .../Analyser/data/bug-13312-no-pollute.php | 37 +++++++++++++++++ .../Analyser/data/bug-13312-stable.php | 32 +++++++++++++++ tests/PHPStan/Analyser/nsrt/bug-13312.php | 18 ++++++++ 15 files changed, 210 insertions(+), 3 deletions(-) create mode 100644 tests/PHPStan/Analyser/Bug13312NoPolluteTest.php create mode 100644 tests/PHPStan/Analyser/Bug13312StableTest.php create mode 100644 tests/PHPStan/Analyser/bug-13312-no-pollute.neon create mode 100644 tests/PHPStan/Analyser/bug-13312-stable.neon create mode 100644 tests/PHPStan/Analyser/data/bug-13312-no-pollute.php create mode 100644 tests/PHPStan/Analyser/data/bug-13312-stable.php diff --git a/conf/config.neon b/conf/config.neon index f64d549182..e9089e1137 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -41,6 +41,7 @@ parameters: rawMessageInBaseline: false reportNestedTooWideType: false assignToByRefForeachExpr: false + narrowForeachBodyNonEmpty: false curlSetOptArrayTypes: false magicDirInInclude: false checkDateIntervalConstructor: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index cf5d3b4ef6..bda5e214c6 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -44,6 +44,7 @@ parametersSchema: rawMessageInBaseline: bool() reportNestedTooWideType: bool() assignToByRefForeachExpr: bool() + narrowForeachBodyNonEmpty: bool() curlSetOptArrayTypes: bool() magicDirInInclude: bool() checkDateIntervalConstructor: bool() diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 7cb8aa2805..1146e8d807 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -142,6 +142,7 @@ use PHPStan\ShouldNotHappenException; use PHPStan\TrinaryLogic; use PHPStan\Type\ClosureType; +use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\FileTypeMapper; @@ -266,6 +267,8 @@ public function __construct( private readonly bool $polluteScopeWithLoopInitialAssignments, #[AutowiredParameter] private readonly bool $polluteScopeWithAlwaysIterableForeach, + #[AutowiredParameter(ref: '%featureToggles.narrowForeachBodyNonEmpty%')] + private readonly bool $narrowForeachBodyNonEmpty, #[AutowiredParameter] private readonly bool $polluteScopeWithBlock, #[AutowiredParameter] @@ -1487,6 +1490,11 @@ public function processStmtNode( $stmt->expr, new Array_([]), ); + // The loop body is only entered when the iteratee is non-empty. Under + // narrowForeachBodyNonEmpty we narrow it there (list to non-empty-list, + // array to non-empty-array) even with polluteScopeWithAlwaysIterableForeach + // off; with the toggle off the body scope is unchanged. + $narrowForeachBody = $this->narrowForeachBodyNonEmpty || $this->polluteScopeWithAlwaysIterableForeach; $this->callNodeCallback($nodeCallback, new InForeachNode($stmt), $scope, $storage); $originalScope = $scope; $bodyScope = $scope; @@ -1509,7 +1517,7 @@ public function processStmtNode( if ($context->isTopLevel()) { $storage = $originalStorage->duplicate(); - $originalScope = $this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope; + $originalScope = $narrowForeachBody ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope; $unrolledResult = $this->tryProcessUnrolledConstantArrayForeach($stmt, $originalScope, $originalStorage, $context); if ($unrolledResult !== null) { $bodyScope = $unrolledResult['bodyScope']; @@ -1520,7 +1528,7 @@ public function processStmtNode( $count = 0; do { $prevScope = $bodyScope; - $bodyScope = $bodyScope->mergeWith($this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope); + $bodyScope = $bodyScope->mergeWith($narrowForeachBody ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope); $storage = $originalStorage->duplicate(); $bodyScope = $this->enterForeach($bodyScope, $storage, $originalScope, $stmt, $nodeCallback); $bodyScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, new NoopNodeCallback(), $context->enterDeep())->filterOutLoopExitPoints(); @@ -1540,7 +1548,7 @@ public function processStmtNode( } } - $bodyScope = $bodyScope->mergeWith($this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope); + $bodyScope = $bodyScope->mergeWith($narrowForeachBody ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope); $storage = $originalStorage; $bodyScope = $this->enterForeach($bodyScope, $storage, $originalScope, $stmt, $nodeCallback); $finalPassContext = $unrolledTotalKeys !== null ? $context->enterUnrolledForeach($unrolledTotalKeys) : $context; @@ -1688,6 +1696,33 @@ public function processStmtNode( } $isIterableAtLeastOnce = $exprType->isIterableAtLeastOnce(); + + $iterateeCertainty = $finalScope->hasExpressionType($stmt->expr); + if ( + $this->narrowForeachBodyNonEmpty + && !$this->polluteScopeWithAlwaysIterableForeach + && !$iterateeCertainty->no() + && !$isIterableAtLeastOnce->yes() + ) { + // With the flag off the after-loop scope must not assume the loop ran, so + // undo the body narrowing: restore the iteratee's possibly-empty-ness + // (keeping element types the body refined). Only the non-emptiness the + // narrowing added is stripped; an iteratee already non-empty before the + // loop keeps it, and a literal like `foreach ([1, 2] as $v)` is skipped. + $finalIterateeType = $finalScope->getType($stmt->expr); + if ($finalIterateeType->isArray()->yes()) { + $finalIterateeNativeType = $finalScope->getNativeType($stmt->expr); + $finalScope = $finalScope->specifyExpressionType( + $stmt->expr, + TypeCombinator::union($finalIterateeType, new ConstantArrayType([], [])), + $finalIterateeNativeType->isArray()->yes() + ? TypeCombinator::union($finalIterateeNativeType, new ConstantArrayType([], [])) + : $finalIterateeNativeType, + $iterateeCertainty, + ); + } + } + if ($isIterableAtLeastOnce->maybe() || $exprType->isIterable()->no()) { $finalScope = $finalScope->mergeWith($scope->filterByTruthyValue(new BooleanOr( new BinaryOp\Identical( diff --git a/src/Testing/RuleTestCase.php b/src/Testing/RuleTestCase.php index 7f70e1f031..c7b5030b5c 100644 --- a/src/Testing/RuleTestCase.php +++ b/src/Testing/RuleTestCase.php @@ -124,6 +124,7 @@ protected function createNodeScopeResolver(): NodeScopeResolver self::getContainer()->getByType(DeepNodeCloner::class), $this->shouldPolluteScopeWithLoopInitialAssignments(), $this->shouldPolluteScopeWithAlwaysIterableForeach(), + self::getContainer()->getParameter('featureToggles')['narrowForeachBodyNonEmpty'], self::getContainer()->getParameter('polluteScopeWithBlock'), [], [], diff --git a/src/Testing/TypeInferenceTestCase.php b/src/Testing/TypeInferenceTestCase.php index 6dbcfd5f4c..666ce5f6ea 100644 --- a/src/Testing/TypeInferenceTestCase.php +++ b/src/Testing/TypeInferenceTestCase.php @@ -99,6 +99,7 @@ protected static function createNodeScopeResolver(): NodeScopeResolver self::getContainer()->getByType(DeepNodeCloner::class), $container->getParameter('polluteScopeWithLoopInitialAssignments'), $container->getParameter('polluteScopeWithAlwaysIterableForeach'), + $container->getParameter('featureToggles')['narrowForeachBodyNonEmpty'], $container->getParameter('polluteScopeWithBlock'), static::getEarlyTerminatingMethodCalls(), static::getEarlyTerminatingFunctionCalls(), diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index fbdcabe954..47d0f6360f 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -841,6 +841,7 @@ private function createAnalyser(): Analyser $container->getByType(DeepNodeCloner::class), false, true, + false, true, [], [], diff --git a/tests/PHPStan/Analyser/Bug13312NoPolluteTest.php b/tests/PHPStan/Analyser/Bug13312NoPolluteTest.php new file mode 100644 index 0000000000..43f89c0d88 --- /dev/null +++ b/tests/PHPStan/Analyser/Bug13312NoPolluteTest.php @@ -0,0 +1,36 @@ +assertFileAsserts($assertType, $file, ...$args); + } + + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/bug-13312-no-pollute.neon', + ]; + } + +} diff --git a/tests/PHPStan/Analyser/Bug13312StableTest.php b/tests/PHPStan/Analyser/Bug13312StableTest.php new file mode 100644 index 0000000000..ff63e16c50 --- /dev/null +++ b/tests/PHPStan/Analyser/Bug13312StableTest.php @@ -0,0 +1,36 @@ +assertFileAsserts($assertType, $file, ...$args); + } + + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/bug-13312-stable.neon', + ]; + } + +} diff --git a/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverRuleTest.php b/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverRuleTest.php index fa04faa0d0..f8cd77ae62 100644 --- a/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverRuleTest.php +++ b/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverRuleTest.php @@ -144,6 +144,7 @@ protected function createNodeScopeResolver(): NodeScopeResolver self::getContainer()->getByType(DeepNodeCloner::class), $this->shouldPolluteScopeWithLoopInitialAssignments(), $this->shouldPolluteScopeWithAlwaysIterableForeach(), + self::getContainer()->getParameter('featureToggles')['narrowForeachBodyNonEmpty'], self::getContainer()->getParameter('polluteScopeWithBlock'), [], [], diff --git a/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php b/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php index 5ede52160a..60cbd18fa1 100644 --- a/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php @@ -77,6 +77,7 @@ protected static function createNodeScopeResolver(): NodeScopeResolver $container->getByType(DeepNodeCloner::class), $container->getParameter('polluteScopeWithLoopInitialAssignments'), $container->getParameter('polluteScopeWithAlwaysIterableForeach'), + $container->getParameter('featureToggles')['narrowForeachBodyNonEmpty'], $container->getParameter('polluteScopeWithBlock'), static::getEarlyTerminatingMethodCalls(), static::getEarlyTerminatingFunctionCalls(), diff --git a/tests/PHPStan/Analyser/bug-13312-no-pollute.neon b/tests/PHPStan/Analyser/bug-13312-no-pollute.neon new file mode 100644 index 0000000000..2827569d0e --- /dev/null +++ b/tests/PHPStan/Analyser/bug-13312-no-pollute.neon @@ -0,0 +1,4 @@ +parameters: + polluteScopeWithAlwaysIterableForeach: false + featureToggles: + narrowForeachBodyNonEmpty: true diff --git a/tests/PHPStan/Analyser/bug-13312-stable.neon b/tests/PHPStan/Analyser/bug-13312-stable.neon new file mode 100644 index 0000000000..3ee516d3be --- /dev/null +++ b/tests/PHPStan/Analyser/bug-13312-stable.neon @@ -0,0 +1,2 @@ +parameters: + polluteScopeWithAlwaysIterableForeach: false diff --git a/tests/PHPStan/Analyser/data/bug-13312-no-pollute.php b/tests/PHPStan/Analyser/data/bug-13312-no-pollute.php new file mode 100644 index 0000000000..2926f97297 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-13312-no-pollute.php @@ -0,0 +1,37 @@ + $arr */ +function foo(array $arr): void { + assertType('list', $arr); + foreach ($arr as $v) { + assertType('non-empty-list', $arr); + } + assertType('list', $arr); + + for ($i = 0; $i < count($arr); ++$i) { + assertType('non-empty-list', $arr); + } + assertType('list', $arr); +} + +/** @param array $arr */ +function fooStringKeyed(array $arr): void { + assertType('array', $arr); + foreach ($arr as $v) { + assertType('non-empty-array', $arr); + } + assertType('array', $arr); +} + +/** @param list $arr */ +function fooReassign(array $arr): void { + foreach ($arr as $v) { + $arr = []; + assertType('array{}', $arr); + } + assertType('array{}', $arr); +} diff --git a/tests/PHPStan/Analyser/data/bug-13312-stable.php b/tests/PHPStan/Analyser/data/bug-13312-stable.php new file mode 100644 index 0000000000..3b952600ea --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-13312-stable.php @@ -0,0 +1,32 @@ + $arr */ +function foo(array $arr): void { + assertType('list', $arr); + foreach ($arr as $v) { + assertType('list', $arr); + } + assertType('list', $arr); + + for ($i = 0; $i < count($arr); ++$i) { + assertType('non-empty-list', $arr); + } + assertType('list', $arr); +} + +/** @param array $arr */ +function fooStringKeyed(array $arr): void { + assertType('array', $arr); + foreach ($arr as $v) { + assertType('array', $arr); + } + assertType('array', $arr); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-13312.php b/tests/PHPStan/Analyser/nsrt/bug-13312.php index 3a7ab22873..9a47594a06 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-13312.php +++ b/tests/PHPStan/Analyser/nsrt/bug-13312.php @@ -32,6 +32,24 @@ function foo(array $arr): void { } +/** @param array $arr */ +function fooStringKeyed(array $arr): void { + assertType('array', $arr); + foreach ($arr as $v) { + assertType('non-empty-array', $arr); + } + assertType('array', $arr); +} + +/** @param list $arr */ +function fooReassign(array $arr): void { + foreach ($arr as $v) { + $arr = []; + assertType('array{}', $arr); + } + assertType('array{}', $arr); +} + function fooBar(mixed $mixed): void { assertType('mixed', $mixed); foreach ($mixed as $v) {