Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ parameters:
rawMessageInBaseline: false
reportNestedTooWideType: false
assignToByRefForeachExpr: false
narrowForeachBodyNonEmpty: false
curlSetOptArrayTypes: false
magicDirInInclude: false
checkDateIntervalConstructor: false
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ parametersSchema:
rawMessageInBaseline: bool()
reportNestedTooWideType: bool()
assignToByRefForeachExpr: bool()
narrowForeachBodyNonEmpty: bool()
curlSetOptArrayTypes: bool()
magicDirInInclude: bool()
checkDateIntervalConstructor: bool()
Expand Down
41 changes: 38 additions & 3 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -266,6 +267,8 @@
private readonly bool $polluteScopeWithLoopInitialAssignments,
#[AutowiredParameter]
private readonly bool $polluteScopeWithAlwaysIterableForeach,
#[AutowiredParameter(ref: '%featureToggles.narrowForeachBodyNonEmpty%')]
private readonly bool $narrowForeachBodyNonEmpty,
#[AutowiredParameter]
private readonly bool $polluteScopeWithBlock,
#[AutowiredParameter]
Expand Down Expand Up @@ -1487,6 +1490,11 @@
$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;
Expand All @@ -1509,7 +1517,7 @@
if ($context->isTopLevel()) {
$storage = $originalStorage->duplicate();

$originalScope = $this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope;
Comment thread
staabm marked this conversation as resolved.
$originalScope = $narrowForeachBody ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope;
$unrolledResult = $this->tryProcessUnrolledConstantArrayForeach($stmt, $originalScope, $originalStorage, $context);
if ($unrolledResult !== null) {
$bodyScope = $unrolledResult['bodyScope'];
Expand All @@ -1520,7 +1528,7 @@
$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();
Expand All @@ -1540,7 +1548,7 @@
}
}

$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;
Expand Down Expand Up @@ -1688,6 +1696,33 @@
}

$isIterableAtLeastOnce = $exprType->isIterableAtLeastOnce();

$iterateeCertainty = $finalScope->hasExpressionType($stmt->expr);
if (
$this->narrowForeachBodyNonEmpty
&& !$this->polluteScopeWithAlwaysIterableForeach
&& !$iterateeCertainty->no()

Check warning on line 1704 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ if ( $this->narrowForeachBodyNonEmpty && !$this->polluteScopeWithAlwaysIterableForeach - && !$iterateeCertainty->no() + && $iterateeCertainty->yes() && !$isIterableAtLeastOnce->yes() ) { // With the flag off the after-loop scope must not assume the loop ran, so
&& !$isIterableAtLeastOnce->yes()

Check warning on line 1705 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $this->narrowForeachBodyNonEmpty && !$this->polluteScopeWithAlwaysIterableForeach && !$iterateeCertainty->no() - && !$isIterableAtLeastOnce->yes() + && $isIterableAtLeastOnce->no() ) { // 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

Check warning on line 1705 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $this->narrowForeachBodyNonEmpty && !$this->polluteScopeWithAlwaysIterableForeach && !$iterateeCertainty->no() - && !$isIterableAtLeastOnce->yes() + && $isIterableAtLeastOnce->no() ) { // 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
) {
// 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(
Expand Down
1 change: 1 addition & 0 deletions src/Testing/RuleTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
[],
[],
Expand Down
1 change: 1 addition & 0 deletions src/Testing/TypeInferenceTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/AnalyserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,7 @@ private function createAnalyser(): Analyser
$container->getByType(DeepNodeCloner::class),
false,
true,
false,
true,
[],
[],
Expand Down
36 changes: 36 additions & 0 deletions tests/PHPStan/Analyser/Bug13312NoPolluteTest.php
Original file line number Diff line number Diff line change
@@ -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',
];
}

}
36 changes: 36 additions & 0 deletions tests/PHPStan/Analyser/Bug13312StableTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php declare(strict_types = 1);

namespace PHPStan\Analyser;

use PHPStan\Testing\TypeInferenceTestCase;
use PHPUnit\Framework\Attributes\DataProvider;

class Bug13312StableTest extends TypeInferenceTestCase
{

public static function dataFileAsserts(): iterable
{
yield from self::gatherAssertTypes(__DIR__ . '/data/bug-13312-stable.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-stable.neon',
];
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
[],
[],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
4 changes: 4 additions & 0 deletions tests/PHPStan/Analyser/bug-13312-no-pollute.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
parameters:
polluteScopeWithAlwaysIterableForeach: false
featureToggles:
narrowForeachBodyNonEmpty: true
Comment on lines +2 to +4

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need more tests with other combinations of this toggles?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added Bug13312StableTest for the toggle-off case (foreach body left as list<mixed>, while the separate for-loop narrowing still applies). Bug13312NoPolluteTest covers toggle-on. Happy to add pollute-on combinations too if you want them.

2 changes: 2 additions & 0 deletions tests/PHPStan/Analyser/bug-13312-stable.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
parameters:
polluteScopeWithAlwaysIterableForeach: false
37 changes: 37 additions & 0 deletions tests/PHPStan/Analyser/data/bug-13312-no-pollute.php
Original file line number Diff line number Diff line change
@@ -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);
}
32 changes: 32 additions & 0 deletions tests/PHPStan/Analyser/data/bug-13312-stable.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php declare(strict_types = 1);

namespace Bug13312Stable;

use function PHPStan\Testing\assertType;

// With polluteScopeWithAlwaysIterableForeach off and narrowForeachBodyNonEmpty off
// (the stable default), the foreach body does not narrow the iterated expression.
// The for-loop narrowing is a separate mechanism and still applies.

/** @param list<mixed> $arr */
function foo(array $arr): void {
assertType('list<mixed>', $arr);
foreach ($arr as $v) {
assertType('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('array<string, int>', $arr);
}
assertType('array<string, int>', $arr);
}
18 changes: 18 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-13312.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading