Skip to content

Commit 8626ab5

Browse files
github-actions[bot]phpstan-bot
github-actions[bot]
authored and
phpstan-bot
committed
Do not assign ErrorType back to array when assign-op result is an error
- When an assign-op like `+=` on an array element produces ErrorType (e.g., `(bool|float|int|string) + int`), the ErrorType was assigned back to the array via `setExistingOffsetValueType`/`setOffsetValueType`. Since ErrorType extends MixedType, `TypeCombinator::union` absorbed the original value type, corrupting the array type to MixedType for ALL elements. - This caused subsequent assign-ops on different keys of the same array to see MixedType instead of the original type, suppressing error reports. - Fix: in `AssignHandler::processAssignVar()`, when the assign-op result is ErrorType, preserve the original element type by traversing the offset chain to recover it. - Applied fix in three locations: ArrayDimFetch branch, ExistingArrayDimFetch branch, and Variable branch. - Covers all assign-op types: `+=`, `-=`, `*=`, `/=`, `%=`, `.=`, `<<=`, `>>=`, `&=`, `|=`, `^=`, `**=`.
1 parent 04a99c1 commit 8626ab5

File tree

4 files changed

+261
-1
lines changed

4 files changed

+261
-1
lines changed

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,12 @@ public function processAssignVar(
241241
$assignedExpr = $this->unwrapAssign($assignedExpr);
242242
$type = $scopeBeforeAssignEval->getType($assignedExpr);
243243

244+
$nativeType = $scope->getNativeType($assignedExpr);
245+
if ($isAssignOp && $type instanceof ErrorType) {
246+
$type = $scopeBeforeAssignEval->getType($var);
247+
$nativeType = $scopeBeforeAssignEval->getNativeType($var);
248+
}
249+
244250
$conditionalExpressions = [];
245251
if ($assignedExpr instanceof Ternary) {
246252
$if = $assignedExpr->if;
@@ -314,7 +320,7 @@ public function processAssignVar(
314320
}
315321

316322
$nodeScopeResolver->callNodeCallback($nodeCallback, new VariableAssignNode($var, $assignedExpr), $scopeBeforeAssignEval, $storage);
317-
$scope = $scope->assignVariable($var->name, $type, $scope->getNativeType($assignedExpr), TrinaryLogic::createYes());
323+
$scope = $scope->assignVariable($var->name, $type, $nativeType, TrinaryLogic::createYes());
318324
foreach ($conditionalExpressions as $exprString => $holders) {
319325
$scope = $scope->addConditionalExpressions($exprString, $holders);
320326
}
@@ -414,6 +420,28 @@ public function processAssignVar(
414420

415421
$valueToWrite = $scope->getType($assignedExpr);
416422
$nativeValueToWrite = $scope->getNativeType($assignedExpr);
423+
424+
if ($isAssignOp && $valueToWrite instanceof ErrorType) {
425+
$originalType = $scope->getType($var);
426+
foreach ($offsetTypes as [$offsetType]) {
427+
if ($offsetType === null) {
428+
break;
429+
}
430+
$originalType = $originalType->getOffsetValueType($offsetType);
431+
}
432+
$valueToWrite = $originalType;
433+
}
434+
if ($isAssignOp && $nativeValueToWrite instanceof ErrorType) {
435+
$originalNativeType = $scope->getNativeType($var);
436+
foreach ($offsetNativeTypes as [$offsetNativeType]) {
437+
if ($offsetNativeType === null) {
438+
break;
439+
}
440+
$originalNativeType = $originalNativeType->getOffsetValueType($offsetNativeType);
441+
}
442+
$nativeValueToWrite = $originalNativeType;
443+
}
444+
417445
$scopeBeforeAssignEval = $scope;
418446

419447
// 3. eval assigned expr
@@ -781,6 +809,21 @@ public function processAssignVar(
781809
$varType = $scope->getType($var);
782810
$varNativeType = $scope->getNativeType($var);
783811

812+
if ($isAssignOp && $valueToWrite instanceof ErrorType) {
813+
$originalType = $varType;
814+
foreach ($offsetTypes as [$offsetType]) {
815+
$originalType = $originalType->getOffsetValueType($offsetType);
816+
}
817+
$valueToWrite = $originalType;
818+
}
819+
if ($isAssignOp && $nativeValueToWrite instanceof ErrorType) {
820+
$originalNativeType = $varNativeType;
821+
foreach ($offsetNativeTypes as [$offsetNativeType]) {
822+
$originalNativeType = $originalNativeType->getOffsetValueType($offsetNativeType);
823+
}
824+
$nativeValueToWrite = $originalNativeType;
825+
}
826+
784827
$offsetValueType = $varType;
785828
$offsetNativeValueType = $varNativeType;
786829
$offsetValueTypeStack = [$offsetValueType];

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug10349Nsrt;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class Foo
8+
{
9+
/**
10+
* @param array<string, array<string, bool|float|int|string>> $expected
11+
*/
12+
public function testTypePreservationAfterErrorAssignOp(array $expected, int $ptr, string $key): void
13+
{
14+
assertType('array<string, array<string, bool|float|int|string>>', $expected);
15+
16+
$expected[$key]['number-1'] += $ptr;
17+
18+
// After += with ErrorType result, the array type should be preserved
19+
assertType('bool|float|int|string', $expected[$key]['number-2']);
20+
}
21+
22+
/**
23+
* @param array<string, bool|float|int|string> $arr
24+
*/
25+
public function testSimpleArrayTypePreservation(array $arr, int $ptr): void
26+
{
27+
assertType('bool|float|int|string', $arr['a']);
28+
29+
$arr['a'] += $ptr;
30+
31+
// After += with ErrorType result, sibling keys should keep their type
32+
assertType('bool|float|int|string', $arr['b']);
33+
}
34+
}

‎tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -835,4 +835,86 @@ public function testBug14080(): void
835835
$this->analyse([__DIR__ . '/data/bug-14080.php'], []);
836836
}
837837

838+
public function testBug10349(): void
839+
{
840+
$this->analyse([__DIR__ . '/data/bug-10349.php'], [
841+
[
842+
'Binary operation "+=" between bool|float|int|string and int results in an error.',
843+
15,
844+
],
845+
[
846+
'Binary operation "+=" between bool|float|int|string and int results in an error.',
847+
20,
848+
],
849+
[
850+
'Binary operation "+=" between bool|float|int|string and int results in an error.',
851+
37,
852+
],
853+
[
854+
'Binary operation "+=" between bool|float|int|string and int results in an error.',
855+
47,
856+
],
857+
[
858+
'Binary operation "+=" between bool|float|int|string and int results in an error.',
859+
49,
860+
],
861+
[
862+
'Binary operation "+=" between bool|float|int|string and int results in an error.',
863+
57,
864+
],
865+
[
866+
'Binary operation "+=" between bool|float|int|string and int results in an error.',
867+
59,
868+
],
869+
[
870+
'Binary operation "-=" between bool|float|int|string and int results in an error.',
871+
67,
872+
],
873+
[
874+
'Binary operation "-=" between bool|float|int|string and int results in an error.',
875+
68,
876+
],
877+
[
878+
'Binary operation "*=" between bool|float|int|string and int results in an error.',
879+
69,
880+
],
881+
[
882+
'Binary operation "*=" between bool|float|int|string and int results in an error.',
883+
70,
884+
],
885+
[
886+
'Binary operation ".=" between array<int>|int and \'foo\' results in an error.',
887+
78,
888+
],
889+
[
890+
'Binary operation ".=" between array<int>|int and \'foo\' results in an error.',
891+
79,
892+
],
893+
[
894+
'Binary operation "/=" between bool|float|int|string and int results in an error.',
895+
87,
896+
],
897+
[
898+
'Binary operation "/=" between bool|float|int|string and int results in an error.',
899+
88,
900+
],
901+
[
902+
'Binary operation "%=" between bool|float|int|string and int results in an error.',
903+
89,
904+
],
905+
[
906+
'Binary operation "%=" between bool|float|int|string and int results in an error.',
907+
90,
908+
],
909+
[
910+
'Binary operation "<<=" between bool|float|int|string and int results in an error.',
911+
98,
912+
],
913+
[
914+
'Binary operation "<<=" between bool|float|int|string and int results in an error.',
915+
99,
916+
],
917+
]);
918+
}
919+
838920
}

‎tests/PHPStan/Rules/Operators/data/bug-10349.php‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug10349;
4+
5+
class Foo
6+
{
7+
/**
8+
* @param array<string, array<string, bool|float|int|string>> $expected
9+
*/
10+
public function issue1A(array $expected, int $ptr): void
11+
{
12+
foreach ($expected as $key => $param) {
13+
if ($param['number-1'] !== false) {
14+
// This gets flagged
15+
$expected[$key]['number-1'] += $ptr;
16+
}
17+
18+
if ($param['number-2'] !== false) {
19+
// This should also get flagged but doesn't
20+
$expected[$key]['number-2'] += $ptr;
21+
}
22+
}
23+
}
24+
25+
/**
26+
* @param array<string, array<string, bool|float|int|string>> $expected
27+
*/
28+
public function issue1B(array $expected, int $ptr): void
29+
{
30+
foreach ($expected as $key => $param) {
31+
if (is_int($expected[$key]['number-1'])) {
32+
$expected[$key]['number-1'] += $ptr;
33+
}
34+
35+
// Even after fixing the first, the second one still doesn't get flagged
36+
if ($param['number-2'] !== false) {
37+
$expected[$key]['number-2'] += $ptr;
38+
}
39+
}
40+
}
41+
42+
/**
43+
* @param array<string, array<string, bool|float|int|string>> $expected
44+
*/
45+
public function multipleOpsNoLoop(array $expected, int $ptr, string $key): void
46+
{
47+
$expected[$key]['number-1'] += $ptr;
48+
// After the first += corrupts the array type, this should still be flagged
49+
$expected[$key]['number-2'] += $ptr;
50+
}
51+
52+
/**
53+
* @param array<string, bool|float|int|string> $arr
54+
*/
55+
public function simpleArray(array $arr, int $ptr): void
56+
{
57+
$arr['a'] += $ptr;
58+
// After the first += corrupts the array type, this should still be flagged
59+
$arr['b'] += $ptr;
60+
}
61+
62+
/**
63+
* @param array<string, bool|float|int|string> $arr
64+
*/
65+
public function otherAssignOps(array $arr, int $ptr): void
66+
{
67+
$arr['a'] -= $ptr;
68+
$arr['b'] -= $ptr;
69+
$arr['c'] *= $ptr;
70+
$arr['d'] *= $ptr;
71+
}
72+
73+
/**
74+
* @param array<string, array<int>|int> $arr
75+
*/
76+
public function concatAssignOps(array $arr): void
77+
{
78+
$arr['a'] .= 'foo';
79+
$arr['b'] .= 'foo';
80+
}
81+
82+
/**
83+
* @param array<string, bool|float|int|string> $arr
84+
*/
85+
public function divAndModAssignOps(array $arr, int $ptr): void
86+
{
87+
$arr['a'] /= $ptr;
88+
$arr['b'] /= $ptr;
89+
$arr['c'] %= $ptr;
90+
$arr['d'] %= $ptr;
91+
}
92+
93+
/**
94+
* @param array<string, bool|float|int|string> $arr
95+
*/
96+
public function bitwiseAssignOps(array $arr, int $ptr): void
97+
{
98+
$arr['a'] <<= $ptr;
99+
$arr['b'] <<= $ptr;
100+
}
101+
}

0 commit comments

Comments
 (0)