Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
48 changes: 48 additions & 0 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -2080,6 +2080,20 @@
$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()) {

Check warning on line 2086 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $varType = $scope->getType($var->var); /** @infection-ignore-all */ $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); - if (!$varType->isArray()->yes() && !$isArrayAccess->no()) { + if (!$varType->isArray()->yes() && $isArrayAccess->yes()) { $throwPoints = array_merge($throwPoints, $this->processExprNode( $stmt, new MethodCall($this->deepNodeCloner->cloneNode($var->var), 'offsetUnset'),

Check warning on line 2086 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $varType = $scope->getType($var->var); /** @infection-ignore-all */ $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); - if (!$varType->isArray()->yes() && !$isArrayAccess->no()) { + if ($varType->isArray()->no() && !$isArrayAccess->no()) { $throwPoints = array_merge($throwPoints, $this->processExprNode( $stmt, new MethodCall($this->deepNodeCloner->cloneNode($var->var), 'offsetUnset'),

Check warning on line 2086 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $varType = $scope->getType($var->var); /** @infection-ignore-all */ $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); - if (!$varType->isArray()->yes() && !$isArrayAccess->no()) { + if (!$varType->isArray()->yes() && $isArrayAccess->yes()) { $throwPoints = array_merge($throwPoints, $this->processExprNode( $stmt, new MethodCall($this->deepNodeCloner->cloneNode($var->var), 'offsetUnset'),

Check warning on line 2086 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $varType = $scope->getType($var->var); /** @infection-ignore-all */ $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); - if (!$varType->isArray()->yes() && !$isArrayAccess->no()) { + if ($varType->isArray()->no() && !$isArrayAccess->no()) { $throwPoints = array_merge($throwPoints, $this->processExprNode( $stmt, new MethodCall($this->deepNodeCloner->cloneNode($var->var), 'offsetUnset'),
$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 {
Expand Down Expand Up @@ -3602,6 +3616,20 @@
$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()) {

Check warning on line 3623 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $varType = $scope->getType($expr->var); /** @infection-ignore-all */ $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); - if (!$varType->isArray()->yes() && !$isArrayAccess->no()) { + if (!$varType->isArray()->yes() && $isArrayAccess->yes()) { $throwPoints = array_merge($throwPoints, $this->processExprNode( $stmt, new MethodCall($this->deepNodeCloner->cloneNode($expr->var), 'offsetGet'),

Check warning on line 3623 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $varType = $scope->getType($expr->var); /** @infection-ignore-all */ $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); - if (!$varType->isArray()->yes() && !$isArrayAccess->no()) { + if ($varType->isArray()->no() && !$isArrayAccess->no()) { $throwPoints = array_merge($throwPoints, $this->processExprNode( $stmt, new MethodCall($this->deepNodeCloner->cloneNode($expr->var), 'offsetGet'),

Check warning on line 3623 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $varType = $scope->getType($expr->var); /** @infection-ignore-all */ $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); - if (!$varType->isArray()->yes() && !$isArrayAccess->no()) { + if (!$varType->isArray()->yes() && $isArrayAccess->yes()) { $throwPoints = array_merge($throwPoints, $this->processExprNode( $stmt, new MethodCall($this->deepNodeCloner->cloneNode($expr->var), 'offsetGet'),

Check warning on line 3623 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $varType = $scope->getType($expr->var); /** @infection-ignore-all */ $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); - if (!$varType->isArray()->yes() && !$isArrayAccess->no()) { + if ($varType->isArray()->no() && !$isArrayAccess->no()) { $throwPoints = array_merge($throwPoints, $this->processExprNode( $stmt, new MethodCall($this->deepNodeCloner->cloneNode($expr->var), 'offsetGet'),
$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;
Expand Down Expand Up @@ -3897,6 +3925,26 @@
$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()) {

Check warning on line 3936 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $varType = $scope->getType($var->var); /** @infection-ignore-all */ $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); - if ($varType->isArray()->yes() || $isArrayAccess->no()) { + if ($varType->isArray()->yes() || !$isArrayAccess->yes()) { continue; }

Check warning on line 3936 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $varType = $scope->getType($var->var); /** @infection-ignore-all */ $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); - if ($varType->isArray()->yes() || $isArrayAccess->no()) { + if (!$varType->isArray()->no() || $isArrayAccess->no()) { continue; }

Check warning on line 3936 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $varType = $scope->getType($var->var); /** @infection-ignore-all */ $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); - if ($varType->isArray()->yes() || $isArrayAccess->no()) { + if ($varType->isArray()->yes() || !$isArrayAccess->yes()) { continue; }

Check warning on line 3936 in src/Analyser/NodeScopeResolver.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $varType = $scope->getType($var->var); /** @infection-ignore-all */ $isArrayAccess = (new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType); - if ($varType->isArray()->yes() || $isArrayAccess->no()) { + if (!$varType->isArray()->no() || $isArrayAccess->no()) { continue; }
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);
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Rules/Classes/InstantiationRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'], []);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
85 changes: 85 additions & 0 deletions tests/PHPStan/Rules/Exceptions/data/bug-11427.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?php

namespace Bug11427;

/** @implements \ArrayAccess<int, int> */
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<int, int>|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
}
}
Loading