Skip to content

Commit e1c87da

Browse files
VincentLangletphpstan-bot
VincentLanglet
authored and
phpstan-bot
committed
Track array value type overwrites in by-reference foreach without key variable
- Extend post-loop array type rewrite in NodeScopeResolver to handle `foreach ($arr as &$v)` (without `$key =>`) by tracking the value variable's type via `OriginalForeachValueExpr` instead of relying on `OriginalForeachKeyExpr` + `ArrayDimFetch` - When the value is always overwritten (OriginalForeachValueExpr absent in all paths), replace the array's value type with the final value variable type — matching the existing behavior for the key-based case - Handle continue exit points for the by-ref without key case with the same logic: check OriginalForeachValueExpr presence in each continue scope - Update assertions in existing tests (bug-13809, bug-14083, bug-14084, bug-14124, bug-14124b, overwritten-arrays) that were asserting the old imprecise behavior
1 parent 7604335 commit e1c87da

8 files changed

Lines changed: 290 additions & 52 deletions

File tree

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 88 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,19 +1294,41 @@ 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+
12971301
if ($stmt->keyVar instanceof Variable && is_string($stmt->keyVar->name)) {
12981302
$originalKeyVarExpr = new OriginalForeachKeyExpr($stmt->keyVar->name);
12991303
if ($finalScope->hasExpressionType($originalKeyVarExpr)->yes()) {
13001304
$scopesWithIterableValueType[] = $finalScope;
13011305
} else {
13021306
$continueExitPointHasUnoriginalKeyType = true;
13031307
}
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+
}
13041315
}
13051316

13061317
foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
13071318
$continueScope = $continueExitPoint->getScope();
13081319
$finalScope = $continueScope->mergeWith($finalScope);
1309-
if ($originalKeyVarExpr === null || !$continueScope->hasExpressionType($originalKeyVarExpr)->yes()) {
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 {
13101332
$continueExitPointHasUnoriginalKeyType = true;
13111333
continue;
13121334
}
@@ -1327,58 +1349,82 @@ public function processStmtNode(
13271349
count($breakExitPoints) === 0
13281350
&& count($scopesWithIterableValueType) > 0
13291351
&& !$continueExitPointHasUnoriginalKeyType
1330-
&& $stmt->keyVar !== null
1352+
&& ($stmt->keyVar !== null || $byRefWithoutKey)
13311353
&& (!$hasExpr->no() || !$stmt->expr instanceof Variable)
13321354
&& $exprType->isArray()->yes()
13331355
&& $exprType->isConstantArray()->no()
13341356
) {
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;
1354-
}
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 ($byRefWithoutKey) {
1366+
$arrayDimFetchLoopTypes = [];
1367+
foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
1368+
$arrayDimFetchLoopTypes[] = $scopeWithIterableValueType->getType($stmt->valueVar);
13551369
}
1356-
$arrayDimFetchLoopTypes[] = $dimFetchType;
1357-
$keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
1358-
}
1370+
$arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes);
13591371

1360-
$arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes);
1361-
$keyLoopType = TypeCombinator::union(...$keyLoopTypes);
1372+
$arrayDimFetchLoopNativeTypes = [];
1373+
foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
1374+
$arrayDimFetchLoopNativeTypes[] = $scopeWithIterableValueType->getNativeType($stmt->valueVar);
1375+
}
1376+
$arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes);
1377+
1378+
$valueTypeChanged = !$arrayDimFetchLoopType->equals($exprType->getIterableValueType());
1379+
} elseif ($stmt->keyVar !== null) {
1380+
$arrayExprDimFetch = new ArrayDimFetch($stmt->expr, $stmt->keyVar);
1381+
$originalValueExpr = null;
1382+
if ($stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name)) {
1383+
$originalValueExpr = new OriginalForeachValueExpr($stmt->valueVar->name);
1384+
}
1385+
$arrayDimFetchLoopTypes = [];
1386+
$keyLoopTypes = [];
1387+
foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
1388+
$dimFetchType = $scopeWithIterableValueType->getType($arrayExprDimFetch);
1389+
// Condition-based narrowings like `is_string($type)` apply to the value
1390+
// variable but not automatically to the array dim fetch, even though the
1391+
// two describe the same element for a given iteration. If the value var
1392+
// hasn't been reassigned (OriginalForeachValueExpr still tracked) we use
1393+
// the narrowed value-var type in place of the broader dim fetch type so
1394+
// the loop's final array rewrite below picks up the sharper element type.
1395+
if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) {
1396+
$valueVarType = $scopeWithIterableValueType->getType($stmt->valueVar);
1397+
if ($dimFetchType->isSuperTypeOf($valueVarType)->yes()) {
1398+
$dimFetchType = $valueVarType;
1399+
}
1400+
}
1401+
$arrayDimFetchLoopTypes[] = $dimFetchType;
1402+
$keyLoopTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
1403+
}
13621404

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;
1405+
$arrayDimFetchLoopType = TypeCombinator::union(...$arrayDimFetchLoopTypes);
1406+
$keyLoopType = TypeCombinator::union(...$keyLoopTypes);
1407+
1408+
$arrayDimFetchLoopNativeTypes = [];
1409+
$keyLoopNativeTypes = [];
1410+
foreach ($scopesWithIterableValueType as $scopeWithIterableValueType) {
1411+
$dimFetchNativeType = $scopeWithIterableValueType->getNativeType($arrayExprDimFetch);
1412+
if ($originalValueExpr !== null && $scopeWithIterableValueType->hasExpressionType($originalValueExpr)->yes()) {
1413+
$valueVarNativeType = $scopeWithIterableValueType->getNativeType($stmt->valueVar);
1414+
if ($dimFetchNativeType->isSuperTypeOf($valueVarNativeType)->yes()) {
1415+
$dimFetchNativeType = $valueVarNativeType;
1416+
}
13711417
}
1418+
$arrayDimFetchLoopNativeTypes[] = $dimFetchNativeType;
1419+
$keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
13721420
}
1373-
$arrayDimFetchLoopNativeTypes[] = $dimFetchNativeType;
1374-
$keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($stmt->keyVar);
1375-
}
13761421

1377-
$arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes);
1378-
$keyLoopNativeType = TypeCombinator::union(...$keyLoopNativeTypes);
1422+
$arrayDimFetchLoopNativeType = TypeCombinator::union(...$arrayDimFetchLoopNativeTypes);
1423+
$keyLoopNativeType = TypeCombinator::union(...$keyLoopNativeTypes);
13791424

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

13831429
if ($valueTypeChanged || $keyTypeChanged) {
13841430
$newExprType = TypeTraverser::map($exprType, static function (Type $type, callable $traverse) use ($arrayDimFetchLoopType, $keyLoopType, $valueTypeChanged, $keyTypeChanged): Type {
@@ -1395,7 +1441,6 @@ public function processStmtNode(
13951441
$valueTypeChanged ? $arrayDimFetchLoopType : $type->getIterableValueType(),
13961442
);
13971443
});
1398-
$nativeExprType = $scope->getNativeType($stmt->expr);
13991444
$newExprNativeType = TypeTraverser::map($nativeExprType, static function (Type $type, callable $traverse) use ($arrayDimFetchLoopNativeType, $keyLoopNativeType, $valueTypeChanged, $keyTypeChanged): Type {
14001445
if ($type instanceof UnionType || $type instanceof IntersectionType) {
14011446
return $traverse($type);

‎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<mixed>', $list);
16+
assertType("list<'foo'>", $list);
1717
}
1818

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

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

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

71-
assertType('list<string>', $list);
71+
assertType("list<'bar'>", $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<string>', $convert);
16+
assertType('list<uppercase-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<string>>', $convert);
19+
assertType('array<string, list<uppercase-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<string>>', $convert);
32+
assertType('array<string, list<uppercase-string>>', $convert);
3333
}
3434

3535
/**

0 commit comments

Comments
 (0)