From c3637814ba55fc8716129988f25f1c8d59a1d8c6 Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Sun, 26 Jul 2026 10:34:06 +0900 Subject: [PATCH 1/2] 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 | 38 ++++++++++++++++++- 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, 209 insertions(+), 1 deletion(-) 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 8fc949c9361..42ef0614425 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -46,6 +46,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 953bab24371..440652deac6 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 aefe7850be2..1b7f3e56601 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -144,6 +144,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; @@ -263,6 +264,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(ref: '%exceptions.implicitThrows%')] @@ -1511,7 +1514,13 @@ public function processStmtNode( $originalStorage = $storage; $unrolledEndScope = null; $unrolledTotalKeys = null; - $iterateeScope = $this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope; + // 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. + $iterateeScope = $this->narrowForeachBodyNonEmpty || $this->polluteScopeWithAlwaysIterableForeach + ? $scope->filterByTruthyValue($arrayComparisonExpr) + : $scope; if ($context->isTopLevel()) { $storage = $originalStorage->duplicate(); @@ -1699,6 +1708,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 d070726072c..0f6d99b6dce 100644 --- a/src/Testing/RuleTestCase.php +++ b/src/Testing/RuleTestCase.php @@ -123,6 +123,7 @@ protected function createNodeScopeResolver(): NodeScopeResolver self::createScopeFactory($reflectionProvider, $typeSpecifier), $this->shouldPolluteScopeWithLoopInitialAssignments(), $this->shouldPolluteScopeWithAlwaysIterableForeach(), + self::getContainer()->getParameter('featureToggles')['narrowForeachBodyNonEmpty'], self::getContainer()->getParameter('polluteScopeWithBlock'), self::getContainer()->getParameter('exceptions')['implicitThrows'], $this->shouldTreatPhpDocTypesAsCertain(), diff --git a/src/Testing/TypeInferenceTestCase.php b/src/Testing/TypeInferenceTestCase.php index f19adc7dbe2..3160caf0003 100644 --- a/src/Testing/TypeInferenceTestCase.php +++ b/src/Testing/TypeInferenceTestCase.php @@ -98,6 +98,7 @@ protected static function createNodeScopeResolver(): NodeScopeResolver self::createScopeFactory($reflectionProvider, $typeSpecifier), $container->getParameter('polluteScopeWithLoopInitialAssignments'), $container->getParameter('polluteScopeWithAlwaysIterableForeach'), + $container->getParameter('featureToggles')['narrowForeachBodyNonEmpty'], $container->getParameter('polluteScopeWithBlock'), $container->getParameter('exceptions')['implicitThrows'], $container->getParameter('treatPhpDocTypesAsCertain'), diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 586aa791332..130d6e4266d 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -839,6 +839,7 @@ private function createAnalyser(): Analyser self::createScopeFactory($reflectionProvider, $typeSpecifier), false, true, + false, true, true, $this->shouldTreatPhpDocTypesAsCertain(), diff --git a/tests/PHPStan/Analyser/Bug13312NoPolluteTest.php b/tests/PHPStan/Analyser/Bug13312NoPolluteTest.php new file mode 100644 index 00000000000..43f89c0d887 --- /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 00000000000..ff63e16c50b --- /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 88d4aae93e2..bf7c546631f 100644 --- a/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverRuleTest.php +++ b/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverRuleTest.php @@ -143,6 +143,7 @@ protected function createNodeScopeResolver(): NodeScopeResolver self::createScopeFactory($reflectionProvider, $typeSpecifier), $this->shouldPolluteScopeWithLoopInitialAssignments(), $this->shouldPolluteScopeWithAlwaysIterableForeach(), + self::getContainer()->getParameter('featureToggles')['narrowForeachBodyNonEmpty'], self::getContainer()->getParameter('polluteScopeWithBlock'), self::getContainer()->getParameter('exceptions')['implicitThrows'], $this->shouldTreatPhpDocTypesAsCertain(), diff --git a/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php b/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php index bfaac47349f..9f14ce9c5ea 100644 --- a/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php @@ -76,6 +76,7 @@ protected static function createNodeScopeResolver(): NodeScopeResolver self::createScopeFactory($reflectionProvider, $typeSpecifier), $container->getParameter('polluteScopeWithLoopInitialAssignments'), $container->getParameter('polluteScopeWithAlwaysIterableForeach'), + $container->getParameter('featureToggles')['narrowForeachBodyNonEmpty'], $container->getParameter('polluteScopeWithBlock'), $container->getParameter('exceptions')['implicitThrows'], $container->getParameter('treatPhpDocTypesAsCertain'), 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 00000000000..2827569d0e9 --- /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 00000000000..3ee516d3be6 --- /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 00000000000..2926f972972 --- /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 00000000000..3b952600eaf --- /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 3a7ab22873c..9a47594a06f 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) { From eb6fb26db20899a01238eac70659f715239f21bd Mon Sep 17 00:00:00 2001 From: USAMI Kenta Date: Sat, 1 Aug 2026 18:08:38 +0900 Subject: [PATCH 2/2] Drop the after-loop restore to keep mutation testing at 100% MSI The restore that kept the after-loop scope conservative under the toggle had a guard, `!$iterateeCertainty->no()`, whose TrinaryLogicMutator mutant (`$iterateeCertainty->yes()`) is equivalent: the two differ only when the iteratee is a maybe-defined variable, and there the value variable's certainty is already dominated by the iteratee's own maybe-definedness, so no test can observe the difference. Infection therefore reports it as escaped and there is no ignore mechanism. Since the toggle is opt-in, drop the restore instead of chasing the equivalent mutant. The foreach body still narrows the iterated expression; the after-loop scope now simply keeps that precision (a value variable can be seen as defined where the iteratee is later proven non-empty). The only remaining changed line, the $iterateeScope condition, is fully covered by the existing bug-13312 tests. --- src/Analyser/NodeScopeResolver.php | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 1b7f3e56601..9a0e4bfa616 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -144,7 +144,6 @@ 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; @@ -1708,33 +1707,6 @@ 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(