Skip to content

Commit 568511a

Browse files
zonuexezonuexe
committed
Narrow iterated expression to non-empty inside foreach body regardless of polluteScopeWithAlwaysIterableForeach
The in-body narrowing of the iterated expression (list -> non-empty-list, array -> non-empty-array) was an accidental side effect of the polluteScopeWithAlwaysIterableForeach scope-pollution mechanism: it only happened when the flag was true. With the flag set to false (as phpstan-strict-rules does), the narrowing silently disappeared even though the loop body is provably entered only when the iteratee is non-empty. Apply the non-empty narrowing at foreach-body entry from the pre-loop scope unconditionally, mirroring the previous flag-true behaviour exactly. The flag now only controls what leaks into the after-loop scope, which is its documented purpose. Closes phpstan/phpstan#13312
1 parent 7e97b9e commit 568511a

10 files changed

Lines changed: 120 additions & 17 deletions

File tree

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1453,6 +1453,11 @@ public function processStmtNode(
14531453
$stmt->expr,
14541454
new Array_([]),
14551455
);
1456+
// Inside the loop body the iterated expression is provably non-empty, so
1457+
// narrow it (list -> non-empty-list, array -> non-empty-array) at body entry
1458+
// from the pre-loop scope. This is independent of polluteScopeWithAlwaysIterableForeach,
1459+
// which only controls what leaks into the after-loop scope.
1460+
$nonEmptyIterateeScope = $scope->filterByTruthyValue($arrayComparisonExpr);
14561461
$this->callNodeCallback($nodeCallback, new InForeachNode($stmt), $scope, $storage);
14571462
$originalScope = $scope;
14581463
$bodyScope = $scope;
@@ -1475,7 +1480,7 @@ public function processStmtNode(
14751480
if ($context->isTopLevel()) {
14761481
$storage = $originalStorage->duplicate();
14771482

1478-
$originalScope = $this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope;
1483+
$originalScope = $nonEmptyIterateeScope;
14791484
$unrolledResult = $this->tryProcessUnrolledConstantArrayForeach($stmt, $originalScope, $originalStorage, $context);
14801485
if ($unrolledResult !== null) {
14811486
$bodyScope = $unrolledResult['bodyScope'];
@@ -1486,7 +1491,7 @@ public function processStmtNode(
14861491
$count = 0;
14871492
do {
14881493
$prevScope = $bodyScope;
1489-
$bodyScope = $bodyScope->mergeWith($this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope);
1494+
$bodyScope = $bodyScope->mergeWith($nonEmptyIterateeScope);
14901495
$storage = $originalStorage->duplicate();
14911496
$bodyScope = $this->enterForeach($bodyScope, $storage, $originalScope, $stmt, $nodeCallback);
14921497
$bodyScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, new NoopNodeCallback(), $context->enterDeep())->filterOutLoopExitPoints();
@@ -1506,7 +1511,7 @@ public function processStmtNode(
15061511
}
15071512
}
15081513

1509-
$bodyScope = $bodyScope->mergeWith($this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope);
1514+
$bodyScope = $bodyScope->mergeWith($nonEmptyIterateeScope);
15101515
$storage = $originalStorage;
15111516
$bodyScope = $this->enterForeach($bodyScope, $storage, $originalScope, $stmt, $nodeCallback);
15121517
$finalPassContext = $unrolledTotalKeys !== null ? $context->enterUnrolledForeach($unrolledTotalKeys) : $context;

‎src/Command/AnalyseCommand.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -477,8 +477,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
477477
if ($generateBaselineFile !== null) {
478478
$this->runDiagnoseExtensions($container, $inceptionResult->getErrorOutput(), $analysisResult->getProcessedFiles());
479479
if (count($internalErrorsTuples) > 0) {
480-
foreach ($internalErrorsTuples as [$internalError]) {
481-
$inceptionResult->getStdOutput()->writeLineFormatted($internalError->getMessage());
480+
foreach ($internalErrorsTuples as [$error]) {
481+
$inceptionResult->getStdOutput()->writeLineFormatted($error->getMessage());
482482
$inceptionResult->getStdOutput()->writeLineFormatted('');
483483
}
484484

‎src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,16 +134,16 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
134134
if ($identifier->isClass()) {
135135
$fetchedClassNode = null;
136136
$fetchedFile = null;
137-
foreach ($files as $file) {
138-
$fetchedClassNodes = $this->fileNodesFetcher->fetchNodes($file)->getClassNodes();
137+
foreach ($files as $sourceFile) {
138+
$fetchedClassNodes = $this->fileNodesFetcher->fetchNodes($sourceFile)->getClassNodes();
139139

140140
if (!array_key_exists($identifierName, $fetchedClassNodes)) {
141141
return null;
142142
}
143143

144144
/** @var FetchedNode<Node\Stmt\ClassLike> $fetchedClassNode */
145145
$fetchedClassNode = current($fetchedClassNodes[$identifierName]);
146-
$fetchedFile = $file;
146+
$fetchedFile = $sourceFile;
147147
}
148148

149149
[$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($fetchedFile, $identifier);

‎src/Type/Constant/ConstantArrayType.php‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -811,7 +811,11 @@ public function isSuperTypeOf(Type $type): IsSuperTypeOfResult
811811
continue;
812812
}
813813

814-
[$thisUnsealedKey, $thisUnsealedValue] = $this->unsealed;
814+
$thisUnsealed = $this->unsealed;
815+
if ($thisUnsealed === null) {
816+
throw new ShouldNotHappenException();
817+
}
818+
[$thisUnsealedKey, $thisUnsealedValue] = $thisUnsealed;
815819
$keyCheck = $thisUnsealedKey->isSuperTypeOf($typeKey);
816820
if ($keyCheck->no()) {
817821
if ($type->isOptionalKey($i)) {
@@ -835,8 +839,13 @@ public function isSuperTypeOf(Type $type): IsSuperTypeOfResult
835839
if ($thisUnsealedness->no()) {
836840
$results[] = IsSuperTypeOfResult::createMaybe();
837841
} else {
838-
[$thisUnsealedKey, $thisUnsealedValue] = $this->unsealed;
839-
[$typeUnsealedKey, $typeUnsealedValue] = $type->unsealed;
842+
$thisUnsealed = $this->unsealed;
843+
$typeUnsealed = $type->unsealed;
844+
if ($thisUnsealed === null || $typeUnsealed === null) {
845+
throw new ShouldNotHappenException();
846+
}
847+
[$thisUnsealedKey, $thisUnsealedValue] = $thisUnsealed;
848+
[$typeUnsealedKey, $typeUnsealedValue] = $typeUnsealed;
840849
$results[] = $thisUnsealedKey->isSuperTypeOf($typeUnsealedKey);
841850
$results[] = $thisUnsealedValue->isSuperTypeOf($typeUnsealedValue);
842851
}

‎src/Type/UnionType.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,9 +239,9 @@ public function accepts(Type $type, bool $strictTypes): AcceptsResult
239239
}
240240
if ($commonReasons !== null && count($commonReasons) > 0) {
241241
$decorated = [];
242-
foreach (array_keys($innerAccepts) as $i) {
242+
foreach (array_keys($innerAccepts) as $j) {
243243
foreach ($commonReasons as $reason) {
244-
$decorated[] = sprintf('Type #%d from the union: %s', $i + 1, $reason);
244+
$decorated[] = sprintf('Type #%d from the union: %s', $j + 1, $reason);
245245
}
246246
}
247247
$result = new AcceptsResult($result->result, $decorated);

‎tests/PHPStan/Analyser/Bug13312NoPolluteTest.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Analyser;
4+
5+
use PHPStan\Testing\TypeInferenceTestCase;
6+
use PHPUnit\Framework\Attributes\DataProvider;
7+
8+
class Bug13312NoPolluteTest extends TypeInferenceTestCase
9+
{
10+
11+
public static function dataFileAsserts(): iterable
12+
{
13+
yield from self::gatherAssertTypes(__DIR__ . '/data/bug-13312-no-pollute.php');
14+
}
15+
16+
/**
17+
* @param mixed ...$args
18+
*/
19+
#[DataProvider('dataFileAsserts')]
20+
public function testFileAsserts(
21+
string $assertType,
22+
string $file,
23+
...$args,
24+
): void
25+
{
26+
$this->assertFileAsserts($assertType, $file, ...$args);
27+
}
28+
29+
public static function getAdditionalConfigFiles(): array
30+
{
31+
return [
32+
__DIR__ . '/bug-13312-no-pollute.neon',
33+
];
34+
}
35+
36+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
parameters:
2+
polluteScopeWithAlwaysIterableForeach: false

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug13312NoPollute;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
/** @param list<mixed> $arr */
8+
function foo(array $arr): void {
9+
assertType('list<mixed>', $arr);
10+
foreach ($arr as $v) {
11+
assertType('non-empty-list<mixed>', $arr);
12+
}
13+
assertType('list<mixed>', $arr);
14+
15+
for ($i = 0; $i < count($arr); ++$i) {
16+
assertType('non-empty-list<mixed>', $arr);
17+
}
18+
assertType('list<mixed>', $arr);
19+
}
20+
21+
/** @param array<string, int> $arr */
22+
function fooStringKeyed(array $arr): void {
23+
assertType('array<string, int>', $arr);
24+
foreach ($arr as $v) {
25+
assertType('non-empty-array<string, int>', $arr);
26+
}
27+
assertType('array<string, int>', $arr);
28+
}
29+
30+
/** @param list<mixed> $arr */
31+
function fooReassign(array $arr): void {
32+
foreach ($arr as $v) {
33+
$arr = [];
34+
assertType('array{}', $arr);
35+
}
36+
assertType('array{}', $arr);
37+
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,24 @@ function foo(array $arr): void {
3232
}
3333

3434

35+
/** @param array<string, int> $arr */
36+
function fooStringKeyed(array $arr): void {
37+
assertType('array<string, int>', $arr);
38+
foreach ($arr as $v) {
39+
assertType('non-empty-array<string, int>', $arr);
40+
}
41+
assertType('array<string, int>', $arr);
42+
}
43+
44+
/** @param list<mixed> $arr */
45+
function fooReassign(array $arr): void {
46+
foreach ($arr as $v) {
47+
$arr = [];
48+
assertType('array{}', $arr);
49+
}
50+
assertType('array{}', $arr);
51+
}
52+
3553
function fooBar(mixed $mixed): void {
3654
assertType('mixed', $mixed);
3755
foreach ($mixed as $v) {

‎tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -961,10 +961,6 @@ public function testBug8467c(): void
961961
'Variable $v might not be defined.',
962962
16,
963963
],
964-
[
965-
'Variable $v might not be defined.',
966-
18,
967-
],
968964
]);
969965
}
970966

0 commit comments

Comments
 (0)