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) {