Skip to content
Closed
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
23 changes: 20 additions & 3 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -4851,12 +4851,24 @@ public function processClosureScope(
);
}

public function processAlwaysIterableForeachScopeWithoutPollute(self $finalScope): self
public function processAlwaysIterableForeachScopeWithoutPollute(self $finalScope, Node\Stmt\Foreach_ $stmt): self
{
$keyVarExprString = $stmt->keyVar instanceof Variable && is_string($stmt->keyVar->name)
? '$' . $stmt->keyVar->name
: null;
$valueVarExprString = $stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name)
? '$' . $stmt->valueVar->name
: null;

$expressionTypes = $this->expressionTypes;
foreach ($finalScope->expressionTypes as $variableExprString => $variableTypeHolder) {
if (!isset($expressionTypes[$variableExprString])) {
$expressionTypes[$variableExprString] = ExpressionTypeHolder::createMaybe($variableTypeHolder->getExpr(), $variableTypeHolder->getType());
if ($variableExprString === $keyVarExprString || $variableExprString === $valueVarExprString) {
$expressionTypes[$variableExprString] = ExpressionTypeHolder::createMaybe($variableTypeHolder->getExpr(), $variableTypeHolder->getType());
continue;
}

$expressionTypes[$variableExprString] = $variableTypeHolder;
continue;
}

Expand All @@ -4869,7 +4881,12 @@ public function processAlwaysIterableForeachScopeWithoutPollute(self $finalScope
$nativeTypes = $this->nativeExpressionTypes;
foreach ($finalScope->nativeExpressionTypes as $variableExprString => $variableTypeHolder) {
if (!isset($nativeTypes[$variableExprString])) {
$nativeTypes[$variableExprString] = ExpressionTypeHolder::createMaybe($variableTypeHolder->getExpr(), $variableTypeHolder->getType());
if ($variableExprString === $keyVarExprString || $variableExprString === $valueVarExprString) {
$nativeTypes[$variableExprString] = ExpressionTypeHolder::createMaybe($variableTypeHolder->getExpr(), $variableTypeHolder->getType());
continue;
}

$nativeTypes[$variableExprString] = $variableTypeHolder;
continue;
}

Expand Down
3 changes: 1 addition & 2 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -1119,8 +1119,7 @@ private function processStmtNode(
} elseif ($isIterableAtLeastOnce->no() || $finalScopeResult->isAlwaysTerminating()) {
$finalScope = $scope;
} elseif (!$this->polluteScopeWithAlwaysIterableForeach) {
$finalScope = $scope->processAlwaysIterableForeachScopeWithoutPollute($finalScope);
// get types from finalScope, but don't create new variables
$finalScope = $scope->processAlwaysIterableForeachScopeWithoutPollute($finalScope, $stmt);
}

if (!$isIterableAtLeastOnce->no()) {
Expand Down
36 changes: 36 additions & 0 deletions tests/PHPStan/Analyser/ForeachLoopNoScopePollutionTest.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;

class ForeachLoopNoScopePollutionTest extends TypeInferenceTestCase
{

/** @return iterable<array<string, mixed[]>> */
public function dataFileAsserts(): iterable
{
yield from $this->gatherAssertTypes(__DIR__ . '/data/foreach-loop-no-scope-pollution.php');
}

/**
* @dataProvider dataFileAsserts
* @param mixed ...$args
*/
public function testFileAsserts(
string $assertType,
string $file,
...$args,
): void
{
$this->assertFileAsserts($assertType, $file, ...$args);
}

public static function getAdditionalConfigFiles(): array
{
return [
__DIR__ . '/foreachLoopNoScopePollution.neon',
];
}

}
115 changes: 115 additions & 0 deletions tests/PHPStan/Analyser/data/foreach-loop-no-scope-pollution.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
<?php declare(strict_types=1);

namespace ForEachLoopNoScopePollution;

use PHPStan\TrinaryLogic;
use function PHPStan\Testing\assertNativeType;
use function PHPStan\Testing\assertType;
use function PHPStan\Testing\assertVariableCertainty;

class ForEachLoopNoScopePollution
{

/** @param int $b */
public function loopThatIteratesAtLeastOnce(int $a, $b): void
{
$items = [17 => 'foo', 'bar' => 19];

foreach ($items as $key => $item) {
$a = rand(0, 1);
$b = rand(0, 1);
$c = rand(0, 1);
}

assertType("17|'bar'", $key);
assertNativeType("17|'bar'", $key);
assertVariableCertainty(TrinaryLogic::createMaybe(), $key);

assertType("19|'foo'", $item);
assertNativeType("19|'foo'", $item);
assertVariableCertainty(TrinaryLogic::createMaybe(), $item);

assertType('int<0, 1>', $a);
assertNativeType('int', $a);
assertVariableCertainty(TrinaryLogic::createYes(), $a);

assertType('int<0, 1>', $b);
assertNativeType('int', $b);
assertVariableCertainty(TrinaryLogic::createYes(), $b);

assertType('int<0, 1>', $c);
assertNativeType('int', $c);
assertVariableCertainty(TrinaryLogic::createYes(), $c);
Copy link
Member

Choose a reason for hiding this comment

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

This should not be yes but maybe. That's the point of polluteScopeWithAlwaysIterableForeach: false.

Please if for loop currently behaves like that then please fix the behaviour too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, the docs just talk about variables set in the loop init statements or the key and value variables. If so, my previous PR basically needs to be reverted and the issue it fixed was by design? :)

I can do this in the evening btw.

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah, sorry about that phpstan/phpstan#9550 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries. Should I open a PR for all those tests only? In case they add value, otherwise I don't mind of course.

Copy link
Member

Choose a reason for hiding this comment

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

It's fine, I think it's well tested :)

}

/** @param int $b */
public function loopThatMightIterateAtLeastOnce(int $a, $b): void
{
$items = [];
if (rand(0, 1)) {
$items[17] = 'foo';
}
if (rand(0, 1)) {
$items['bar'] = 19;
}

foreach ($items as $key => $item) {
$a = rand(0, 1);
$b = rand(0, 1);
$c = rand(0, 1);
}

assertType("17|'bar'", $key);
assertNativeType("17|'bar'", $key);
assertVariableCertainty(TrinaryLogic::createMaybe(), $key);

assertType("19|'foo'", $item);
assertNativeType("19|'foo'", $item);
assertVariableCertainty(TrinaryLogic::createMaybe(), $item);

assertType('int', $a);
assertNativeType('int', $a);
assertVariableCertainty(TrinaryLogic::createYes(), $a);

assertType('int', $b);
assertNativeType('mixed', $b);
assertVariableCertainty(TrinaryLogic::createYes(), $b);

assertType('int<0, 1>', $c);
assertNativeType('int', $c);
assertVariableCertainty(TrinaryLogic::createMaybe(), $c);
}

/** @param int $b */
public function loopThatNeverIterates(int $a, $b): void
{
$items = [];

foreach ($items as $key => $item) {
$a = rand(0, 1);
$b = rand(0, 1);
$c = rand(0, 1);
}

assertType('*ERROR*', $key);
assertNativeType('*ERROR*', $key);
assertVariableCertainty(TrinaryLogic::createNo(), $key);

assertType('*ERROR*', $item);
assertNativeType('*ERROR*', $item);
assertVariableCertainty(TrinaryLogic::createNo(), $item);

assertType('int', $a);
assertNativeType('int', $a);
assertVariableCertainty(TrinaryLogic::createYes(), $a);

assertType('int', $b);
assertNativeType('mixed', $b);
assertVariableCertainty(TrinaryLogic::createYes(), $b);

assertType('*ERROR*', $c);
assertNativeType('*ERROR*', $c);
assertVariableCertainty(TrinaryLogic::createNo(), $c);
}

}
2 changes: 2 additions & 0 deletions tests/PHPStan/Analyser/foreachLoopNoScopePollution.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
parameters:
polluteScopeWithAlwaysIterableForeach: false
115 changes: 115 additions & 0 deletions tests/PHPStan/Analyser/nsrt/foreach-loop.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
<?php declare(strict_types=1);

namespace ForEachLoop;

use PHPStan\TrinaryLogic;
use function PHPStan\Testing\assertNativeType;
use function PHPStan\Testing\assertType;
use function PHPStan\Testing\assertVariableCertainty;

class ForEachLoop
{

/** @param int $b */
public function loopThatIteratesAtLeastOnce(int $a, $b): void
{
$items = [17 => 'foo', 'bar' => 19];

foreach ($items as $key => $item) {
$a = rand(0, 1);
$b = rand(0, 1);
$c = rand(0, 1);
}

assertType("17|'bar'", $key);
assertNativeType("17|'bar'", $key);
assertVariableCertainty(TrinaryLogic::createYes(), $key);

assertType("19|'foo'", $item);
assertNativeType("19|'foo'", $item);
assertVariableCertainty(TrinaryLogic::createYes(), $item);

assertType('int<0, 1>', $a);
assertNativeType('int', $a);
assertVariableCertainty(TrinaryLogic::createYes(), $a);

assertType('int<0, 1>', $b);
assertNativeType('int', $b);
assertVariableCertainty(TrinaryLogic::createYes(), $b);

assertType('int<0, 1>', $c);
assertNativeType('int', $c);
assertVariableCertainty(TrinaryLogic::createYes(), $c);
}

/** @param int $b */
public function loopThatMightIterateAtLeastOnce(int $a, $b): void
{
$items = [];
if (rand(0, 1)) {
$items[17] = 'foo';
}
if (rand(0, 1)) {
$items['bar'] = 19;
}

foreach ($items as $key => $item) {
$a = rand(0, 1);
$b = rand(0, 1);
$c = rand(0, 1);
}

assertType("17|'bar'", $key);
assertNativeType("17|'bar'", $key);
assertVariableCertainty(TrinaryLogic::createMaybe(), $key);

assertType("19|'foo'", $item);
assertNativeType("19|'foo'", $item);
assertVariableCertainty(TrinaryLogic::createMaybe(), $item);

assertType('int', $a);
assertNativeType('int', $a);
assertVariableCertainty(TrinaryLogic::createYes(), $a);

assertType('int', $b);
assertNativeType('mixed', $b);
assertVariableCertainty(TrinaryLogic::createYes(), $b);

assertType('int<0, 1>', $c);
assertNativeType('int', $c);
assertVariableCertainty(TrinaryLogic::createMaybe(), $c);
}

/** @param int $b */
public function loopThatNeverIterates(int $a, $b): void
{
$items = [];

foreach ($items as $key => $item) {
$a = rand(0, 1);
$b = rand(0, 1);
$c = rand(0, 1);
}

assertType('*ERROR*', $key);
assertNativeType('*ERROR*', $key);
assertVariableCertainty(TrinaryLogic::createNo(), $key);

assertType('*ERROR*', $item);
assertNativeType('*ERROR*', $item);
assertVariableCertainty(TrinaryLogic::createNo(), $item);

assertType('int', $a);
assertNativeType('int', $a);
assertVariableCertainty(TrinaryLogic::createYes(), $a);

assertType('int', $b);
assertNativeType('mixed', $b);
assertVariableCertainty(TrinaryLogic::createYes(), $b);

assertType('*ERROR*', $c);
assertNativeType('*ERROR*', $c);
assertVariableCertainty(TrinaryLogic::createNo(), $c);
}

}
4 changes: 0 additions & 4 deletions tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -525,10 +525,6 @@ public function dataForeachPolluteScopeWithAlwaysIterableForeach(): array
'Variable $val might not be defined.',
20,
],
[
'Variable $test might not be defined.',
21,
],
[
'Variable $key might not be defined.',
32,
Expand Down
Loading