Skip to content

Commit c363781

Browse files
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 19151d3 commit c363781

15 files changed

Lines changed: 209 additions & 1 deletion

conf/config.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ parameters:
4646
rawMessageInBaseline: false
4747
reportNestedTooWideType: false
4848
assignToByRefForeachExpr: false
49+
narrowForeachBodyNonEmpty: false
4950
curlSetOptArrayTypes: false
5051
magicDirInInclude: false
5152
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: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@
144144
use PHPStan\ShouldNotHappenException;
145145
use PHPStan\TrinaryLogic;
146146
use PHPStan\Type\ClosureType;
147+
use PHPStan\Type\Constant\ConstantArrayType;
147148
use PHPStan\Type\Constant\ConstantIntegerType;
148149
use PHPStan\Type\Constant\ConstantStringType;
149150
use PHPStan\Type\FileTypeMapper;
@@ -263,6 +264,8 @@ public function __construct(
263264
private readonly bool $polluteScopeWithLoopInitialAssignments,
264265
#[AutowiredParameter]
265266
private readonly bool $polluteScopeWithAlwaysIterableForeach,
267+
#[AutowiredParameter(ref: '%featureToggles.narrowForeachBodyNonEmpty%')]
268+
private readonly bool $narrowForeachBodyNonEmpty,
266269
#[AutowiredParameter]
267270
private readonly bool $polluteScopeWithBlock,
268271
#[AutowiredParameter(ref: '%exceptions.implicitThrows%')]
@@ -1511,7 +1514,13 @@ public function processStmtNode(
15111514
$originalStorage = $storage;
15121515
$unrolledEndScope = null;
15131516
$unrolledTotalKeys = null;
1514-
$iterateeScope = $this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope;
1517+
// The loop body is only entered when the iteratee is non-empty. Under
1518+
// narrowForeachBodyNonEmpty we narrow it there (list to non-empty-list,
1519+
// array to non-empty-array) even with polluteScopeWithAlwaysIterableForeach
1520+
// off; with the toggle off the body scope is unchanged.
1521+
$iterateeScope = $this->narrowForeachBodyNonEmpty || $this->polluteScopeWithAlwaysIterableForeach
1522+
? $scope->filterByTruthyValue($arrayComparisonExpr)
1523+
: $scope;
15151524
if ($context->isTopLevel()) {
15161525
$storage = $originalStorage->duplicate();
15171526

@@ -1699,6 +1708,33 @@ public function processStmtNode(
16991708
}
17001709

17011710
$isIterableAtLeastOnce = $exprType->isIterableAtLeastOnce();
1711+
1712+
$iterateeCertainty = $finalScope->hasExpressionType($stmt->expr);
1713+
if (
1714+
$this->narrowForeachBodyNonEmpty
1715+
&& !$this->polluteScopeWithAlwaysIterableForeach
1716+
&& !$iterateeCertainty->no()
1717+
&& !$isIterableAtLeastOnce->yes()
1718+
) {
1719+
// With the flag off the after-loop scope must not assume the loop ran, so
1720+
// undo the body narrowing: restore the iteratee's possibly-empty-ness
1721+
// (keeping element types the body refined). Only the non-emptiness the
1722+
// narrowing added is stripped; an iteratee already non-empty before the
1723+
// loop keeps it, and a literal like `foreach ([1, 2] as $v)` is skipped.
1724+
$finalIterateeType = $finalScope->getType($stmt->expr);
1725+
if ($finalIterateeType->isArray()->yes()) {
1726+
$finalIterateeNativeType = $finalScope->getNativeType($stmt->expr);
1727+
$finalScope = $finalScope->specifyExpressionType(
1728+
$stmt->expr,
1729+
TypeCombinator::union($finalIterateeType, new ConstantArrayType([], [])),
1730+
$finalIterateeNativeType->isArray()->yes()
1731+
? TypeCombinator::union($finalIterateeNativeType, new ConstantArrayType([], []))
1732+
: $finalIterateeNativeType,
1733+
$iterateeCertainty,
1734+
);
1735+
}
1736+
}
1737+
17021738
if ($isIterableAtLeastOnce->maybe() || $exprType->isIterable()->no()) {
17031739
$finalScope = $finalScope->mergeWith($scope->filterByTruthyValue(new BooleanOr(
17041740
new BinaryOp\Identical(

src/Testing/RuleTestCase.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ protected function createNodeScopeResolver(): NodeScopeResolver
123123
self::createScopeFactory($reflectionProvider, $typeSpecifier),
124124
$this->shouldPolluteScopeWithLoopInitialAssignments(),
125125
$this->shouldPolluteScopeWithAlwaysIterableForeach(),
126+
self::getContainer()->getParameter('featureToggles')['narrowForeachBodyNonEmpty'],
126127
self::getContainer()->getParameter('polluteScopeWithBlock'),
127128
self::getContainer()->getParameter('exceptions')['implicitThrows'],
128129
$this->shouldTreatPhpDocTypesAsCertain(),

src/Testing/TypeInferenceTestCase.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ protected static function createNodeScopeResolver(): NodeScopeResolver
9898
self::createScopeFactory($reflectionProvider, $typeSpecifier),
9999
$container->getParameter('polluteScopeWithLoopInitialAssignments'),
100100
$container->getParameter('polluteScopeWithAlwaysIterableForeach'),
101+
$container->getParameter('featureToggles')['narrowForeachBodyNonEmpty'],
101102
$container->getParameter('polluteScopeWithBlock'),
102103
$container->getParameter('exceptions')['implicitThrows'],
103104
$container->getParameter('treatPhpDocTypesAsCertain'),

tests/PHPStan/Analyser/AnalyserTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,7 @@ private function createAnalyser(): Analyser
839839
self::createScopeFactory($reflectionProvider, $typeSpecifier),
840840
false,
841841
true,
842+
false,
842843
true,
843844
true,
844845
$this->shouldTreatPhpDocTypesAsCertain(),
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+
}
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
@@ -143,6 +143,7 @@ protected function createNodeScopeResolver(): NodeScopeResolver
143143
self::createScopeFactory($reflectionProvider, $typeSpecifier),
144144
$this->shouldPolluteScopeWithLoopInitialAssignments(),
145145
$this->shouldPolluteScopeWithAlwaysIterableForeach(),
146+
self::getContainer()->getParameter('featureToggles')['narrowForeachBodyNonEmpty'],
146147
self::getContainer()->getParameter('polluteScopeWithBlock'),
147148
self::getContainer()->getParameter('exceptions')['implicitThrows'],
148149
$this->shouldTreatPhpDocTypesAsCertain(),

tests/PHPStan/Analyser/Fiber/FiberNodeScopeResolverTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ protected static function createNodeScopeResolver(): NodeScopeResolver
7676
self::createScopeFactory($reflectionProvider, $typeSpecifier),
7777
$container->getParameter('polluteScopeWithLoopInitialAssignments'),
7878
$container->getParameter('polluteScopeWithAlwaysIterableForeach'),
79+
$container->getParameter('featureToggles')['narrowForeachBodyNonEmpty'],
7980
$container->getParameter('polluteScopeWithBlock'),
8081
$container->getParameter('exceptions')['implicitThrows'],
8182
$container->getParameter('treatPhpDocTypesAsCertain'),

0 commit comments

Comments
 (0)