Skip to content

Commit c3389dc

Browse files
zonuexestaabm
zonuexe
authored and
staabm
committed
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
1 parent e68b401 commit c3389dc

15 files changed

Lines changed: 210 additions & 3 deletions

‎conf/config.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ parameters:
4141
rawMessageInBaseline: false
4242
reportNestedTooWideType: false
4343
assignToByRefForeachExpr: false
44+
narrowForeachBodyNonEmpty: false
4445
curlSetOptArrayTypes: false
4546
magicDirInInclude: false
4647
checkDateIntervalConstructor: false

‎conf/parametersSchema.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ parametersSchema:
4444
rawMessageInBaseline: bool()
4545
reportNestedTooWideType: bool()
4646
assignToByRefForeachExpr: bool()
47+
narrowForeachBodyNonEmpty: bool()
4748
curlSetOptArrayTypes: bool()
4849
magicDirInInclude: bool()
4950
checkDateIntervalConstructor: bool()

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@
142142
use PHPStan\ShouldNotHappenException;
143143
use PHPStan\TrinaryLogic;
144144
use PHPStan\Type\ClosureType;
145+
use PHPStan\Type\Constant\ConstantArrayType;
145146
use PHPStan\Type\Constant\ConstantIntegerType;
146147
use PHPStan\Type\Constant\ConstantStringType;
147148
use PHPStan\Type\FileTypeMapper;
@@ -266,6 +267,8 @@ public function __construct(
266267
private readonly bool $polluteScopeWithLoopInitialAssignments,
267268
#[AutowiredParameter]
268269
private readonly bool $polluteScopeWithAlwaysIterableForeach,
270+
#[AutowiredParameter(ref: '%featureToggles.narrowForeachBodyNonEmpty%')]
271+
private readonly bool $narrowForeachBodyNonEmpty,
269272
#[AutowiredParameter]
270273
private readonly bool $polluteScopeWithBlock,
271274
#[AutowiredParameter]
@@ -1487,6 +1490,11 @@ public function processStmtNode(
14871490
$stmt->expr,
14881491
new Array_([]),
14891492
);
1493+
// The loop body is only entered when the iteratee is non-empty. Under
1494+
// narrowForeachBodyNonEmpty we narrow it there (list to non-empty-list,
1495+
// array to non-empty-array) even with polluteScopeWithAlwaysIterableForeach
1496+
// off; with the toggle off the body scope is unchanged.
1497+
$narrowForeachBody = $this->narrowForeachBodyNonEmpty || $this->polluteScopeWithAlwaysIterableForeach;
14901498
$this->callNodeCallback($nodeCallback, new InForeachNode($stmt), $scope, $storage);
14911499
$originalScope = $scope;
14921500
$bodyScope = $scope;
@@ -1509,7 +1517,7 @@ public function processStmtNode(
15091517
if ($context->isTopLevel()) {
15101518
$storage = $originalStorage->duplicate();
15111519

1512-
$originalScope = $this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope;
1520+
$originalScope = $narrowForeachBody ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope;
15131521
$unrolledResult = $this->tryProcessUnrolledConstantArrayForeach($stmt, $originalScope, $originalStorage, $context);
15141522
if ($unrolledResult !== null) {
15151523
$bodyScope = $unrolledResult['bodyScope'];
@@ -1520,7 +1528,7 @@ public function processStmtNode(
15201528
$count = 0;
15211529
do {
15221530
$prevScope = $bodyScope;
1523-
$bodyScope = $bodyScope->mergeWith($this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope);
1531+
$bodyScope = $bodyScope->mergeWith($narrowForeachBody ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope);
15241532
$storage = $originalStorage->duplicate();
15251533
$bodyScope = $this->enterForeach($bodyScope, $storage, $originalScope, $stmt, $nodeCallback);
15261534
$bodyScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, new NoopNodeCallback(), $context->enterDeep())->filterOutLoopExitPoints();
@@ -1540,7 +1548,7 @@ public function processStmtNode(
15401548
}
15411549
}
15421550

1543-
$bodyScope = $bodyScope->mergeWith($this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope);
1551+
$bodyScope = $bodyScope->mergeWith($narrowForeachBody ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope);
15441552
$storage = $originalStorage;
15451553
$bodyScope = $this->enterForeach($bodyScope, $storage, $originalScope, $stmt, $nodeCallback);
15461554
$finalPassContext = $unrolledTotalKeys !== null ? $context->enterUnrolledForeach($unrolledTotalKeys) : $context;
@@ -1688,6 +1696,33 @@ public function processStmtNode(
16881696
}
16891697

16901698
$isIterableAtLeastOnce = $exprType->isIterableAtLeastOnce();
1699+
1700+
$iterateeCertainty = $finalScope->hasExpressionType($stmt->expr);
1701+
if (
1702+
$this->narrowForeachBodyNonEmpty
1703+
&& !$this->polluteScopeWithAlwaysIterableForeach
1704+
&& !$iterateeCertainty->no()
1705+
&& !$isIterableAtLeastOnce->yes()
1706+
) {
1707+
// With the flag off the after-loop scope must not assume the loop ran, so
1708+
// undo the body narrowing: restore the iteratee's possibly-empty-ness
1709+
// (keeping element types the body refined). Only the non-emptiness the
1710+
// narrowing added is stripped; an iteratee already non-empty before the
1711+
// loop keeps it, and a literal like `foreach ([1, 2] as $v)` is skipped.
1712+
$finalIterateeType = $finalScope->getType($stmt->expr);
1713+
if ($finalIterateeType->isArray()->yes()) {
1714+
$finalIterateeNativeType = $finalScope->getNativeType($stmt->expr);
1715+
$finalScope = $finalScope->specifyExpressionType(
1716+
$stmt->expr,
1717+
TypeCombinator::union($finalIterateeType, new ConstantArrayType([], [])),
1718+
$finalIterateeNativeType->isArray()->yes()
1719+
? TypeCombinator::union($finalIterateeNativeType, new ConstantArrayType([], []))
1720+
: $finalIterateeNativeType,
1721+
$iterateeCertainty,
1722+
);
1723+
}
1724+
}
1725+
16911726
if ($isIterableAtLeastOnce->maybe() || $exprType->isIterable()->no()) {
16921727
$finalScope = $finalScope->mergeWith($scope->filterByTruthyValue(new BooleanOr(
16931728
new BinaryOp\Identical(

‎src/Testing/RuleTestCase.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ protected function createNodeScopeResolver(): NodeScopeResolver
124124
self::getContainer()->getByType(DeepNodeCloner::class),
125125
$this->shouldPolluteScopeWithLoopInitialAssignments(),
126126
$this->shouldPolluteScopeWithAlwaysIterableForeach(),
127+
self::getContainer()->getParameter('featureToggles')['narrowForeachBodyNonEmpty'],
127128
self::getContainer()->getParameter('polluteScopeWithBlock'),
128129
[],
129130
[],

‎src/Testing/TypeInferenceTestCase.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ protected static function createNodeScopeResolver(): NodeScopeResolver
9999
self::getContainer()->getByType(DeepNodeCloner::class),
100100
$container->getParameter('polluteScopeWithLoopInitialAssignments'),
101101
$container->getParameter('polluteScopeWithAlwaysIterableForeach'),
102+
$container->getParameter('featureToggles')['narrowForeachBodyNonEmpty'],
102103
$container->getParameter('polluteScopeWithBlock'),
103104
static::getEarlyTerminatingMethodCalls(),
104105
static::getEarlyTerminatingFunctionCalls(),

‎tests/PHPStan/Analyser/AnalyserTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,7 @@ private function createAnalyser(): Analyser
841841
$container->getByType(DeepNodeCloner::class),
842842
false,
843843
true,
844+
false,
844845
true,
845846
[],
846847
[],

‎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/Bug13312StableTest.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 Bug13312StableTest extends TypeInferenceTestCase
9+
{
10+
11+
public static function dataFileAsserts(): iterable
12+
{
13+
yield from self::gatherAssertTypes(__DIR__ . '/data/bug-13312-stable.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-stable.neon',
33+
];
34+
}
35+
36+
}

‎tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverRuleTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ protected function createNodeScopeResolver(): NodeScopeResolver
144144
self::getContainer()->getByType(DeepNodeCloner::class),
145145
$this->shouldPolluteScopeWithLoopInitialAssignments(),
146146
$this->shouldPolluteScopeWithAlwaysIterableForeach(),
147+
self::getContainer()->getParameter('featureToggles')['narrowForeachBodyNonEmpty'],
147148
self::getContainer()->getParameter('polluteScopeWithBlock'),
148149
[],
149150
[],

‎tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ protected static function createNodeScopeResolver(): NodeScopeResolver
7777
$container->getByType(DeepNodeCloner::class),
7878
$container->getParameter('polluteScopeWithLoopInitialAssignments'),
7979
$container->getParameter('polluteScopeWithAlwaysIterableForeach'),
80+
$container->getParameter('featureToggles')['narrowForeachBodyNonEmpty'],
8081
$container->getParameter('polluteScopeWithBlock'),
8182
static::getEarlyTerminatingMethodCalls(),
8283
static::getEarlyTerminatingFunctionCalls(),

0 commit comments

Comments
 (0)