Skip to content

Commit 85c4052

Browse files
phpstan-botVincentLangletclaude
authored
Fix phpstan/phpstan#14333: Setting an array key doesn't update a reference (#5257)
Co-authored-by: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Vincent Langlet <vincentlanglet@hotmail.fr>
1 parent bebb16f commit 85c4052

File tree

5 files changed

+324
-20
lines changed

5 files changed

+324
-20
lines changed

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
use PHPStan\Type\ConstantTypeHelper;
5656
use PHPStan\Type\ErrorType;
5757
use PHPStan\Type\IntegerRangeType;
58+
use PHPStan\Type\IntegerType;
5859
use PHPStan\Type\MixedType;
5960
use PHPStan\Type\ObjectType;
6061
use PHPStan\Type\StaticTypeFactory;
@@ -69,6 +70,7 @@
6970
use function array_slice;
7071
use function count;
7172
use function in_array;
73+
use function is_int;
7274
use function is_string;
7375

7476
/**
@@ -315,6 +317,10 @@ public function processAssignVar(
315317
foreach ($conditionalExpressions as $exprString => $holders) {
316318
$scope = $scope->addConditionalExpressions($exprString, $holders);
317319
}
320+
321+
if ($assignedExpr instanceof Expr\Array_) {
322+
$scope = $this->processArrayByRefItems($scope, $var->name, $assignedExpr, new Variable($var->name));
323+
}
318324
} else {
319325
$nameExprResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context);
320326
$hasYield = $hasYield || $nameExprResult->hasYield();
@@ -936,6 +942,67 @@ private function isImplicitArrayCreation(array $dimFetchStack, Scope $scope): Tr
936942
return $scope->hasVariableType($varNode->name)->negate();
937943
}
938944

945+
private function processArrayByRefItems(MutatingScope $scope, string $rootVarName, Expr\Array_ $arrayExpr, Expr $parentExpr): MutatingScope
946+
{
947+
$implicitIndex = 0;
948+
foreach ($arrayExpr->items as $arrayItem) {
949+
if ($arrayItem->key !== null) {
950+
$keyType = $scope->getType($arrayItem->key)->toArrayKey();
951+
952+
if ($implicitIndex !== null) {
953+
$keyValues = $keyType->getConstantScalarValues();
954+
if (count($keyValues) === 1) {
955+
$keyValue = $keyValues[0];
956+
if (is_int($keyValue) && $keyValue >= $implicitIndex) {
957+
$implicitIndex = $keyValue + 1;
958+
}
959+
} elseif (!$keyType->isInteger()->no()) {
960+
// Key could be an integer, but we don't know which one,
961+
// so subsequent implicit indices are unpredictable
962+
$implicitIndex = null;
963+
}
964+
}
965+
966+
$dimExpr = $arrayItem->key;
967+
} elseif ($implicitIndex !== null) {
968+
$dimExpr = new Node\Scalar\Int_($implicitIndex);
969+
$implicitIndex++;
970+
} else {
971+
$dimExpr = new TypeExpr(new IntegerType());
972+
}
973+
974+
if ($arrayItem->value instanceof Expr\Array_) {
975+
$dimFetchExpr = new ArrayDimFetch($parentExpr, $dimExpr);
976+
$scope = $this->processArrayByRefItems($scope, $rootVarName, $arrayItem->value, $dimFetchExpr);
977+
}
978+
979+
if (!$arrayItem->byRef || !$arrayItem->value instanceof Variable || !is_string($arrayItem->value->name)) {
980+
continue;
981+
}
982+
983+
$refVarName = $arrayItem->value->name;
984+
$dimFetchExpr = new ArrayDimFetch($parentExpr, $dimExpr);
985+
$refType = $scope->getType(new Variable($refVarName));
986+
$refNativeType = $scope->getNativeType(new Variable($refVarName));
987+
988+
// When $rootVarName's array key changes, update $refVarName
989+
$scope = $scope->assignExpression(
990+
new IntertwinedVariableByReferenceWithExpr($rootVarName, new Variable($refVarName), $dimFetchExpr),
991+
$refType,
992+
$refNativeType,
993+
);
994+
995+
// When $refVarName changes, update $rootVarName's array key
996+
$scope = $scope->assignExpression(
997+
new IntertwinedVariableByReferenceWithExpr($refVarName, $dimFetchExpr, new Variable($refVarName)),
998+
$refType,
999+
$refNativeType,
1000+
);
1001+
}
1002+
1003+
return $scope;
1004+
}
1005+
9391006
/**
9401007
* @param list<ArrayDimFetch> $dimFetchStack
9411008
* @param non-empty-list<array{Type|null, ArrayDimFetch}> $offsetTypes

‎src/Analyser/MutatingScope.php‎

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2582,7 +2582,7 @@ public function assignVariable(string $variableName, Type $type, Type $nativeTyp
25822582
$scope->nativeExpressionTypes[$exprString] = new ExpressionTypeHolder($node, $nativeType, $certainty);
25832583
}
25842584

2585-
foreach ($scope->expressionTypes as $expressionType) {
2585+
foreach ($scope->expressionTypes as $exprString => $expressionType) {
25862586
if (!$expressionType->getExpr() instanceof IntertwinedVariableByReferenceWithExpr) {
25872587
continue;
25882588
}
@@ -2593,6 +2593,16 @@ public function assignVariable(string $variableName, Type $type, Type $nativeTyp
25932593
continue;
25942594
}
25952595

2596+
$assignedExpr = $expressionType->getExpr()->getAssignedExpr();
2597+
if (
2598+
$assignedExpr instanceof Expr\ArrayDimFetch
2599+
&& !$this->isDimFetchPathReachable($scope, $assignedExpr)
2600+
) {
2601+
unset($scope->expressionTypes[$exprString]);
2602+
unset($scope->nativeExpressionTypes[$exprString]);
2603+
continue;
2604+
}
2605+
25962606
$has = $scope->hasExpressionType($expressionType->getExpr()->getExpr());
25972607
if (
25982608
$expressionType->getExpr()->getExpr() instanceof Variable
@@ -2611,18 +2621,41 @@ public function assignVariable(string $variableName, Type $type, Type $nativeTyp
26112621
array_merge($intertwinedPropagatedFrom, [$variableName]),
26122622
);
26132623
} else {
2624+
$targetRootVar = $this->getIntertwinedRefRootVariableName($expressionType->getExpr()->getExpr());
2625+
if ($targetRootVar !== null && in_array($targetRootVar, $intertwinedPropagatedFrom, true)) {
2626+
continue;
2627+
}
26142628
$scope = $scope->assignExpression(
26152629
$expressionType->getExpr()->getExpr(),
26162630
$scope->getType($expressionType->getExpr()->getAssignedExpr()),
26172631
$scope->getNativeType($expressionType->getExpr()->getAssignedExpr()),
26182632
);
26192633
}
2620-
26212634
}
26222635

26232636
return $scope;
26242637
}
26252638

2639+
private function isDimFetchPathReachable(self $scope, Expr\ArrayDimFetch $dimFetch): bool
2640+
{
2641+
if ($dimFetch->dim === null) {
2642+
return false;
2643+
}
2644+
2645+
if (!$dimFetch->var instanceof Expr\ArrayDimFetch) {
2646+
return true;
2647+
}
2648+
2649+
$varType = $scope->getType($dimFetch->var);
2650+
$dimType = $scope->getType($dimFetch->dim);
2651+
2652+
if (!$varType->hasOffsetValueType($dimType)->yes()) {
2653+
return false;
2654+
}
2655+
2656+
return $this->isDimFetchPathReachable($scope, $dimFetch->var);
2657+
}
2658+
26262659
private function unsetExpression(Expr $expr): self
26272660
{
26282661
$scope = $this;
@@ -2833,12 +2866,6 @@ public function invalidateExpression(Expr $expressionToInvalidate, bool $require
28332866

28342867
foreach ($expressionTypes as $exprString => $exprTypeHolder) {
28352868
$exprExpr = $exprTypeHolder->getExpr();
2836-
if (
2837-
$exprExpr instanceof IntertwinedVariableByReferenceWithExpr
2838-
&& $exprExpr->isVariableToVariableReference()
2839-
) {
2840-
continue;
2841-
}
28422869
if (!$this->shouldInvalidateExpression($exprStringToInvalidate, $expressionToInvalidate, $exprExpr, $exprString, $requireMoreCharacters, $invalidatingClass)) {
28432870
continue;
28442871
}
@@ -2906,8 +2933,32 @@ public function invalidateExpression(Expr $expressionToInvalidate, bool $require
29062933
);
29072934
}
29082935

2936+
private function getIntertwinedRefRootVariableName(Expr $expr): ?string
2937+
{
2938+
if ($expr instanceof Variable && is_string($expr->name)) {
2939+
return $expr->name;
2940+
}
2941+
if ($expr instanceof Expr\ArrayDimFetch) {
2942+
return $this->getIntertwinedRefRootVariableName($expr->var);
2943+
}
2944+
return null;
2945+
}
2946+
29092947
private function shouldInvalidateExpression(string $exprStringToInvalidate, Expr $exprToInvalidate, Expr $expr, string $exprString, bool $requireMoreCharacters = false, ?ClassReflection $invalidatingClass = null): bool
29102948
{
2949+
if (
2950+
$expr instanceof IntertwinedVariableByReferenceWithExpr
2951+
&& $exprToInvalidate instanceof Variable
2952+
&& is_string($exprToInvalidate->name)
2953+
&& (
2954+
$expr->getVariableName() === $exprToInvalidate->name
2955+
|| $this->getIntertwinedRefRootVariableName($expr->getExpr()) === $exprToInvalidate->name
2956+
|| $this->getIntertwinedRefRootVariableName($expr->getAssignedExpr()) === $exprToInvalidate->name
2957+
)
2958+
) {
2959+
return false;
2960+
}
2961+
29112962
if ($requireMoreCharacters && $exprStringToInvalidate === $exprString) {
29122963
return false;
29132964
}

‎src/Node/Expr/IntertwinedVariableByReferenceWithExpr.php‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44

55
use Override;
66
use PhpParser\Node\Expr;
7-
use PhpParser\Node\Expr\Variable;
87
use PHPStan\Node\VirtualNode;
9-
use function is_string;
108

119
final class IntertwinedVariableByReferenceWithExpr extends Expr implements VirtualNode
1210
{
@@ -31,14 +29,6 @@ public function getAssignedExpr(): Expr
3129
return $this->assignedExpr;
3230
}
3331

34-
public function isVariableToVariableReference(): bool
35-
{
36-
return $this->expr instanceof Variable
37-
&& is_string($this->expr->name)
38-
&& $this->assignedExpr instanceof Variable
39-
&& is_string($this->assignedExpr->name);
40-
}
41-
4232
#[Override]
4333
public function getType(): string
4434
{

‎src/Type/Accessory/HasOffsetValueType.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,8 @@ public function isOffsetAccessLegal(): TrinaryLogic
156156

157157
public function hasOffsetValueType(Type $offsetType): TrinaryLogic
158158
{
159-
if ($offsetType->isConstantScalarValue()->yes() && $offsetType->equals($this->offsetType)) {
159+
$arrayKeyType = $offsetType->toArrayKey();
160+
if ($arrayKeyType->isConstantScalarValue()->yes() && $arrayKeyType->equals($this->offsetType)) {
160161
return TrinaryLogic::createYes();
161162
}
162163

@@ -165,7 +166,8 @@ public function hasOffsetValueType(Type $offsetType): TrinaryLogic
165166

166167
public function getOffsetValueType(Type $offsetType): Type
167168
{
168-
if ($offsetType->isConstantScalarValue()->yes() && $offsetType->equals($this->offsetType)) {
169+
$arrayKeyType = $offsetType->toArrayKey();
170+
if ($arrayKeyType->isConstantScalarValue()->yes() && $arrayKeyType->equals($this->offsetType)) {
169171
return $this->valueType;
170172
}
171173

0 commit comments

Comments
 (0)