Skip to content

Commit 1a672b3

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> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QW8VaDtn3cMdQowPSh1xGG
1 parent 184d34e commit 1a672b3

20 files changed

Lines changed: 805 additions & 175 deletions

‎src/Analyser/ExprHandler/ArrayDimFetchHandler.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515
use PHPStan\Analyser\ExpressionResultStorage;
1616
use PHPStan\Analyser\ExprHandler;
1717
use PHPStan\Analyser\ExprHandler\Helper\DefaultNarrowingHelper;
18+
use PHPStan\Analyser\ExprHandler\Helper\IllegalOffsetTypeHelper;
1819
use PHPStan\Analyser\ExprHandler\Helper\MethodCallReturnTypeHelper;
1920
use PHPStan\Analyser\ExprHandler\Helper\MethodThrowPointHelper;
2021
use PHPStan\Analyser\Generics\TemplateArgumentFrame;
22+
use PHPStan\Analyser\InternalThrowPoint;
2123
use PHPStan\Analyser\IssetabilityDescriptor;
2224
use PHPStan\Analyser\MutatingScope;
2325
use PHPStan\Analyser\NodeScopeResolver;
@@ -29,13 +31,15 @@
2931
use PHPStan\Dependency\Dependencies;
3032
use PHPStan\DependencyInjection\AutowiredService;
3133
use PHPStan\Node\Expr\TypeExpr;
34+
use PHPStan\Php\PhpVersion;
3235
use PHPStan\Reflection\ParametersAcceptorSelector;
3336
use PHPStan\Turbo\ShadowedByTurboExtension;
3437
use PHPStan\Type\ErrorType;
3538
use PHPStan\Type\NeverType;
3639
use PHPStan\Type\ObjectType;
3740
use PHPStan\Type\Type;
3841
use PHPStan\Type\TypeCombinator;
42+
use TypeError;
3943
use function array_merge;
4044
use function is_string;
4145

@@ -52,6 +56,7 @@ public function __construct(
5256
private DefaultNarrowingHelper $defaultNarrowingHelper,
5357
private MethodThrowPointHelper $methodThrowPointHelper,
5458
private MethodCallReturnTypeHelper $methodCallReturnTypeHelper,
59+
private PhpVersion $phpVersion,
5560
)
5661
{
5762
}
@@ -130,6 +135,16 @@ public function composeResult(NodeScopeResolver $nodeScopeResolver, Stmt $stmt,
130135
$offsetGetCall = new MethodCall($expr->var, new Identifier('offsetGet'), [new Arg($expr->dim)], [TemplateArgumentFrame::SYNTHETIC_SITE_ATTRIBUTE => true]);
131136
}
132137

138+
if (
139+
$this->phpVersion->throwsTypeErrorForIllegalOffsets()
140+
// only array and string offset reads throw TypeError for illegal offsets,
141+
// reads on null and other scalars emit a warning, ArrayAccess objects go through offsetGet()
142+
&& (!$varType->isArray()->no() || !$varType->isString()->no())
143+
&& IllegalOffsetTypeHelper::mayOffsetThrowTypeError($dimResult->getType())
144+
) {
145+
$throwPoints[] = InternalThrowPoint::createExplicit($scope, new ObjectType(TypeError::class), $expr, false);
146+
}
147+
133148
$result = $this->expressionResultFactory->create(
134149
$scope,
135150
beforeScope: $beforeScope,

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
use PHPStan\Analyser\ExprHandler;
3434
use PHPStan\Analyser\ExprHandler\Helper\DefaultNarrowingHelper;
3535
use PHPStan\Analyser\ExprHandler\Helper\IdenticalNarrowingHelper;
36+
use PHPStan\Analyser\ExprHandler\Helper\IllegalOffsetTypeHelper;
3637
use PHPStan\Analyser\ExprHandler\Helper\MethodThrowPointHelper;
3738
use PHPStan\Analyser\ExprHandler\Helper\NonNullabilityHelper;
3839
use PHPStan\Analyser\ExprHandler\Helper\VirtualExprResultHelper;
@@ -852,6 +853,25 @@ private function doPrepareTarget(
852853
$nodeScopeResolver->callNodeCallback($nodeCallback, $dimFetch, $enterExpressionAssign ? $callbackScope->enterExpressionAssign($dimFetch) : $callbackScope, $storage);
853854
}
854855

856+
if ($this->phpVersion->throwsTypeErrorForIllegalOffsets()) {
857+
foreach ($offsetTypes as $i => [$offsetType, $offsetDimFetch]) {
858+
// $arr[] = ... never throws for the offset
859+
if ($offsetType === null) {
860+
continue;
861+
}
862+
if (!IllegalOffsetTypeHelper::mayOffsetThrowTypeError($offsetType)) {
863+
continue;
864+
}
865+
// writes to ArrayAccess objects go through offsetSet() whose throw points applyWrite() models;
866+
// the container is the previous chain link - the root for the first dimension
867+
$containerResult = $i === 0 ? $varResult : $deferredDimFetchResults[$i - 1][1];
868+
if ((new ObjectType(ArrayAccess::class))->isSuperTypeOf($containerResult->getType())->yes()) {
869+
continue;
870+
}
871+
$throwPoints[] = InternalThrowPoint::createExplicit($scope, new ObjectType(TypeError::class), $offsetDimFetch, false);
872+
}
873+
}
874+
855875
if ($mode->issetSemanticsForRead()) {
856876
// `$lvalue ??= ...` reads the chain with isset() semantics. The root
857877
// and dimensions were just walked, so each chain link's read is

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Analyser\ExprHandler\Helper;
4+
5+
use PHPStan\Turbo\ShadowedByTurboExtension;
6+
use PHPStan\Type\Type;
7+
8+
#[ShadowedByTurboExtension(implementation: __DIR__ . '/../../../../turbo-ext/src/IllegalOffsetTypeHelper.cpp')]
9+
final class IllegalOffsetTypeHelper
10+
{
11+
12+
/**
13+
* On PHP 8.0+ using an array or an object as an array/string offset throws TypeError
14+
* (https://wiki.php.net/rfc/engine_warnings). Float, bool and null keys are coerced
15+
* with at most a deprecation, resource keys with a warning - no TypeError.
16+
*/
17+
public static function mayOffsetThrowTypeError(Type $offsetType): bool
18+
{
19+
return !$offsetType->isArray()->no() || !$offsetType->isObject()->no();
20+
}
21+
22+
}

‎src/Php/PhpVersion.php‎

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

191+
// see https://wiki.php.net/rfc/engine_warnings - "Illegal offset type" family
192+
public function throwsTypeErrorForIllegalOffsets(): bool
193+
{
194+
return $this->versionId >= 80000;
195+
}
196+
191197
public function supportsHhPrintfSpecifier(): bool
192198
{
193199
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)