Skip to content

Commit b0e6316

Browse files
gnutixclaude
gnutix
and
claude
committed
Model TypeError throw points for illegal array key offsets
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 411daa4 commit b0e6316

12 files changed

Lines changed: 410 additions & 115 deletions

File tree

‎src/Analyser/ExprHandler/ArrayDimFetchHandler.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
use PHPStan\Analyser\ExpressionResult;
1414
use PHPStan\Analyser\ExpressionResultStorage;
1515
use PHPStan\Analyser\ExprHandler;
16+
use PHPStan\Analyser\ExprHandler\Helper\IllegalOffsetTypeHelper;
1617
use PHPStan\Analyser\ExprHandler\Helper\NullsafeShortCircuitingHelper;
18+
use PHPStan\Analyser\InternalThrowPoint;
1719
use PHPStan\Analyser\MutatingScope;
1820
use PHPStan\Analyser\NodeScopeResolver;
1921
use PHPStan\Analyser\NoopNodeCallback;
@@ -23,9 +25,11 @@
2325
use PHPStan\Analyser\TypeSpecifierContext;
2426
use PHPStan\DependencyInjection\AutowiredService;
2527
use PHPStan\Node\Expr\TypeExpr;
28+
use PHPStan\Php\PhpVersion;
2629
use PHPStan\Type\NeverType;
2730
use PHPStan\Type\ObjectType;
2831
use PHPStan\Type\Type;
32+
use TypeError;
2933
use function array_merge;
3034

3135
/**
@@ -35,6 +39,10 @@
3539
final class ArrayDimFetchHandler implements ExprHandler
3640
{
3741

42+
public function __construct(private PhpVersion $phpVersion)
43+
{
44+
}
45+
3846
public function supports(Expr $expr): bool
3947
{
4048
return $expr instanceof ArrayDimFetch;
@@ -109,6 +117,16 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
109117
)->getThrowPoints());
110118
}
111119

120+
if (
121+
$this->phpVersion->throwsTypeErrorForIllegalOffsets()
122+
// only array and string offset reads throw TypeError for illegal offsets,
123+
// reads on null and other scalars emit a warning, ArrayAccess objects go through offsetGet()
124+
&& (!$varType->isArray()->no() || !$varType->isString()->no())
125+
&& IllegalOffsetTypeHelper::mayOffsetThrowTypeError($scope->getType($expr->dim))
126+
) {
127+
$throwPoints[] = InternalThrowPoint::createExplicit($scope, new ObjectType(TypeError::class), $expr, false);
128+
}
129+
112130
return new ExpressionResult(
113131
$scope,
114132
hasYield: $dimResult->hasYield() || $varResult->hasYield(),

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
use PHPStan\Analyser\ExpressionResultStorage;
2929
use PHPStan\Analyser\ExpressionTypeHolder;
3030
use PHPStan\Analyser\ExprHandler;
31+
use PHPStan\Analyser\ExprHandler\Helper\IllegalOffsetTypeHelper;
3132
use PHPStan\Analyser\ImpurePoint;
3233
use PHPStan\Analyser\InternalThrowPoint;
3334
use PHPStan\Analyser\MutatingScope;
@@ -608,6 +609,24 @@ public function processAssignVar(
608609
}
609610
}
610611

612+
if ($this->phpVersion->throwsTypeErrorForIllegalOffsets()) {
613+
foreach ($offsetTypes as [$offsetType, $offsetDimFetch]) {
614+
// $arr[] = ... never throws for the offset
615+
if ($offsetType === null) {
616+
continue;
617+
}
618+
if (!IllegalOffsetTypeHelper::mayOffsetThrowTypeError($offsetType)) {
619+
continue;
620+
}
621+
// writes to ArrayAccess objects go through offsetSet() whose throw points are modelled below
622+
$containerType = $scope->getType($offsetDimFetch->var);
623+
if ((new ObjectType(ArrayAccess::class))->isSuperTypeOf($containerType)->yes()) {
624+
continue;
625+
}
626+
$throwPoints[] = InternalThrowPoint::createExplicit($scope, new ObjectType(TypeError::class), $offsetDimFetch, false);
627+
}
628+
}
629+
611630
$valueToWrite = $scope->getType($assignedExpr);
612631
$nativeValueToWrite = $scope->getNativeType($assignedExpr);
613632
$scopeBeforeAssignEval = $scope;

‎src/Analyser/ExprHandler/Helper/IllegalOffsetTypeHelper.php‎

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 PHPStan\Analyser\ExprHandler\Helper;
4+
5+
use PHPStan\Type\ArrayType;
6+
use PHPStan\Type\MixedType;
7+
use PHPStan\Type\ObjectWithoutClassType;
8+
use PHPStan\Type\Type;
9+
use PHPStan\Type\UnionType;
10+
11+
final class IllegalOffsetTypeHelper
12+
{
13+
14+
/**
15+
* On PHP 8.0+ using an array or an object as an array/string offset throws TypeError
16+
* (https://wiki.php.net/rfc/engine_warnings). Float, bool and null keys are coerced
17+
* with at most a deprecation, resource keys with a warning - no TypeError.
18+
*/
19+
public static function mayOffsetThrowTypeError(Type $offsetType): bool
20+
{
21+
$illegalOffsetType = new UnionType([
22+
new ArrayType(new MixedType(), new MixedType()),
23+
new ObjectWithoutClassType(),
24+
]);
25+
26+
return !$illegalOffsetType->isSuperTypeOf($offsetType)->no();
27+
}
28+
29+
}

‎src/Php/PhpVersion.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,12 @@ public function throwsValueErrorForInternalFunctions(): bool
186186
return $this->versionId >= 80000;
187187
}
188188

189+
// see https://wiki.php.net/rfc/engine_warnings - "Illegal offset type" family
190+
public function throwsTypeErrorForIllegalOffsets(): bool
191+
{
192+
return $this->versionId >= 80000;
193+
}
194+
189195
public function supportsHhPrintfSpecifier(): bool
190196
{
191197
return $this->versionId >= 80000;

‎tests/PHPStan/Analyser/nsrt/throw-points/array-dim-fetch.php‎

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

55
use PHPStan\TrinaryLogic;
66
use function PHPStan\Testing\assertVariableCertainty;
7-
use function ThrowPoints\Helpers\doesntThrow;
87
use function ThrowPoints\Helpers\maybeThrows;
98

10-
function () {
11-
try {
12-
[][doesntThrow()];
13-
$foo = 1;
14-
} finally {
15-
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
16-
}
17-
};
18-
199
function () {
2010
try {
2111
[][maybeThrows()];

‎tests/PHPStan/Analyser/nsrt/throw-points/assign-op.php‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,6 @@ function () {
4141
}
4242
};
4343

44-
function () {
45-
try {
46-
$foo[doesntThrow()] .= 0;
47-
} finally {
48-
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
49-
}
50-
};
51-
5244
function () {
5345
try {
5446
$foo[maybeThrows()] .= 0;

‎tests/PHPStan/Analyser/nsrt/throw-points/assign.php‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,6 @@ function () {
4949
}
5050
};
5151

52-
function () {
53-
try {
54-
$foo[doesntThrow()] = 0;
55-
} finally {
56-
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
57-
}
58-
};
59-
6052
function () {
6153
try {
6254
$foo[maybeThrows()] = 0;
@@ -153,14 +145,6 @@ function () {
153145
}
154146
};
155147

156-
function () {
157-
try {
158-
[$foo[doesntThrow()]] = 1;
159-
} finally {
160-
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
161-
}
162-
};
163-
164148
function () {
165149
try {
166150
[$foo[maybeThrows()]] = 1;

‎tests/PHPStan/Analyser/nsrt/throw-points/illegal-array-offset-php7.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php // lint < 8.0
2+
3+
namespace ThrowPoints\IllegalArrayOffset;
4+
5+
use PHPStan\TrinaryLogic;
6+
use function PHPStan\Testing\assertVariableCertainty;
7+
use function ThrowPoints\Helpers\doesntThrow;
8+
9+
// Before PHP 8.0 an illegal (array/object) offset does not throw TypeError, so a
10+
// non-throwing offset expression introduces no throw point and $foo is always assigned.
11+
12+
function () {
13+
try {
14+
$foo[doesntThrow()] = 0;
15+
} finally {
16+
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
17+
}
18+
};
19+
20+
function () {
21+
try {
22+
[$foo[doesntThrow()]] = 1;
23+
} finally {
24+
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
25+
}
26+
};
27+
28+
function () {
29+
try {
30+
$foo[doesntThrow()] .= 0;
31+
} finally {
32+
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
33+
}
34+
};
35+
36+
function () {
37+
try {
38+
[][doesntThrow()];
39+
$foo = 1;
40+
} finally {
41+
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
42+
}
43+
};

‎tests/PHPStan/Analyser/nsrt/throw-points/illegal-array-offset-php8.php‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php // lint >= 8.0
2+
3+
namespace ThrowPoints\IllegalArrayOffset;
4+
5+
use PHPStan\TrinaryLogic;
6+
use function PHPStan\Testing\assertVariableCertainty;
7+
use function ThrowPoints\Helpers\doesntThrow;
8+
9+
// doesntThrow() never throws by itself, but on PHP 8.0+ its mixed return value used
10+
// as an array/string offset may be an array or an object, which throws TypeError.
11+
// That TypeError is the only throw point here, so $foo may be left unassigned.
12+
13+
function () {
14+
try {
15+
$foo[doesntThrow()] = 0;
16+
} finally {
17+
assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
18+
}
19+
};
20+
21+
function () {
22+
try {
23+
[$foo[doesntThrow()]] = 1;
24+
} finally {
25+
assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
26+
}
27+
};
28+
29+
function () {
30+
try {
31+
$foo[doesntThrow()] .= 0;
32+
} finally {
33+
assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
34+
}
35+
};
36+
37+
function () {
38+
try {
39+
[][doesntThrow()];
40+
$foo = 1;
41+
} finally {
42+
assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
43+
}
44+
};

‎tests/PHPStan/Levels/data/arrayOffsetAccess-4.json‎

Lines changed: 1 addition & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,6 @@
1414
"line": 18,
1515
"ignorable": true
1616
},
17-
{
18-
"message": "Expression \"$a[$objectOrInt]\" on a separate line does not do anything.",
19-
"line": 19,
20-
"ignorable": true
21-
},
22-
{
23-
"message": "Expression \"$a[$objectOrNull]\" on a separate line does not do anything.",
24-
"line": 20,
25-
"ignorable": true
26-
},
27-
{
28-
"message": "Expression \"$a[$explicitlyMixed]\" on a separate line does not do anything.",
29-
"line": 21,
30-
"ignorable": true
31-
},
32-
{
33-
"message": "Expression \"$a[$implicitlyMixed]\" on a separate line does not do anything.",
34-
"line": 22,
35-
"ignorable": true
36-
},
3717
{
3818
"message": "Expression \"$arrayOrObject[42]\" on a separate line does not do anything.",
3919
"line": 24,
@@ -49,26 +29,6 @@
4929
"line": 27,
5030
"ignorable": true
5131
},
52-
{
53-
"message": "Expression \"$arrayOrObject[$objectOrInt]\" on a separate line does not do anything.",
54-
"line": 28,
55-
"ignorable": true
56-
},
57-
{
58-
"message": "Expression \"$arrayOrObject[$objectOrNull]\" on a separate line does not do anything.",
59-
"line": 29,
60-
"ignorable": true
61-
},
62-
{
63-
"message": "Expression \"$arrayOrObject[$explicitlyMixed]\" on a separate line does not do anything.",
64-
"line": 30,
65-
"ignorable": true
66-
},
67-
{
68-
"message": "Expression \"$arrayOrObject[$implicitlyMixed]\" on a separate line does not do anything.",
69-
"line": 31,
70-
"ignorable": true
71-
},
7232
{
7333
"message": "Expression \"$explicitlyMixed[42]\" on a separate line does not do anything.",
7434
"line": 33,
@@ -84,26 +44,6 @@
8444
"line": 36,
8545
"ignorable": true
8646
},
87-
{
88-
"message": "Expression \"$explicitlyMixed[$objectOrInt]\" on a separate line does not do anything.",
89-
"line": 37,
90-
"ignorable": true
91-
},
92-
{
93-
"message": "Expression \"$explicitlyMixed[$objectOrNull]\" on a separate line does not do anything.",
94-
"line": 38,
95-
"ignorable": true
96-
},
97-
{
98-
"message": "Expression \"$explicitlyMixed[$explicitlyMixed]\" on a separate line does not do anything.",
99-
"line": 39,
100-
"ignorable": true
101-
},
102-
{
103-
"message": "Expression \"$explicitlyMixed[$implicitlyMixed]\" on a separate line does not do anything.",
104-
"line": 40,
105-
"ignorable": true
106-
},
10747
{
10848
"message": "Expression \"$implicitlyMixed[42]\" on a separate line does not do anything.",
10949
"line": 42,
@@ -118,25 +58,5 @@
11858
"message": "Expression \"$implicitlyMixed[$intOrNull]\" on a separate line does not do anything.",
11959
"line": 45,
12060
"ignorable": true
121-
},
122-
{
123-
"message": "Expression \"$implicitlyMixed[$objectOrInt]\" on a separate line does not do anything.",
124-
"line": 46,
125-
"ignorable": true
126-
},
127-
{
128-
"message": "Expression \"$implicitlyMixed[$objectOrNull]\" on a separate line does not do anything.",
129-
"line": 47,
130-
"ignorable": true
131-
},
132-
{
133-
"message": "Expression \"$implicitlyMixed[$explicitlyMixed]\" on a separate line does not do anything.",
134-
"line": 48,
135-
"ignorable": true
136-
},
137-
{
138-
"message": "Expression \"$implicitlyMixed[$implicitlyMixed]\" on a separate line does not do anything.",
139-
"line": 49,
140-
"ignorable": true
14161
}
142-
]
62+
]

0 commit comments

Comments
 (0)