Skip to content

Commit c4a3c75

Browse files
staabmphpstan-bot
authored andcommitted
Fix incorrect narrowing of nested array dim fetch after assignment in loop
When a constant array shape (e.g. array{-1: 0, 0: 0, 2: 0, 1: 0}) was assigned to a nested property dim fetch like $this->prop[$key], and then an inner loop modified it via ++$this->prop[$key][$offset], the constant shape was lost during loop processing. This caused false positive "Offset might not exist" errors even though all offsets were explicitly set. Two changes fix this: 1. In AssignHandler's descending phase, when the scope tracks a more precise type for an intermediate dim fetch and that type has all the offsets needed by the next level, use the scope's tracked type instead of recomputing from the parent's general array type. 2. In MutatingScope::generalizeVariableTypeHolders, when a parent expression is generalized and both scopes track a child expression as constant arrays with matching keys, generalize the child instead of invalidating it. This preserves the constant array shape through loop generalization. Fixes phpstan/phpstan#13669
1 parent c009cb0 commit c4a3c75

4 files changed

Lines changed: 76 additions & 2 deletions

File tree

src/Analyser/ExprHandler/AssignHandler.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -948,7 +948,8 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
948948

949949
$offsetValueTypeStack = [$offsetValueType];
950950
$generalizeOnWrite = $offsetTypes[array_key_last($offsetTypes)][0] !== null;
951-
foreach (array_slice($offsetTypes, 0, -1) as [$offsetType, $dimFetch]) {
951+
$slicedOffsets = array_slice($offsetTypes, 0, -1);
952+
foreach ($slicedOffsets as $k => [$offsetType, $dimFetch]) {
952953
if ($offsetType === null) {
953954
$offsetValueType = new ConstantArrayType([], []);
954955
$generalizeOnWrite = false;
@@ -959,7 +960,13 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
959960
} elseif ($has->maybe()) {
960961
if ($scope->hasExpressionType($dimFetch)->yes()) {
961962
$generalizeOnWrite = false;
962-
$offsetValueType = $offsetValueType->getOffsetValueType($offsetType);
963+
$scopeType = $scope->getType($dimFetch);
964+
$nextOffsetType = $offsetTypes[$k + 1][0] ?? null;
965+
if ($nextOffsetType !== null && $scopeType->hasOffsetValueType($nextOffsetType)->yes()) {
966+
$offsetValueType = $scopeType;
967+
} else {
968+
$offsetValueType = $offsetValueType->getOffsetValueType($offsetType);
969+
}
963970
} else {
964971
$offsetValueType = TypeCombinator::union($offsetValueType->getOffsetValueType($offsetType), new ConstantArrayType([], []));
965972
}

src/Analyser/MutatingScope.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3852,6 +3852,18 @@ private function generalizeVariableTypeHolders(
38523852
continue;
38533853
}
38543854

3855+
if (isset($otherVariableTypeHolders[$variableExprString])) {
3856+
$thisType = $variableTypeHolder->getType();
3857+
$otherType = $otherVariableTypeHolders[$variableExprString]->getType();
3858+
if (
3859+
$thisType->isConstantArray()->yes()
3860+
&& $otherType->isConstantArray()->yes()
3861+
&& $thisType->getIterableKeyType()->equals($otherType->getIterableKeyType())
3862+
) {
3863+
break;
3864+
}
3865+
}
3866+
38553867
continue 2;
38563868
}
38573869
if (!isset($otherVariableTypeHolders[$variableExprString])) {

tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,4 +1263,11 @@ public function testBug14308(): void
12631263
$this->analyse([__DIR__ . '/data/bug-14308.php'], []);
12641264
}
12651265

1266+
public function testBug13669(): void
1267+
{
1268+
$this->reportPossiblyNonexistentGeneralArrayOffset = true;
1269+
1270+
$this->analyse([__DIR__ . '/data/bug-13669.php'], []);
1271+
}
1272+
12661273
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug13669;
4+
5+
final class Foo
6+
{
7+
/**
8+
* @var array<int, array<MailStatus::CODE_*, int>>
9+
*/
10+
private array $mailCounts;
11+
12+
/** @var array<int, array<MailStatus::CODE_*>> */
13+
private array $sources;
14+
15+
/** @param array<int, array<MailStatus::CODE_*>> $sources */
16+
private function __construct(array $sources)
17+
{
18+
$this->mailCounts = [];
19+
$this->sources = $sources;
20+
}
21+
22+
public function countMailStates(): void
23+
{
24+
foreach ($this->sources as $templateId => $mails) {
25+
$this->mailCounts[$templateId] = [
26+
MailStatus::CODE_DELETED => 0,
27+
MailStatus::CODE_NOT_ACTIVE => 0,
28+
MailStatus::CODE_ACTIVE => 0,
29+
MailStatus::CODE_SIMULATION => 0,
30+
];
31+
32+
foreach ($mails as $mail) {
33+
++$this->mailCounts[$templateId][$mail];
34+
}
35+
}
36+
}
37+
}
38+
39+
final class MailStatus
40+
{
41+
public const CODE_DELETED = -1;
42+
43+
public const CODE_NOT_ACTIVE = 0;
44+
45+
public const CODE_SIMULATION = 1;
46+
47+
public const CODE_ACTIVE = 2;
48+
}

0 commit comments

Comments
 (0)