diff --git a/CLAUDE.md b/CLAUDE.md index e33c66a1c2..f09cbdd321 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -344,6 +344,10 @@ PHPStan tracks whether expressions/statements have side effects ("impure points" Bugs occur when impure points are missed (e.g. inherited constructors of anonymous classes) or when `clearstatcache()` calls don't invalidate filesystem function return types. +### ArrayAccess throw point synthesis in NodeScopeResolver + +When PHP code uses array syntax (`$x[1]`) on an `ArrayAccess` object, the corresponding `offsetGet`/`offsetSet`/`offsetExists`/`offsetUnset` methods are called implicitly. For dead-catch detection to work, `NodeScopeResolver` must synthesize `MethodCall` nodes for these implicit calls and collect their throw points. The pattern: check `!$varType->isArray()->yes() && !(new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->no()`, then `processExprNode` a synthetic `new MethodCall($var, 'offsetXxx')` with `NoopNodeCallback` to extract throw points. This is done in four places: `ArrayDimFetch` handler (offsetGet), assignment to `ArrayDimFetch` (offsetSet), `Unset_` handler (offsetUnset), and `Isset_` handler (offsetExists). + ### FunctionCallParametersCheck: by-reference argument validation `FunctionCallParametersCheck` (`src/Rules/FunctionCallParametersCheck.php`) validates arguments passed to functions/methods. For by-reference parameters, it checks whether the argument is a valid lvalue (variable, array dim fetch, property fetch). It also allows function/method calls that return by reference (`&getString()`, `&staticGetString()`, `&refFunction()`), using `returnsByReference()` on the resolved reflection. The class is manually instantiated in ~20 test files, so adding a constructor parameter requires updating all of them. The `Scope` interface provides `getMethodReflection()` for method calls, while `ReflectionProvider` (injected into the class) is needed for resolving function reflections. diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 8a807ea745..963225c9ef 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -2080,6 +2080,20 @@ private function processStmtNode( $throwPoints = array_merge($throwPoints, $exprResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $exprResult->getImpurePoints()); if ($var instanceof ArrayDimFetch && $var->dim !== null) { + $varType = $scope->getType($var->var); + /** @infection-ignore-all */ + $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); + if (!$varType->isArray()->yes() && !$isArrayAccess->no()) { + $throwPoints = array_merge($throwPoints, $this->processExprNode( + $stmt, + new MethodCall($this->deepNodeCloner->cloneNode($var->var), 'offsetUnset'), + $scope, + $storage, + new NoopNodeCallback(), + ExpressionContext::createDeep(), + )->getThrowPoints()); + } + $clonedVar = $this->deepNodeCloner->cloneNode($var->var); $traverser = new NodeTraverser(); $traverser->addVisitor(new class () extends NodeVisitorAbstract { @@ -3602,6 +3616,20 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto $impurePoints = array_merge($impurePoints, $result->getImpurePoints()); $isAlwaysTerminating = $isAlwaysTerminating || $result->isAlwaysTerminating(); $scope = $result->getScope(); + + $varType = $scope->getType($expr->var); + /** @infection-ignore-all */ + $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); + if (!$varType->isArray()->yes() && !$isArrayAccess->no()) { + $throwPoints = array_merge($throwPoints, $this->processExprNode( + $stmt, + new MethodCall($this->deepNodeCloner->cloneNode($expr->var), 'offsetGet'), + $scope, + $storage, + new NoopNodeCallback(), + ExpressionContext::createDeep(), + )->getThrowPoints()); + } } elseif ($expr instanceof Array_) { $itemNodes = []; $hasYield = false; @@ -3897,6 +3925,26 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto $impurePoints = array_merge($impurePoints, $result->getImpurePoints()); $isAlwaysTerminating = $isAlwaysTerminating || $result->isAlwaysTerminating(); $nonNullabilityResults[] = $nonNullabilityResult; + + if (!($var instanceof ArrayDimFetch)) { + continue; + } + + $varType = $scope->getType($var->var); + /** @infection-ignore-all */ + $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); + if ($varType->isArray()->yes() || $isArrayAccess->no()) { + continue; + } + + $throwPoints = array_merge($throwPoints, $this->processExprNode( + $stmt, + new MethodCall($this->deepNodeCloner->cloneNode($var->var), 'offsetExists'), + $scope, + $storage, + new NoopNodeCallback(), + ExpressionContext::createDeep(), + )->getThrowPoints()); } foreach (array_reverse($expr->vars) as $var) { $scope = $this->lookForUnsetAllowedUndefinedExpressions($scope, $var); diff --git a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php index 3232e63282..511bf6136d 100644 --- a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php +++ b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php @@ -484,7 +484,7 @@ public function testBug10248(): void $this->analyse([__DIR__ . '/data/bug-10248.php'], []); } - #[RequiresPhp('>= 8.0')] + #[RequiresPhp('>= 8.2')] public function testBug11815(): void { $this->analyse([__DIR__ . '/data/bug-11815.php'], []); diff --git a/tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php b/tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php index 272c010d97..e7d97b6546 100644 --- a/tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php +++ b/tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php @@ -604,6 +604,11 @@ public function testBug9568(): void $this->analyse([__DIR__ . '/data/bug-9568.php'], []); } + public function testBug11427(): void + { + $this->analyse([__DIR__ . '/data/bug-11427.php'], []); + } + #[RequiresPhp('>= 8.4')] public function testPropertyHooks(): void { diff --git a/tests/PHPStan/Rules/Exceptions/data/bug-11427.php b/tests/PHPStan/Rules/Exceptions/data/bug-11427.php new file mode 100644 index 0000000000..279a2f448d --- /dev/null +++ b/tests/PHPStan/Rules/Exceptions/data/bug-11427.php @@ -0,0 +1,85 @@ + */ +class C implements \ArrayAccess { + #[\ReturnTypeWillChange] + public function offsetExists($offset) { + throw new \Exception("exists"); + } + + #[\ReturnTypeWillChange] + public function offsetGet($offset) { + throw new \Exception("get"); + } + + #[\ReturnTypeWillChange] + public function offsetSet($offset, $value) { + throw new \Exception("set"); + } + + #[\ReturnTypeWillChange] + public function offsetUnset($offset) { + throw new \Exception("unset"); + } +} + +function test(C $c): void { + try { + $x = isset($c[1]); + } catch (\Exception $e) { + // offsetExists can throw + } + + try { + $x = $c[1]; + } catch (\Exception $e) { + // offsetGet can throw + } + + try { + $c[1] = 1; + } catch (\Exception $e) { + // offsetSet can throw + } + + try { + unset($c[1]); + } catch (\Exception $e) { + // offsetUnset can throw + } +} + +/** + * Union type where isArray() returns maybe and isSuperTypeOf(ArrayAccess) returns maybe. + * This ensures the conditions in NodeScopeResolver are tested with types + * that distinguish !->yes() from ->no() and !->no() from ->yes(). + * + * @param array|C $c + */ +function testArrayOrArrayAccess($c): void { + try { + $x = isset($c[1]); + } catch (\Exception $e) { + // offsetExists can throw when $c is C + } + + try { + $x = $c[1]; + } catch (\Exception $e) { + // offsetGet can throw when $c is C + } + + try { + $c[1] = 1; + } catch (\Exception $e) { + // offsetSet can throw when $c is C + } + + try { + unset($c[1]); + } catch (\Exception $e) { + // offsetUnset can throw when $c is C + } +}