Skip to content

Commit 84aeaa3

Browse files
Use before-scope for evaluating array_push/array_unshift/array_splice argument types
- When `array_unshift($this->arr, array_pop($this->arr))` was processed, `getArrayFunctionAppendingType` re-evaluated argument types using `$scope->getType()` on the post-args scope. By that point, the inner `array_pop` had already modified `$this->arr` to be possibly empty, causing `array_pop` to be re-evaluated as returning `T|null` instead of `T`. - Pre-compute argument types from `ExpressionResultStorage::findBeforeScope()` which gives the scope that existed when each argument was originally evaluated. Pass pre-computed PHPDoc and native types separately to `getArrayFunctionAppendingType`. - Apply the same fix to `array_splice` handler, which also re-evaluated offset, length, and replacement argument types in the post-args scope. - Probed sort/shuffle/arsort handlers: these only read the array argument itself (not other args from the scope), so they are not affected.
1 parent 5c70842 commit 84aeaa3

4 files changed

Lines changed: 102 additions & 4 deletions

File tree

src/Analyser/ExprHandler/FuncCallHandler.php

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -435,9 +435,22 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
435435
$arrayArgType = $scope->getType($arrayArg);
436436
$arrayArgNativeType = $scope->getNativeType($arrayArg);
437437

438-
$offsetType = $scope->getType($normalizedExpr->getArgs()[1]->value);
439-
$lengthType = isset($normalizedExpr->getArgs()[2]) ? $scope->getType($normalizedExpr->getArgs()[2]->value) : new NullType();
440-
$replacementType = isset($normalizedExpr->getArgs()[3]) ? $scope->getType($normalizedExpr->getArgs()[3]->value) : new ConstantArrayType([], []);
438+
$offsetType = $scopeBeforeArgs->getType($normalizedExpr->getArgs()[1]->value);
439+
440+
if (isset($normalizedExpr->getArgs()[2])) {
441+
$lengthType = $scopeBeforeArgs->getType($normalizedExpr->getArgs()[2]->value);
442+
} else {
443+
$lengthType = new NullType();
444+
}
445+
446+
if (isset($normalizedExpr->getArgs()[3])) {
447+
$replacementArg = $normalizedExpr->getArgs()[3]->value;
448+
$replacementType = $scopeBeforeArgs->getType($replacementArg);
449+
$replacementNativeType = $scopeBeforeArgs->getNativeType($replacementArg);
450+
} else {
451+
$replacementType = new ConstantArrayType([], []);
452+
$replacementNativeType = new ConstantArrayType([], []);
453+
}
441454

442455
$scope = $nodeScopeResolver->processVirtualAssign(
443456
$scope,
@@ -446,7 +459,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
446459
$arrayArg,
447460
new NativeTypeExpr(
448461
$arrayArgType->spliceArray($offsetType, $lengthType, $replacementType),
449-
$arrayArgNativeType->spliceArray($offsetType, $lengthType, $replacementType),
462+
$arrayArgNativeType->spliceArray($offsetType, $lengthType, $replacementNativeType),
450463
),
451464
$nodeCallback,
452465
)->getScope();

tests/PHPStan/Analyser/nsrt/bug-13510.php

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,56 @@ public function testTwoLines(array $arr): void
2121
array_unshift($arr, $popped);
2222
assertType('non-empty-list<int>', $arr);
2323
}
24+
}
25+
26+
class Bar
27+
{
28+
/** @var array<int> */
29+
public array $arr = [];
30+
31+
public function test(): void
32+
{
33+
if (count($this->arr) === 0) {
34+
throw new \Exception();
35+
}
36+
assertType('non-empty-array<int>', $this->arr);
37+
array_unshift($this->arr, array_pop($this->arr));
38+
assertType('non-empty-array<int>', $this->arr);
39+
}
40+
41+
public function testArrayPush(): void
42+
{
43+
if (count($this->arr) === 0) {
44+
throw new \Exception();
45+
}
46+
array_push($this->arr, array_pop($this->arr));
47+
assertType('non-empty-array<int>', $this->arr);
48+
}
2449

50+
public function testArrayUnshiftWithArrayShift(): void
51+
{
52+
if (count($this->arr) === 0) {
53+
throw new \Exception();
54+
}
55+
array_unshift($this->arr, array_shift($this->arr));
56+
assertType('non-empty-array<int>', $this->arr);
57+
}
58+
59+
public function testArrayPushWithArrayShift(): void
60+
{
61+
if (count($this->arr) === 0) {
62+
throw new \Exception();
63+
}
64+
array_push($this->arr, array_shift($this->arr));
65+
assertType('non-empty-array<int>', $this->arr);
66+
}
67+
68+
public function testArraySplice(): void
69+
{
70+
if (count($this->arr) === 0) {
71+
throw new \Exception();
72+
}
73+
array_splice($this->arr, 0, 0, [array_pop($this->arr)]);
74+
assertType('non-empty-array<(int<0, max>|string), int>', $this->arr);
75+
}
2576
}

tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,4 +1071,9 @@ public function testBug10749(): void
10711071
$this->analyse([__DIR__ . '/data/bug-10749.php'], []);
10721072
}
10731073

1074+
public function testBug13510(): void
1075+
{
1076+
$this->analyse([__DIR__ . '/data/bug-13510.php'], []);
1077+
}
1078+
10741079
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug13510;
4+
5+
class Foo {
6+
/** @var array<int> */
7+
public array $arr = [];
8+
9+
public function testArrayUnshift(): void {
10+
if (count($this->arr) === 0) {
11+
throw new \Exception('Narrow to non-empty-array');
12+
}
13+
array_unshift($this->arr, array_pop($this->arr));
14+
}
15+
16+
public function testArrayPush(): void {
17+
if (count($this->arr) === 0) {
18+
throw new \Exception('Narrow to non-empty-array');
19+
}
20+
array_push($this->arr, array_pop($this->arr));
21+
}
22+
23+
public function testArraySplice(): void {
24+
if (count($this->arr) === 0) {
25+
throw new \Exception('Narrow to non-empty-array');
26+
}
27+
array_splice($this->arr, 0, 0, [array_pop($this->arr)]);
28+
}
29+
}

0 commit comments

Comments
 (0)