Skip to content

Commit ff2ce20

Browse files
staabmstaabm
authored
Fix "offset might not exist" false-positives when offset is a expression
1 parent 3346acb commit ff2ce20

File tree

16 files changed

+442
-23
lines changed

16 files changed

+442
-23
lines changed

‎src/Analyser/MutatingScope.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4308,7 +4308,14 @@ public function specifyExpressionType(Expr $expr, Type $type, Type $nativeType,
43084308
}
43094309

43104310
$scope = $this;
4311-
if ($expr instanceof Expr\ArrayDimFetch && $expr->dim !== null) {
4311+
if (
4312+
$expr instanceof Expr\ArrayDimFetch
4313+
&& $expr->dim !== null
4314+
&& !$expr->dim instanceof Expr\PreInc
4315+
&& !$expr->dim instanceof Expr\PreDec
4316+
&& !$expr->dim instanceof Expr\PostDec
4317+
&& !$expr->dim instanceof Expr\PostInc
4318+
) {
43124319
$dimType = $scope->getType($expr->dim)->toArrayKey();
43134320
if ($dimType->isInteger()->yes() || $dimType->isString()->yes()) {
43144321
$exprVarType = $scope->getType($expr->var);

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 55 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5706,10 +5706,10 @@ private function processAssignVar(
57065706
$offsetValueType = $varType;
57075707
$offsetNativeValueType = $varNativeType;
57085708

5709-
$valueToWrite = $this->produceArrayDimFetchAssignValueToWrite($dimFetchStack, $offsetTypes, $offsetValueType, $valueToWrite, $scope);
5709+
[$valueToWrite, $additionalExpressions] = $this->produceArrayDimFetchAssignValueToWrite($dimFetchStack, $offsetTypes, $offsetValueType, $valueToWrite, $scope);
57105710

57115711
if (!$offsetValueType->equals($offsetNativeValueType) || !$valueToWrite->equals($nativeValueToWrite)) {
5712-
$nativeValueToWrite = $this->produceArrayDimFetchAssignValueToWrite($dimFetchStack, $offsetNativeTypes, $offsetNativeValueType, $nativeValueToWrite, $scope);
5712+
[$nativeValueToWrite, $additionalNativeExpressions] = $this->produceArrayDimFetchAssignValueToWrite($dimFetchStack, $offsetNativeTypes, $offsetNativeValueType, $nativeValueToWrite, $scope);
57135713
} else {
57145714
$rewritten = false;
57155715
foreach ($offsetTypes as $i => $offsetType) {
@@ -5728,7 +5728,7 @@ private function processAssignVar(
57285728
continue;
57295729
}
57305730

5731-
$nativeValueToWrite = $this->produceArrayDimFetchAssignValueToWrite($dimFetchStack, $offsetNativeTypes, $offsetNativeValueType, $nativeValueToWrite, $scope);
5731+
[$nativeValueToWrite] = $this->produceArrayDimFetchAssignValueToWrite($dimFetchStack, $offsetNativeTypes, $offsetNativeValueType, $nativeValueToWrite, $scope);
57325732
$rewritten = true;
57335733
break;
57345734
}
@@ -5781,6 +5781,16 @@ private function processAssignVar(
57815781
}
57825782
}
57835783

5784+
foreach ($additionalExpressions as $k => $additionalExpression) {
5785+
[$expr, $type] = $additionalExpression;
5786+
$nativeType = $type;
5787+
if (isset($additionalNativeExpressions[$k])) {
5788+
[, $nativeType] = $additionalNativeExpressions[$k];
5789+
}
5790+
5791+
$scope = $scope->assignExpression($expr, $type, $nativeType);
5792+
}
5793+
57845794
if (!$varType->isArray()->yes() && !(new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->no()) {
57855795
$throwPoints = array_merge($throwPoints, $this->processExprNode(
57865796
$stmt,
@@ -6134,9 +6144,13 @@ private function isImplicitArrayCreation(array $dimFetchStack, Scope $scope): Tr
61346144
/**
61356145
* @param list<ArrayDimFetch> $dimFetchStack
61366146
* @param list<Type|null> $offsetTypes
6147+
*
6148+
* @return array{Type, list<array{Expr, Type}>}
61376149
*/
6138-
private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, array $offsetTypes, Type $offsetValueType, Type $valueToWrite, Scope $scope): Type
6150+
private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, array $offsetTypes, Type $offsetValueType, Type $valueToWrite, Scope $scope): array
61396151
{
6152+
$originalValueToWrite = $valueToWrite;
6153+
61406154
$offsetValueTypeStack = [$offsetValueType];
61416155
foreach (array_slice($offsetTypes, 0, -1) as $offsetType) {
61426156
if ($offsetType === null) {
@@ -6204,28 +6218,47 @@ private function produceArrayDimFetchAssignValueToWrite(array $dimFetchStack, ar
62046218
continue;
62056219
}
62066220

6207-
if ($scope->hasExpressionType($arrayDimFetch)->yes()) { // keep list for $list[$index] assignments
6221+
if (!$arrayDimFetch->dim instanceof BinaryOp\Plus) {
6222+
continue;
6223+
}
6224+
6225+
if ( // keep list for $list[$index + 1] assignments
6226+
$arrayDimFetch->dim->right instanceof Variable
6227+
&& $arrayDimFetch->dim->left instanceof Node\Scalar\Int_
6228+
&& $arrayDimFetch->dim->left->value === 1
6229+
&& $scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->right))->yes()
6230+
) {
62086231
$valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType());
6209-
} elseif ($arrayDimFetch->dim instanceof BinaryOp\Plus) {
6210-
if ( // keep list for $list[$index + 1] assignments
6211-
$arrayDimFetch->dim->right instanceof Variable
6212-
&& $arrayDimFetch->dim->left instanceof Node\Scalar\Int_
6213-
&& $arrayDimFetch->dim->left->value === 1
6214-
&& $scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->right))->yes()
6215-
) {
6216-
$valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType());
6217-
} elseif ( // keep list for $list[1 + $index] assignments
6218-
$arrayDimFetch->dim->left instanceof Variable
6219-
&& $arrayDimFetch->dim->right instanceof Node\Scalar\Int_
6220-
&& $arrayDimFetch->dim->right->value === 1
6221-
&& $scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->left))->yes()
6222-
) {
6223-
$valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType());
6224-
}
6232+
} elseif ( // keep list for $list[1 + $index] assignments
6233+
$arrayDimFetch->dim->left instanceof Variable
6234+
&& $arrayDimFetch->dim->right instanceof Node\Scalar\Int_
6235+
&& $arrayDimFetch->dim->right->value === 1
6236+
&& $scope->hasExpressionType(new ArrayDimFetch($arrayDimFetch->var, $arrayDimFetch->dim->left))->yes()
6237+
) {
6238+
$valueToWrite = TypeCombinator::intersect($valueToWrite, new AccessoryArrayListType());
6239+
}
6240+
}
6241+
6242+
$additionalExpressions = [];
6243+
$offsetValueType = $valueToWrite;
6244+
$lastDimKey = array_key_last($dimFetchStack);
6245+
foreach ($dimFetchStack as $key => $dimFetch) {
6246+
if ($dimFetch->dim === null) {
6247+
$additionalExpressions = [];
6248+
break;
62256249
}
6250+
6251+
if ($key === $lastDimKey) {
6252+
$offsetValueType = $originalValueToWrite;
6253+
} else {
6254+
$offsetType = $scope->getType($dimFetch->dim);
6255+
$offsetValueType = $offsetValueType->getOffsetValueType($offsetType);
6256+
}
6257+
6258+
$additionalExpressions[] = [$dimFetch, $offsetValueType];
62266259
}
62276260

6228-
return $valueToWrite;
6261+
return [$valueToWrite, $additionalExpressions];
62296262
}
62306263

62316264
private function unwrapAssign(Expr $expr): Expr

‎tests/PHPStan/Analyser/NodeScopeResolverTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ private static function findTestFiles(): iterable
9898
yield __DIR__ . '/../Rules/Generics/data/bug-3769.php';
9999
yield __DIR__ . '/../Rules/Generics/data/bug-6301.php';
100100
yield __DIR__ . '/../Rules/PhpDoc/data/bug-4643.php';
101+
yield __DIR__ . '/../Rules/Arrays/data/bug-13538.php';
101102

102103
if (PHP_VERSION_ID >= 80000) {
103104
yield __DIR__ . '/../Rules/Comparison/data/bug-4857.php';

‎tests/PHPStan/Analyser/nsrt/bug-13039.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace Bug13039;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
function doFoo() {
8+
/** @var list<array<string, mixed>> */
9+
$transactions = [];
10+
11+
assertType('list<array<string, mixed>>', $transactions);
12+
13+
foreach (array_keys($transactions) as $k) {
14+
$transactions[$k]['Shares'] = [];
15+
$transactions[$k]['Shares']['Projects'] = [];
16+
$transactions[$k]['Shares']['People'] = [];
17+
}
18+
19+
assertType('list<array<string, mixed>>', $transactions);
20+
}
21+

‎tests/PHPStan/Analyser/nsrt/bug-13214.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
namespace Bug13214;
4+
5+
use function PHPStan\Testing\assertType;
6+
use stdClass;
7+
8+
class HelloWorld
9+
{
10+
/**
11+
* @param ArrayAccess<int, ?object> $array
12+
*/
13+
public function sayHello(ArrayAccess $array): void
14+
{
15+
$child = new stdClass();
16+
17+
assert($array[1] === null);
18+
19+
assertType('null', $array[1]);
20+
21+
$array[1] = $child;
22+
23+
assertType(stdClass::class, $array[1]);
24+
}
25+
26+
/**
27+
* @param array<int, ?object> $array
28+
*/
29+
public function sayHelloArray(array $array): void
30+
{
31+
$child = new stdClass();
32+
33+
assert(($array[1] ?? null) === null);
34+
35+
assertType('object|null', $array[1]);
36+
37+
$array[1] = $child;
38+
39+
assertType(stdClass::class, $array[1]);
40+
}
41+
}

‎tests/PHPStan/Levels/data/arrayAccess.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,17 @@ public function doLorem(
4444
}
4545

4646
}
47+
48+
/**
49+
* @return mixed[]
50+
*/
51+
function bug12931():array {
52+
/** @var array<string, array<string, int>> $data */
53+
$data = [];
54+
$data['attr'] = [];
55+
$data['attr']['first'] = 1;
56+
$data['attr']['second'] = 2;
57+
$data['attr']['third'] = 3;
58+
59+
return $data;
60+
}

‎tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,4 +1007,28 @@ public function testBug12926(): void
10071007
$this->analyse([__DIR__ . '/data/bug-12926.php'], []);
10081008
}
10091009

1010+
public function testBug13538(): void
1011+
{
1012+
$this->reportPossiblyNonexistentConstantArrayOffset = true;
1013+
$this->reportPossiblyNonexistentGeneralArrayOffset = true;
1014+
1015+
$this->analyse([__DIR__ . '/data/bug-13538.php'], [
1016+
[
1017+
"Offset int might not exist on non-empty-array<int, ''>.",
1018+
13,
1019+
],
1020+
[
1021+
"Offset int might not exist on non-empty-array<int, ''>.",
1022+
17,
1023+
],
1024+
]);
1025+
}
1026+
1027+
public function testBug12805(): void
1028+
{
1029+
$this->reportPossiblyNonexistentGeneralArrayOffset = true;
1030+
1031+
$this->analyse([__DIR__ . '/data/bug-12805.php'], []);
1032+
}
1033+
10101034
}

‎tests/PHPStan/Rules/Arrays/data/bug-12805.php‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug12805;
4+
5+
/**
6+
* @param array<string, array{ rtx?: int }> $operations
7+
* @return array<string, array{ rtx: int }>
8+
*/
9+
function bug(array $operations): array {
10+
$base = [];
11+
12+
foreach ($operations as $operationName => $operation) {
13+
if (!isset($base[$operationName])) {
14+
$base[$operationName] = [];
15+
}
16+
if (!isset($base[$operationName]['rtx'])) {
17+
$base[$operationName]['rtx'] = 0;
18+
}
19+
$base[$operationName]['rtx'] += $operation['rtx'] ?? 0;
20+
}
21+
22+
return $base;
23+
}

‎tests/PHPStan/Rules/Arrays/data/bug-13538.php‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<?php
2+
3+
namespace Bug13538;
4+
5+
use LogicException;
6+
use function PHPStan\Testing\assertType;
7+
8+
/** @param list<string> $arr */
9+
function doFoo(array $arr, int $i, int $i2): void
10+
{
11+
$logs = [];
12+
$logs[$i] = '';
13+
echo $logs[$i2];
14+
15+
assertType("non-empty-array<int, ''>", $logs);
16+
assertType("''", $logs[$i]);
17+
assertType("''", $logs[$i2]); // could be mixed
18+
19+
foreach ($arr as $value) {
20+
echo $logs[$i];
21+
22+
assertType("non-empty-array<int, ''>", $logs);
23+
assertType("''", $logs[$i]);
24+
}
25+
}
26+
27+
/** @param list<string> $arr */
28+
function doFooBar(array $arr): void
29+
{
30+
if (!defined('LOG_DIR')) {
31+
throw new LogicException();
32+
}
33+
34+
$logs = [];
35+
$logs[LOG_DIR] = '';
36+
37+
assertType("non-empty-array<''>", $logs);
38+
assertType("''", $logs[LOG_DIR]);
39+
40+
foreach ($arr as $value) {
41+
echo $logs[LOG_DIR];
42+
43+
assertType("non-empty-array<''>", $logs);
44+
assertType("''", $logs[LOG_DIR]);
45+
}
46+
}
47+
48+
function doBar(array $arr, int $i, string $s): void
49+
{
50+
$logs = [];
51+
$logs[$i][$s] = '';
52+
assertType("non-empty-array<int, non-empty-array<string, ''>>", $logs);
53+
assertType("non-empty-array<string, ''>", $logs[$i]);
54+
assertType("''", $logs[$i][$s]);
55+
foreach ($arr as $value) {
56+
assertType("non-empty-array<int, non-empty-array<string, ''>>", $logs);
57+
assertType("non-empty-array<string, ''>", $logs[$i]);
58+
assertType("''", $logs[$i][$s]);
59+
echo $logs[$i][$s];
60+
}
61+
}

‎tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,4 +1121,10 @@ public function testBug7773(): void
11211121
$this->analyse([__DIR__ . '/data/bug-7773.php'], []);
11221122
}
11231123

1124+
public function testPr4375(): void
1125+
{
1126+
$this->treatPhpDocTypesAsCertain = true;
1127+
$this->analyse([__DIR__ . '/data/pr-4375.php'], []);
1128+
}
1129+
11241130
}

0 commit comments

Comments
 (0)