Skip to content

Commit 2a99875

Browse files
VincentLangletphpstan-bot
authored andcommitted
Fix false positive with array_key_exists after by-reference assignment from ArrayDimFetch
- When creating a reference from an ArrayDimFetch (e.g. $ref = &$arr['key']), set up IntertwinedVariableByReferenceWithExpr so writes through $ref propagate back to $arr - Added propagateRefTypeToArrayDimFetch() to handle constant-dim ArrayDimFetch targets using setOffsetValueType instead of HasOffsetValueType intersection which would produce never type for incompatible constant array shapes - New regression test in tests/PHPStan/Rules/Comparison/data/bug-14449.php
1 parent 7092778 commit 2a99875

4 files changed

Lines changed: 164 additions & 5 deletions

File tree

src/Analyser/ExprHandler/AssignHandler.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,31 @@ static function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $contex
155155
);
156156
$scope = $result->getScope();
157157

158+
if (
159+
$expr instanceof AssignRef
160+
&& $expr->var instanceof Variable
161+
&& is_string($expr->var->name)
162+
&& $expr->expr instanceof ArrayDimFetch
163+
) {
164+
$rootExpr = $expr->expr;
165+
while ($rootExpr instanceof ArrayDimFetch) {
166+
$rootExpr = $rootExpr->var;
167+
}
168+
169+
if ($rootExpr instanceof Variable && is_string($rootExpr->name)) {
170+
$varName = $expr->var->name;
171+
$type = $scope->getType($expr->var);
172+
$nativeType = $scope->getNativeType($expr->var);
173+
174+
// When $varName is assigned, update the ArrayDimFetch expression
175+
$scope = $scope->assignExpression(
176+
new IntertwinedVariableByReferenceWithExpr($varName, $expr->expr, new Variable($varName)),
177+
$type,
178+
$nativeType,
179+
);
180+
}
181+
}
182+
158183
if (
159184
$expr instanceof AssignRef
160185
&& $expr->var instanceof Variable

src/Analyser/MutatingScope.php

Lines changed: 97 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@
109109
use function array_map;
110110
use function array_merge;
111111
use function array_pop;
112+
use function array_reverse;
112113
use function array_slice;
113114
use function array_unique;
114115
use function array_values;
@@ -2623,17 +2624,108 @@ public function assignVariable(string $variableName, Type $type, Type $nativeTyp
26232624
if ($targetRootVar !== null && in_array($targetRootVar, $intertwinedPropagatedFrom, true)) {
26242625
continue;
26252626
}
2626-
$scope = $scope->assignExpression(
2627-
$expressionType->getExpr()->getExpr(),
2628-
$scope->getType($expressionType->getExpr()->getAssignedExpr()),
2629-
$scope->getNativeType($expressionType->getExpr()->getAssignedExpr()),
2630-
);
2627+
$targetExpr = $expressionType->getExpr()->getExpr();
2628+
$newType = $scope->getType($expressionType->getExpr()->getAssignedExpr());
2629+
$newNativeType = $scope->getNativeType($expressionType->getExpr()->getAssignedExpr());
2630+
2631+
if ($targetExpr instanceof Expr\ArrayDimFetch && $targetRootVar !== null && $this->hasConstantDimInChain($targetExpr)) {
2632+
$scope = $this->propagateRefTypeToArrayDimFetch($scope, $targetExpr, $newType, $newNativeType, $intertwinedPropagatedFrom, $variableName);
2633+
} else {
2634+
$scope = $scope->assignExpression(
2635+
$targetExpr,
2636+
$newType,
2637+
$newNativeType,
2638+
);
2639+
}
26312640
}
26322641
}
26332642

26342643
return $scope;
26352644
}
26362645

2646+
private function hasConstantDimInChain(Expr\ArrayDimFetch $dimFetch): bool
2647+
{
2648+
$current = $dimFetch;
2649+
while ($current instanceof Expr\ArrayDimFetch) {
2650+
if ($current->dim !== null) {
2651+
$dimType = $this->getType($current->dim)->toArrayKey();
2652+
if ($dimType->isConstantScalarValue()->yes()) {
2653+
return true;
2654+
}
2655+
}
2656+
$current = $current->var;
2657+
}
2658+
2659+
return false;
2660+
}
2661+
2662+
/**
2663+
* @param list<string> $intertwinedPropagatedFrom
2664+
*/
2665+
private function propagateRefTypeToArrayDimFetch(
2666+
self $scope,
2667+
Expr\ArrayDimFetch $targetExpr,
2668+
Type $newType,
2669+
Type $newNativeType,
2670+
array $intertwinedPropagatedFrom,
2671+
string $variableName,
2672+
): self
2673+
{
2674+
// Collect the chain of ArrayDimFetch from leaf to root
2675+
$dimStack = [];
2676+
$current = $targetExpr;
2677+
while ($current instanceof Expr\ArrayDimFetch) {
2678+
$dimStack[] = $current->dim;
2679+
$current = $current->var;
2680+
}
2681+
2682+
// Build intermediate types from root to leaf
2683+
$dimStack = array_reverse($dimStack);
2684+
$rootType = $scope->getType($current);
2685+
$rootNativeType = $scope->getNativeType($current);
2686+
2687+
$intermediateTypes = [$rootType];
2688+
$intermediateNativeTypes = [$rootNativeType];
2689+
$currentType = $rootType;
2690+
$currentNativeType = $rootNativeType;
2691+
for ($i = 0; $i < count($dimStack) - 1; $i++) {
2692+
$dim = $dimStack[$i];
2693+
if ($dim === null) {
2694+
return $scope->assignExpression($targetExpr, $newType, $newNativeType);
2695+
}
2696+
$dimType = $scope->getType($dim)->toArrayKey();
2697+
$currentType = $currentType->getOffsetValueType($dimType);
2698+
$currentNativeType = $currentNativeType->getOffsetValueType($dimType);
2699+
$intermediateTypes[] = $currentType;
2700+
$intermediateNativeTypes[] = $currentNativeType;
2701+
}
2702+
2703+
// Build up from the leaf using setOffsetValueType
2704+
$builtType = $newType;
2705+
$builtNativeType = $newNativeType;
2706+
for ($i = count($dimStack) - 1; $i >= 0; $i--) {
2707+
$dim = $dimStack[$i];
2708+
if ($dim === null) {
2709+
return $scope->assignExpression($targetExpr, $newType, $newNativeType);
2710+
}
2711+
$dimType = $scope->getType($dim)->toArrayKey();
2712+
$builtType = $intermediateTypes[$i]->setOffsetValueType($dimType, $builtType);
2713+
$builtNativeType = $intermediateNativeTypes[$i]->setOffsetValueType($dimType, $builtNativeType);
2714+
}
2715+
2716+
if ($current instanceof Variable && is_string($current->name)) {
2717+
return $scope->assignVariable(
2718+
$current->name,
2719+
$builtType,
2720+
$builtNativeType,
2721+
TrinaryLogic::createYes(),
2722+
array_merge($intertwinedPropagatedFrom, [$variableName]),
2723+
);
2724+
}
2725+
2726+
return $scope->assignExpression($targetExpr, $newType, $newNativeType);
2727+
}
2728+
26372729
private function isDimFetchPathReachable(self $scope, Expr\ArrayDimFetch $dimFetch): bool
26382730
{
26392731
if ($dimFetch->dim === null) {

tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,4 +1226,10 @@ public function testBug12063(): void
12261226
$this->analyse([__DIR__ . '/data/bug-12063.php'], []);
12271227
}
12281228

1229+
public function testBug14449(): void
1230+
{
1231+
$this->treatPhpDocTypesAsCertain = true;
1232+
$this->analyse([__DIR__ . '/data/bug-14449.php'], []);
1233+
}
1234+
12291235
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug14449;
4+
5+
/**
6+
* @return \Generator<int,array{'id':int}>
7+
*/
8+
function DataGenerator() : \Generator
9+
{
10+
yield [ 'id' => 1 ] ;
11+
yield [ 'id' => 1 ] ;
12+
}
13+
14+
function () : void {
15+
$results = [];
16+
17+
$generator = DataGenerator();
18+
foreach ( $generator as $data )
19+
{
20+
$id = $data['id'];
21+
if ( !array_key_exists($id, $results ) )
22+
{
23+
$results[$id] = [];
24+
}
25+
if ( !array_key_exists('data',$results[$id]) )
26+
{
27+
$results[$id]['data'] = [];
28+
}
29+
30+
$resultData = &$results[$id]['data'];
31+
if ( !array_key_exists('id', $results[$id]['data']) )
32+
{
33+
$resultData['id'] = $id;
34+
}
35+
}
36+
};

0 commit comments

Comments
 (0)