Skip to content

Commit d2331ab

Browse files
committed
Fix phpstan/phpstan#14347: Array key set via ??= lost after modifying different key
- When an array dim fetch assignment (e.g. $arr['all']++) reassigns the root variable, expression types for dynamic array dim fetches (e.g. $arr[$card->value]) are now preserved - Added getDynamicArrayDimFetchExpressionTypes() and restoreExpressionTypes() to MutatingScope - New regression test in tests/PHPStan/Rules/Arrays/data/bug-14347.php
1 parent 6895e54 commit d2331ab

4 files changed

Lines changed: 114 additions & 0 deletions

File tree

src/Analyser/ExprHandler/AssignHandler.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,7 @@ public function processAssignVar(
469469

470470
if ($varType->isArray()->yes() || !(new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->yes()) {
471471
if ($var instanceof Variable && is_string($var->name)) {
472+
$preservedDimFetchTypes = $scope->getDynamicArrayDimFetchExpressionTypes($var->name);
472473
$nodeScopeResolver->callNodeCallback($nodeCallback, new VariableAssignNode($var, new TypeExpr($valueToWrite)), $scopeBeforeAssignEval, $storage);
473474
$scope = $scope->assignVariable($var->name, $valueToWrite, $nativeValueToWrite, TrinaryLogic::createYes());
474475
} else {
@@ -505,6 +506,11 @@ public function processAssignVar(
505506
$scope = $scope->assignExpression($expr, $type, $nativeType);
506507
}
507508

509+
if (isset($preservedDimFetchTypes)) {
510+
$scope = $scope->restoreExpressionTypes($preservedDimFetchTypes);
511+
unset($preservedDimFetchTypes);
512+
}
513+
508514
$setVarType = $scope->getType($originalVar->var);
509515
if (
510516
!$setVarType instanceof ErrorType

src/Analyser/MutatingScope.php

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2857,6 +2857,68 @@ public function assignInitializedProperty(Type $fetchedOnType, string $propertyN
28572857
return $this->assignExpression(new PropertyInitializationExpr($propertyName), new MixedType(), new MixedType());
28582858
}
28592859

2860+
/**
2861+
* @return list<array{string, ExpressionTypeHolder, ExpressionTypeHolder}>
2862+
*/
2863+
public function getDynamicArrayDimFetchExpressionTypes(string $variableName): array
2864+
{
2865+
$result = [];
2866+
foreach ($this->expressionTypes as $exprString => $exprTypeHolder) {
2867+
$expr = $exprTypeHolder->getExpr();
2868+
if (!$expr instanceof Expr\ArrayDimFetch) {
2869+
continue;
2870+
}
2871+
if (!$expr->var instanceof Variable || !is_string($expr->var->name) || $expr->var->name !== $variableName) {
2872+
continue;
2873+
}
2874+
if ($expr->dim === null || $expr->dim instanceof Scalar) {
2875+
continue;
2876+
}
2877+
$nativeHolder = $this->nativeExpressionTypes[$exprString] ?? $exprTypeHolder;
2878+
$result[] = [$exprString, $exprTypeHolder, $nativeHolder];
2879+
}
2880+
return $result;
2881+
}
2882+
2883+
/**
2884+
* @param list<array{string, ExpressionTypeHolder, ExpressionTypeHolder}> $holders
2885+
*/
2886+
public function restoreExpressionTypes(array $holders): self
2887+
{
2888+
$expressionTypes = $this->expressionTypes;
2889+
$nativeExpressionTypes = $this->nativeExpressionTypes;
2890+
$changed = false;
2891+
foreach ($holders as [$exprString, $holder, $nativeHolder]) {
2892+
if (isset($expressionTypes[$exprString])) {
2893+
continue;
2894+
}
2895+
$expressionTypes[$exprString] = $holder;
2896+
$nativeExpressionTypes[$exprString] = $nativeHolder;
2897+
$changed = true;
2898+
}
2899+
if (!$changed) {
2900+
return $this;
2901+
}
2902+
return $this->scopeFactory->create(
2903+
$this->context,
2904+
$this->isDeclareStrictTypes(),
2905+
$this->getFunction(),
2906+
$this->getNamespace(),
2907+
$expressionTypes,
2908+
$nativeExpressionTypes,
2909+
$this->conditionalExpressions,
2910+
$this->inClosureBindScopeClasses,
2911+
$this->anonymousFunctionReflection,
2912+
$this->inFirstLevelStatement,
2913+
$this->currentlyAssignedExpressions,
2914+
$this->currentlyAllowedUndefinedExpressions,
2915+
$this->inFunctionCallsStack,
2916+
$this->afterExtractCall,
2917+
$this->parentScope,
2918+
$this->nativeTypesPromoted,
2919+
);
2920+
}
2921+
28602922
public function invalidateExpression(Expr $expressionToInvalidate, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): self
28612923
{
28622924
$expressionTypes = $this->expressionTypes;

tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php

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

1266+
#[RequiresPhp('>= 8.1')]
1267+
public function testBug14347(): void
1268+
{
1269+
$this->reportPossiblyNonexistentConstantArrayOffset = true;
1270+
1271+
$this->analyse([__DIR__ . '/data/bug-14347.php'], []);
1272+
}
1273+
12661274
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php // lint >= 8.1
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug14347;
6+
7+
enum Suit: string {
8+
case Clubs = 'c';
9+
case Diamonds = 'd';
10+
case Hearts = 'h';
11+
case Spades = 's';
12+
}
13+
/**
14+
* @param array<array-key, Suit> $cards
15+
* @return array<non-empty-string, non-negative-int>
16+
*/
17+
function countCards(array $cards): array {
18+
$cardCounts = ['all' => 0];
19+
foreach ($cards as $card) {
20+
$cardCounts['all']++;
21+
$cardCounts[$card->value] ??= 0;
22+
$cardCounts[$card->value]++;
23+
}
24+
return $cardCounts;
25+
}
26+
/**
27+
* @param array<array-key, Suit> $cards
28+
* @return array<non-empty-string, non-negative-int>
29+
*/
30+
function countCardsBroken(array $cards): array {
31+
$cardCounts = ['all' => 0];
32+
foreach ($cards as $card) {
33+
$cardCounts[$card->value] ??= 0;
34+
$cardCounts['all']++;
35+
$cardCounts[$card->value]++;
36+
}
37+
return $cardCounts;
38+
}

0 commit comments

Comments
 (0)