Skip to content

Commit 6c69c44

Browse files
ondrejmirtesondrejmirtes
committed
Revert "Track array value type overwrites in by-reference foreach without key variable (#5542)"
This reverts commit d883cd8.
1 parent d883cd8 commit 6c69c44

10 files changed

Lines changed: 52 additions & 376 deletions

File tree

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 43 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,41 +1294,19 @@ public function processStmtNode(
12941294

12951295
$originalKeyVarExpr = null;
12961296
$continueExitPointHasUnoriginalKeyType = false;
1297-
$byRefWithoutKey = $stmt->byRef
1298-
&& $stmt->keyVar === null
1299-
&& $stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name);
1300-
13011297
if ($stmt->keyVar instanceof Variable && is_string($stmt->keyVar->name)) {
13021298
$originalKeyVarExpr = new OriginalForeachKeyExpr($stmt->keyVar->name);
13031299
if ($finalScope->hasExpressionType($originalKeyVarExpr)->yes()) {
13041300
$scopesWithIterableValueType[] = $finalScope;
13051301
} else {
13061302
$continueExitPointHasUnoriginalKeyType = true;
13071303
}
1308-
} elseif ($byRefWithoutKey) {
1309-
$originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name);
1310-
if (!$finalScope->hasExpressionType($originalValueExpr)->yes()) {
1311-
$scopesWithIterableValueType[] = $finalScope;
1312-
} else {
1313-
$continueExitPointHasUnoriginalKeyType = true;
1314-
}
13151304
}
13161305

13171306
foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
13181307
$continueScope = $continueExitPoint->getScope();
13191308
$finalScope = $continueScope->mergeWith($finalScope);
1320-
if ($originalKeyVarExpr !== null) {
1321-
if (!$continueScope->hasExpressionType($originalKeyVarExpr)->yes()) {
1322-
$continueExitPointHasUnoriginalKeyType = true;
1323-
continue;
1324-
}
1325-
} elseif ($byRefWithoutKey) {
1326-
$originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name);
1327-
if ($continueScope->hasExpressionType($originalValueExpr)->yes()) {
1328-
$continueExitPointHasUnoriginalKeyType = true;
1329-
continue;
1330-
}
1331-
} else {
1309+
if ($originalKeyVarExpr === null || !$continueScope->hasExpressionType($originalKeyVarExpr)->yes()) {
13321310
$continueExitPointHasUnoriginalKeyType = true;
13331311
continue;
13341312
}
@@ -1349,82 +1327,58 @@ public function processStmtNode(
13491327
count($breakExitPoints) === 0
13501328
&& count($scopesWithIterableValueType) > 0
13511329
&& !$continueExitPointHasUnoriginalKeyType
1352-
&& ($stmt->keyVar !== null || $byRefWithoutKey)
1330+
&& $stmt->keyVar !== null
13531331
&& (!$hasExpr->no() || !$stmt->expr instanceof Variable)
13541332
&& $exprType->isArray()->yes()
13551333
&& $exprType->isConstantArray()->no()
13561334
) {
1357-
$nativeExprType = $scope->getNativeType($stmt->expr);
1358-
$arrayDimFetchLoopType = $exprType->getIterableValueType();
1359-
$arrayDimFetchLoopNativeType = $nativeExprType->getIterableValueType();
1360-
$keyLoopType = $exprType->getIterableKeyType();
1361-
$keyLoopNativeType = $nativeExprType->getIterableKeyType();
1362-
$valueTypeChanged = false;
1363-
$keyTypeChanged = false;
1364-
1365-
if ($stmt->keyVar !== null) {
1366-
$arrayExprDimFetch = new ArrayDimFetch($stmt->expr, $stmt->keyVar);
1367-
$originalValueExpr = null;
1368-
if ($stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name)) {
1369-
$originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name);
1370-
}
1371-
$arrayDimFetchLoopTypes = [];
1372-
$keyLoopTypes = [];
1373-
foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
1374-
$dimFetchType = $scopeWithIterableValueType->getType($arrayExprDimFetch);
1375-
// Condition-based narrowings like `is_string($type)` apply to the value
1376-
// variable but not automatically to the array dim fetch, even though the
1377-
// two describe the same element for a given iteration. If the value var
1378-
// hasn't been reassigned (OriginalForeachValueExpr still tracked) we use
1379-
// the narrowed value-var type in place of the broader dim fetch type so
1380-
// the loop's final array rewrite below picks up the sharper element type.
1381-
if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) {
1382-
$valueVarType = $scopeWithIterableValueType->getType($stmt->valueVar);
1383-
if ($dimFetchType->isSuperTypeOf($valueVarType)->yes()) {
1384-
$dimFetchType = $valueVarType;
1385-
}
1386-
}
1387-
$arrayDimFetchLoopTypes[] = $dimFetchType;
1388-
$keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
1389-
}
1390-
1391-
$arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes);
1392-
$keyLoopType = TypeCombinator::union(...$keyLoopTypes);
1393-
1394-
$arrayDimFetchLoopNativeTypes = [];
1395-
$keyLoopNativeTypes = [];
1396-
foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
1397-
$dimFetchNativeType = $scopeWithIterableValueType->getNativeType($arrayExprDimFetch);
1398-
if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) {
1399-
$valueVarNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar);
1400-
if ($dimFetchNativeType->isSuperTypeOf($valueVarNativeType)->yes()) {
1401-
$dimFetchNativeType = $valueVarNativeType;
1402-
}
1335+
$arrayExprDimFetch = new ArrayDimFetch($stmt->expr, $stmt->keyVar);
1336+
$originalValueExpr = null;
1337+
if ($stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name)) {
1338+
$originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name);
1339+
}
1340+
$arrayDimFetchLoopTypes = [];
1341+
$keyLoopTypes = [];
1342+
foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
1343+
$dimFetchType = $scopeWithIterableValueType->getType($arrayExprDimFetch);
1344+
// Condition-based narrowings like `is_string($type)` apply to the value
1345+
// variable but not automatically to the array dim fetch, even though the
1346+
// two describe the same element for a given iteration. If the value var
1347+
// hasn't been reassigned (OriginalForeachValueExpr still tracked) we use
1348+
// the narrowed value-var type in place of the broader dim fetch type so
1349+
// the loop's final array rewrite below picks up the sharper element type.
1350+
if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) {
1351+
$valueVarType = $scopeWithIterableValueType->getType($stmt->valueVar);
1352+
if ($dimFetchType->isSuperTypeOf($valueVarType)->yes()) {
1353+
$dimFetchType = $valueVarType;
14031354
}
1404-
$arrayDimFetchLoopNativeTypes[] = $dimFetchNativeType;
1405-
$keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
14061355
}
1356+
$arrayDimFetchLoopTypes[] = $dimFetchType;
1357+
$keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
1358+
}
14071359

1408-
$arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes);
1409-
$keyLoopNativeType = TypeCombinator::union(...$keyLoopNativeTypes);
1360+
$arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes);
1361+
$keyLoopType = TypeCombinator::union(...$keyLoopTypes);
14101362

1411-
$valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType());
1412-
$keyTypeChanged = !$keyLoopType->equals($exprType->getIterableKeyType());
1413-
} elseif ($byRefWithoutKey) {
1414-
$arrayDimFetchLoopTypes = [];
1415-
foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
1416-
$arrayDimFetchLoopTypes[] = $scopeWithIterableValueType->getType($stmt->valueVar);
1363+
$arrayDimFetchLoopNativeTypes = [];
1364+
$keyLoopNativeTypes = [];
1365+
foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
1366+
$dimFetchNativeType = $scopeWithIterableValueType->getNativeType($arrayExprDimFetch);
1367+
if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) {
1368+
$valueVarNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar);
1369+
if ($dimFetchNativeType->isSuperTypeOf($valueVarNativeType)->yes()) {
1370+
$dimFetchNativeType = $valueVarNativeType;
1371+
}
14171372
}
1418-
$arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes);
1373+
$arrayDimFetchLoopNativeTypes[] = $dimFetchNativeType;
1374+
$keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
1375+
}
14191376

1420-
$arrayDimFetchLoopNativeTypes = [];
1421-
foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
1422-
$arrayDimFetchLoopNativeTypes[] = $scopeWithIterableValueType->getNativeType($stmt->valueVar);
1423-
}
1424-
$arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes);
1377+
$arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes);
1378+
$keyLoopNativeType = TypeCombinator::union(...$keyLoopNativeTypes);
14251379

1426-
$valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType());
1427-
}
1380+
$valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType());
1381+
$keyTypeChanged = !$keyLoopType->equals($exprType->getIterableKeyType());
14281382

14291383
if ($valueTypeChanged || $keyTypeChanged) {
14301384
$newExprType = TypeTraverser::map($exprType, static function (Type $type, callable $traverse) use ($arrayDimFetchLoopType, $keyLoopType, $valueTypeChanged, $keyTypeChanged): Type {
@@ -1441,6 +1395,7 @@ public function processStmtNode(
14411395
$valueTypeChanged ? $arrayDimFetchLoopType : $type->getIterableValueType(),
14421396
);
14431397
});
1398+
$nativeExprType = $scope->getNativeType($stmt->expr);
14441399
$newExprNativeType = TypeTraverser::map($nativeExprType, static function (Type $type, callable $traverse) use ($arrayDimFetchLoopNativeType, $keyLoopNativeType, $valueTypeChanged, $keyTypeChanged): Type {
14451400
if ($type instanceof UnionType || $type instanceof IntersectionType) {
14461401
return $traverse($type);

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

Lines changed: 0 additions & 27 deletions
This file was deleted.

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ function foo(array $list): void
1313
$value = 'foo';
1414
}
1515

16-
assertType("list<'foo'>", $list);
16+
assertType('list<mixed>', $list);
1717
}
1818

1919
/**
@@ -56,7 +56,7 @@ function bar3(array $list): void
5656
}
5757
}
5858

59-
assertType("list<'foo'|'maybe'>", $list);
59+
assertType("list<mixed>", $list); // could be list<'foo'|'maybe'>
6060
}
6161

6262
/**
@@ -68,7 +68,7 @@ function baz(array $list): void
6868
$value = 'bar';
6969
}
7070

71-
assertType("list<'bar'>", $list);
71+
assertType('list<string>', $list);
7272
}
7373

7474
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ function example(array $convert): void {
1313
foreach ($convert as &$item) {
1414
$item = strtoupper($item);
1515
}
16-
assertType('list<uppercase-string>', $convert);
16+
assertType('list<string>', $convert);
1717
}
1818

1919
/**

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ function example(array $convert): void
1616
$val = strtoupper($val); // https://github.com/phpstan/phpstan/issues/14083
1717
}
1818
}
19-
assertType('array<string, list<uppercase-string>>', $convert);
19+
assertType('array<string, list<string>>', $convert);
2020
}
2121

2222
/**
@@ -29,7 +29,7 @@ function example2(array $convert): void
2929
$inner[$key] = strtoupper($val);
3030
}
3131
}
32-
assertType('array<string, list<uppercase-string>>', $convert);
32+
assertType('array<string, list<string>>', $convert);
3333
}
3434

3535
/**

‎tests/PHPStan/Analyser/nsrt/bug-1940-with-key.php‎

Lines changed: 0 additions & 59 deletions
This file was deleted.

0 commit comments

Comments
 (0)