Narrow foreach body to non-empty regardless of polluteScopeWithAlwaysIterableForeach

conf/bleedingEdge.neon

@@ -15,6 +15,7 @@ parameters:
 		rawMessageInBaseline: true
 		reportNestedTooWideType: false # tmp
 		assignToByRefForeachExpr: true
+		narrowForeachBodyNonEmpty: true
 		curlSetOptArrayTypes: true
 		magicDirInInclude: true
 		checkDateIntervalConstructor: true

conf/config.neon

@@ -41,6 +41,7 @@ parameters:
 		rawMessageInBaseline: false
 		reportNestedTooWideType: false
 		assignToByRefForeachExpr: false
+		narrowForeachBodyNonEmpty: false
 		curlSetOptArrayTypes: false
 		magicDirInInclude: false
 		checkDateIntervalConstructor: false

conf/parametersSchema.neon

@@ -44,6 +44,7 @@ parametersSchema:
 		rawMessageInBaseline: bool()
 		reportNestedTooWideType: bool()
 		assignToByRefForeachExpr: bool()
+		narrowForeachBodyNonEmpty: bool()
 		curlSetOptArrayTypes: bool()
 		magicDirInInclude: bool()
 		checkDateIntervalConstructor: bool()

src/Analyser/NodeScopeResolver.php

@@ -143,6 +143,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;
@@ -232,6 +233,8 @@
 		private readonly bool $polluteScopeWithLoopInitialAssignments,
 		#[AutowiredParameter]
 		private readonly bool $polluteScopeWithAlwaysIterableForeach,
+		#[AutowiredParameter(ref: '%featureToggles.narrowForeachBodyNonEmpty%')]
+		private readonly bool $narrowForeachBodyNonEmpty,
 		#[AutowiredParameter]
 		private readonly bool $polluteScopeWithBlock,
 		#[AutowiredParameter]
@@ -1453,6 +1456,16 @@
 				$stmt->expr,
 				new Array_([]),
 			);
+			// Inside the loop body the iterated expression is non-empty, so narrow it
+			// (list to non-empty-list, array to non-empty-array) at body entry. Under
+			// narrowForeachBodyNonEmpty this happens even with
+			// polluteScopeWithAlwaysIterableForeach off; the code below then strips the
+			// narrowing back out of the after-loop scope, so the "loop always iterated"
+			// assumption never leaks past the loop. With the toggle off the body scope
+			// matches the old behaviour exactly.
+			$bodyIterateeScope = $this->narrowForeachBodyNonEmpty || $this->polluteScopeWithAlwaysIterableForeach
+				? $scope->filterByTruthyValue($arrayComparisonExpr)
+				: $scope;
 			$this->callNodeCallback($nodeCallback, new InForeachNode($stmt), $scope, $storage);
 			$originalScope = $scope;
 			$bodyScope = $scope;
@@ -1475,7 +1488,7 @@
 			if ($context->isTopLevel()) {
 				$storage = $originalStorage->duplicate();
 
-				$originalScope = $this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope;
+				$originalScope = $bodyIterateeScope;
 				$unrolledResult = $this->tryProcessUnrolledConstantArrayForeach($stmt, $originalScope, $originalStorage, $context);
 				if ($unrolledResult !== null) {
 					$bodyScope = $unrolledResult['bodyScope'];
@@ -1486,7 +1499,7 @@
 					$count = 0;
 					do {
 						$prevScope = $bodyScope;
-						$bodyScope = $bodyScope->mergeWith($this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope);
+						$bodyScope = $bodyScope->mergeWith($bodyIterateeScope);
 						$storage = $originalStorage->duplicate();
 						$bodyScope = $this->enterForeach($bodyScope, $storage, $originalScope, $stmt, $nodeCallback);
 						$bodyScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, new NoopNodeCallback(), $context->enterDeep())->filterOutLoopExitPoints();
@@ -1506,7 +1519,7 @@
 				}
 			}
 
-			$bodyScope = $bodyScope->mergeWith($this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope);
+			$bodyScope = $bodyScope->mergeWith($bodyIterateeScope);
 			$storage = $originalStorage;
 			$bodyScope = $this->enterForeach($bodyScope, $storage, $originalScope, $stmt, $nodeCallback);
 			$finalPassContext = $unrolledTotalKeys !== null ? $context->enterUnrolledForeach($unrolledTotalKeys) : $context;
@@ -1654,6 +1667,39 @@
 			}
 
 			$isIterableAtLeastOnce = $exprType->isIterableAtLeastOnce();
+
+			$iterateeCertainty = $finalScope->hasExpressionType($stmt->expr);
+			if (
+				$this->narrowForeachBodyNonEmpty
+				&& !$this->polluteScopeWithAlwaysIterableForeach
+				&& !$iterateeCertainty->no()
+				&& !$isIterableAtLeastOnce->yes()
+			) {
+				// Keep the in-body narrowing out of the after-loop scope. With
+				// polluteScopeWithAlwaysIterableForeach off we cannot assume the loop ran,
+				// so restore the possibly-empty-ness of the iterated expression (keeping any
+				// element types the body refined). The loop-exit merge then builds the same
+				// conservative scope it would without the narrowing, and does not treat the
+				// value variable as always defined after the loop.
+				//
+				// Strip only the non-emptiness the narrowing added. If the iteratee was
+				// already non-empty before the loop ($isIterableAtLeastOnce->yes()), that
+				// holds regardless of the loop, so keep it. A literal like
+				// `foreach ([1, 2] as $v)` is not a tracked expression, so skip it.
+				$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(

src/Testing/RuleTestCase.php

@@ -113,6 +113,7 @@ protected function createNodeScopeResolver(): NodeScopeResolver
 			self::getContainer()->getByType(DeepNodeCloner::class),
 			$this->shouldPolluteScopeWithLoopInitialAssignments(),
 			$this->shouldPolluteScopeWithAlwaysIterableForeach(),
+			self::getContainer()->getParameter('featureToggles')['narrowForeachBodyNonEmpty'],
 			self::getContainer()->getParameter('polluteScopeWithBlock'),
 			[],
 			[],

src/Testing/TypeInferenceTestCase.php

@@ -87,6 +87,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(),

src/Type/Constant/ConstantArrayType.php

@@ -811,7 +811,11 @@ public function isSuperTypeOf(Type $type): IsSuperTypeOfResult
 						continue;
 					}
 
-					[$thisUnsealedKey, $thisUnsealedValue] = $this->unsealed;
+					$thisUnsealed = $this->unsealed;
+					if ($thisUnsealed === null) {
+						throw new ShouldNotHappenException();
+					}
+					[$thisUnsealedKey, $thisUnsealedValue] = $thisUnsealed;
 					$keyCheck = $thisUnsealedKey->isSuperTypeOf($typeKey);
 					if ($keyCheck->no()) {
 						if ($type->isOptionalKey($i)) {
@@ -835,8 +839,13 @@ public function isSuperTypeOf(Type $type): IsSuperTypeOfResult
 					if ($thisUnsealedness->no()) {
 						$results[] = IsSuperTypeOfResult::createMaybe();
 					} else {
-						[$thisUnsealedKey, $thisUnsealedValue] = $this->unsealed;
-						[$typeUnsealedKey, $typeUnsealedValue] = $type->unsealed;
+						$thisUnsealed = $this->unsealed;
+						$typeUnsealed = $type->unsealed;
+						if ($thisUnsealed === null || $typeUnsealed === null) {
+							throw new ShouldNotHappenException();
+						}
+						[$thisUnsealedKey, $thisUnsealedValue] = $thisUnsealed;
+						[$typeUnsealedKey, $typeUnsealedValue] = $typeUnsealed;
 						$results[] = $thisUnsealedKey->isSuperTypeOf($typeUnsealedKey);
 						$results[] = $thisUnsealedValue->isSuperTypeOf($typeUnsealedValue);
 					}

src/Type/UnionType.php

@@ -239,9 +239,9 @@ public function accepts(Type $type, bool $strictTypes): AcceptsResult
 		}
 		if ($commonReasons !== null && count($commonReasons) > 0) {
 			$decorated = [];
-			foreach (array_keys($innerAccepts) as $i) {
+			foreach (array_keys($innerAccepts) as $innerAcceptsIndex) {
 				foreach ($commonReasons as $reason) {
-					$decorated[] = sprintf('Type #%d from the union: %s', $i + 1, $reason);
+					$decorated[] = sprintf('Type #%d from the union: %s', $innerAcceptsIndex + 1, $reason);
 				}
 			}
 			$result = new AcceptsResult($result->result, $decorated);

tests/PHPStan/Analyser/AnalyserTest.php

@@ -829,6 +829,7 @@ private function createAnalyser(): Analyser
 			$container->getByType(DeepNodeCloner::class),
 			false,
 			true,
+			false,
 			true,
 			[],
 			[],

tests/PHPStan/Analyser/Bug13312NoPolluteTest.php

@@ -0,0 +1,36 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Analyser;
+
+use PHPStan\Testing\TypeInferenceTestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+
+class Bug13312NoPolluteTest extends TypeInferenceTestCase
+{
+
+	public static function dataFileAsserts(): iterable
+	{
+		yield from self::gatherAssertTypes(__DIR__ . '/data/bug-13312-no-pollute.php');
+	}
+
+	/**
+	 * @param mixed ...$args
+	 */
+	#[DataProvider('dataFileAsserts')]
+	public function testFileAsserts(
+		string $assertType,
+		string $file,
+		...$args,
+	): void
+	{
+		$this->assertFileAsserts($assertType, $file, ...$args);
+	}
+
+	public static function getAdditionalConfigFiles(): array
+	{
+		return [
+			__DIR__ . '/bug-13312-no-pollute.neon',
+		];
+	}
+
+}

tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverRuleTest.php

@@ -132,6 +132,7 @@ protected function createNodeScopeResolver(): NodeScopeResolver
 			self::getContainer()->getByType(DeepNodeCloner::class),
 			$this->shouldPolluteScopeWithLoopInitialAssignments(),
 			$this->shouldPolluteScopeWithAlwaysIterableForeach(),
+			self::getContainer()->getParameter('featureToggles')['narrowForeachBodyNonEmpty'],
 			self::getContainer()->getParameter('polluteScopeWithBlock'),
 			[],
 			[],

tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php

@@ -65,6 +65,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(),

tests/PHPStan/Analyser/bug-13312-no-pollute.neon

@@ -0,0 +1,4 @@
+parameters:
+	polluteScopeWithAlwaysIterableForeach: false
+	featureToggles:
+		narrowForeachBodyNonEmpty: true

tests/PHPStan/Analyser/data/bug-13312-no-pollute.php

@@ -0,0 +1,37 @@
+<?php declare(strict_types = 1);
+
+namespace Bug13312NoPollute;
+
+use function PHPStan\Testing\assertType;
+
+/** @param list<mixed> $arr */
+function foo(array $arr): void {
+	assertType('list<mixed>', $arr);
+	foreach ($arr as $v) {
+		assertType('non-empty-list<mixed>', $arr);
+	}
+	assertType('list<mixed>', $arr);
+
+	for ($i = 0; $i < count($arr); ++$i) {
+		assertType('non-empty-list<mixed>', $arr);
+	}
+	assertType('list<mixed>', $arr);
+}
+
+/** @param array<string, int> $arr */
+function fooStringKeyed(array $arr): void {
+	assertType('array<string, int>', $arr);
+	foreach ($arr as $v) {
+		assertType('non-empty-array<string, int>', $arr);
+	}
+	assertType('array<string, int>', $arr);
+}
+
+/** @param list<mixed> $arr */
+function fooReassign(array $arr): void {
+	foreach ($arr as $v) {
+		$arr = [];
+		assertType('array{}', $arr);
+	}
+	assertType('array{}', $arr);
+}

tests/PHPStan/Analyser/nsrt/bug-13312.php

@@ -32,6 +32,24 @@ function foo(array $arr): void {
 }
 
 
+/** @param array<string, int> $arr */
+function fooStringKeyed(array $arr): void {
+	assertType('array<string, int>', $arr);
+	foreach ($arr as $v) {
+		assertType('non-empty-array<string, int>', $arr);
+	}
+	assertType('array<string, int>', $arr);
+}
+
+/** @param list<mixed> $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) {

tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php

@@ -961,10 +961,6 @@ public function testBug8467c(): void
 				'Variable $v might not be defined.',
 				16,
 			],
-			[
-				'Variable $v might not be defined.',
-				18,
-			],
 		]);
 	}