Skip to content

Commit 2c0286f

Browse files
ondrejmirtesVincentLanglet
authored andcommitted
Consider ArrayAccess side effects for dead catch analysis
- Synthesize offsetGet throw points in ArrayDimFetch handler for ArrayAccess objects - Synthesize offsetUnset throw points in Unset_ handler for ArrayAccess objects - Synthesize offsetExists throw points in Isset_ handler for ArrayAccess objects - offsetSet was already handled; this brings the other three operations to parity - New regression test in tests/PHPStan/Rules/Exceptions/data/bug-11427.php
1 parent 925f29c commit 2c0286f

3 files changed

Lines changed: 132 additions & 0 deletions

File tree

src/Analyser/NodeScopeResolver.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2090,6 +2090,18 @@ private function processStmtNode(
20902090
$throwPoints = array_merge($throwPoints, $exprResult->getThrowPoints());
20912091
$impurePoints = array_merge($impurePoints, $exprResult->getImpurePoints());
20922092
if ($var instanceof ArrayDimFetch && $var->dim !== null) {
2093+
$varType = $scope->getType($var->var);
2094+
if (!$varType->isArray()->yes() && !(new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->no()) {
2095+
$throwPoints = array_merge($throwPoints, $this->processExprNode(
2096+
$stmt,
2097+
new MethodCall($this->deepNodeCloner->cloneNode($var->var), 'offsetUnset'),
2098+
$scope,
2099+
$storage,
2100+
new NoopNodeCallback(),
2101+
ExpressionContext::createDeep(),
2102+
)->getThrowPoints());
2103+
}
2104+
20932105
$clonedVar = $this->deepNodeCloner->cloneNode($var->var);
20942106
$traverser = new NodeTraverser();
20952107
$traverser->addVisitor(new class () extends NodeVisitorAbstract {
@@ -3612,6 +3624,18 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
36123624
$impurePoints = array_merge($impurePoints, $result->getImpurePoints());
36133625
$isAlwaysTerminating = $isAlwaysTerminating || $result->isAlwaysTerminating();
36143626
$scope = $result->getScope();
3627+
3628+
$varType = $scope->getType($expr->var);
3629+
if (!$varType->isArray()->yes() && !(new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->no()) {
3630+
$throwPoints = array_merge($throwPoints, $this->processExprNode(
3631+
$stmt,
3632+
new MethodCall($this->deepNodeCloner->cloneNode($expr->var), 'offsetGet'),
3633+
$scope,
3634+
$storage,
3635+
new NoopNodeCallback(),
3636+
ExpressionContext::createDeep(),
3637+
)->getThrowPoints());
3638+
}
36153639
} elseif ($expr instanceof Array_) {
36163640
$itemNodes = [];
36173641
$hasYield = false;
@@ -3909,6 +3933,24 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
39093933
$impurePoints = array_merge($impurePoints, $result->getImpurePoints());
39103934
$isAlwaysTerminating = $isAlwaysTerminating || $result->isAlwaysTerminating();
39113935
$nonNullabilityResults[] = $nonNullabilityResult;
3936+
3937+
if (!($var instanceof ArrayDimFetch)) {
3938+
continue;
3939+
}
3940+
3941+
$varType = $scope->getType($var->var);
3942+
if ($varType->isArray()->yes() || (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->no()) {
3943+
continue;
3944+
}
3945+
3946+
$throwPoints = array_merge($throwPoints, $this->processExprNode(
3947+
$stmt,
3948+
new MethodCall($this->deepNodeCloner->cloneNode($var->var), 'offsetExists'),
3949+
$scope,
3950+
$storage,
3951+
new NoopNodeCallback(),
3952+
ExpressionContext::createDeep(),
3953+
)->getThrowPoints());
39123954
}
39133955
foreach (array_reverse($expr->vars) as $var) {
39143956
$scope = $this->lookForUnsetAllowedUndefinedExpressions($scope, $var);

tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,11 @@ public function testBug9267(): void
614614
$this->analyse([__DIR__ . '/data/bug-9267.php'], []);
615615
}
616616

617+
public function testBug11427(): void
618+
{
619+
$this->analyse([__DIR__ . '/data/bug-11427.php'], []);
620+
}
621+
617622
#[RequiresPhp('>= 8.4')]
618623
public function testPropertyHooks(): void
619624
{
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
<?php
2+
3+
namespace Bug11427;
4+
5+
/** @implements \ArrayAccess<int, int> */
6+
class C implements \ArrayAccess {
7+
#[\ReturnTypeWillChange]
8+
public function offsetExists($offset) {
9+
throw new \Exception("exists");
10+
}
11+
12+
#[\ReturnTypeWillChange]
13+
public function offsetGet($offset) {
14+
throw new \Exception("get");
15+
}
16+
17+
#[\ReturnTypeWillChange]
18+
public function offsetSet($offset, $value) {
19+
throw new \Exception("set");
20+
}
21+
22+
#[\ReturnTypeWillChange]
23+
public function offsetUnset($offset) {
24+
throw new \Exception("unset");
25+
}
26+
}
27+
28+
function test(C $c): void {
29+
try {
30+
$x = isset($c[1]);
31+
} catch (\Exception $e) {
32+
// offsetExists can throw
33+
}
34+
35+
try {
36+
$x = $c[1];
37+
} catch (\Exception $e) {
38+
// offsetGet can throw
39+
}
40+
41+
try {
42+
$c[1] = 1;
43+
} catch (\Exception $e) {
44+
// offsetSet can throw
45+
}
46+
47+
try {
48+
unset($c[1]);
49+
} catch (\Exception $e) {
50+
// offsetUnset can throw
51+
}
52+
}
53+
54+
/**
55+
* Union type where isArray() returns maybe and isSuperTypeOf(ArrayAccess) returns maybe.
56+
* This ensures the conditions in NodeScopeResolver are tested with types
57+
* that distinguish !->yes() from ->no() and !->no() from ->yes().
58+
*
59+
* @param array<int, int>|C $c
60+
*/
61+
function testArrayOrArrayAccess($c): void {
62+
try {
63+
$x = isset($c[1]);
64+
} catch (\Exception $e) {
65+
// offsetExists can throw when $c is C
66+
}
67+
68+
try {
69+
$x = $c[1];
70+
} catch (\Exception $e) {
71+
// offsetGet can throw when $c is C
72+
}
73+
74+
try {
75+
$c[1] = 1;
76+
} catch (\Exception $e) {
77+
// offsetSet can throw when $c is C
78+
}
79+
80+
try {
81+
unset($c[1]);
82+
} catch (\Exception $e) {
83+
// offsetUnset can throw when $c is C
84+
}
85+
}

0 commit comments

Comments
 (0)