Skip to content
Closed
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 @@ -232,6 +232,10 @@ When considering a bug fix that involves checking "is this type a Foo?", first c

`MutatingScope` has separate methods for entering arrow functions (`enterArrowFunctionWithoutReflection`) and closures (`enterAnonymousFunctionWithoutReflection`). Both iterate over parameters and assign types, but they must use the same logic for computing parameter types. In particular, both must call `getFunctionType($parameter->type, $isNullable, $parameter->variadic)` to properly handle variadic parameters (wrapping the inner type in `ArrayType`). Shortcuts like `new MixedType()` for untyped parameters skip the variadic wrapping and cause variadic args to be typed as `mixed` instead of `array<int|string, mixed>`. When fixing bugs in one method, check the other for the same issue.

### Arrow function vs closure `$this` property type narrowing

Arrow functions and closures must handle `$this` property expression types consistently. Closures build their expression types from scratch and only preserve readonly property fetches on `$this` (lines ~2762-2776 of `enterAnonymousFunctionWithoutReflection`). Arrow functions start with the full parent scope (`$arrowFunctionScope = $this`) and must explicitly filter out non-readonly `$this` property fetches via `invalidateNonReadonlyThisPropertyFetches()`. Without this filtering, arrow functions incorrectly inherit narrowed property types from the parent scope (e.g. after `$this->dates = []`, the arrow function would see `array{}` instead of the PHPDoc type `array<int, DateTime>`). This only applies to non-static arrow functions where `$this` is available. Note: when fixing code that relied on property narrowing inside arrow functions, the pattern is to assign the property to a local variable before the arrow function (e.g. `$default = $this->default; fn () => $default->method()`).

### MutatingScope: expression invalidation during scope merging

When two scopes are merged (e.g. after if/else branches), `MutatingScope::generalizeWith()` must invalidate dependent expressions. If variable `$i` changes, then `$locations[$i]` must be invalidated too. Bugs arise when stale `ExpressionTypeHolder` entries survive scope merges. Fix pattern: in `MutatingScope`, when a root expression changes, skip/invalidate all deep expressions that depend on it.
Expand Down
35 changes: 33 additions & 2 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -2852,6 +2852,29 @@
return $filteredExpressionTypes;
}

/**
* @param array<string, ExpressionTypeHolder> $expressionTypes
* @return array<string, ExpressionTypeHolder>
*/
private function invalidateNonReadonlyThisPropertyFetches(array $expressionTypes): array
{
$filteredExpressionTypes = [];
foreach ($expressionTypes as $exprString => $expressionType) {
$expr = $expressionType->getExpr();
if (
$expr instanceof PropertyFetch
&& $expr->var instanceof Variable
&& is_string($expr->var->name)
&& $expr->var->name === 'this'
&& !$this->isReadonlyPropertyFetch($expr, true)
) {
continue;
}
$filteredExpressionTypes[$exprString] = $expressionType;
}
return $filteredExpressionTypes;
}

/**
* @api
* @param ParameterReflection[]|null $callableParameters
Expand Down Expand Up @@ -2920,13 +2943,21 @@
$arrowFunctionScope = $arrowFunctionScope->invalidateExpression(new Variable('this'));
}

$expressionTypes = $this->invalidateStaticExpressions($arrowFunctionScope->expressionTypes);
$nativeExpressionTypes = $arrowFunctionScope->nativeExpressionTypes;

if (!$arrowFunction->static && $this->hasVariableType('this')->yes()) {

Check warning on line 2949 in src/Analyser/MutatingScope.php

View workflow job for this annotation

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

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $expressionTypes = $this->invalidateStaticExpressions($arrowFunctionScope->expressionTypes); $nativeExpressionTypes = $arrowFunctionScope->nativeExpressionTypes; - if (!$arrowFunction->static && $this->hasVariableType('this')->yes()) { + if (!$arrowFunction->static && !$this->hasVariableType('this')->no()) { $expressionTypes = $this->invalidateNonReadonlyThisPropertyFetches($expressionTypes); $nativeExpressionTypes = $this->invalidateNonReadonlyThisPropertyFetches($nativeExpressionTypes); }

Check warning on line 2949 in src/Analyser/MutatingScope.php

View workflow job for this annotation

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

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $expressionTypes = $this->invalidateStaticExpressions($arrowFunctionScope->expressionTypes); $nativeExpressionTypes = $arrowFunctionScope->nativeExpressionTypes; - if (!$arrowFunction->static && $this->hasVariableType('this')->yes()) { + if (!$arrowFunction->static && !$this->hasVariableType('this')->no()) { $expressionTypes = $this->invalidateNonReadonlyThisPropertyFetches($expressionTypes); $nativeExpressionTypes = $this->invalidateNonReadonlyThisPropertyFetches($nativeExpressionTypes); }
$expressionTypes = $this->invalidateNonReadonlyThisPropertyFetches($expressionTypes);
$nativeExpressionTypes = $this->invalidateNonReadonlyThisPropertyFetches($nativeExpressionTypes);
}

return $this->scopeFactory->create(
$arrowFunctionScope->context,
$this->isDeclareStrictTypes(),
$arrowFunctionScope->getFunction(),
$arrowFunctionScope->getNamespace(),
$this->invalidateStaticExpressions($arrowFunctionScope->expressionTypes),
$arrowFunctionScope->nativeExpressionTypes,
$expressionTypes,
$nativeExpressionTypes,
$arrowFunctionScope->conditionalExpressions,
$arrowFunctionScope->inClosureBindScopeClasses,
new ClosureType(),
Expand Down
3 changes: 2 additions & 1 deletion src/Type/Generic/TemplateTypeTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ public function describe(VerbosityLevel $level): string
}
$defaultDescription = '';
if ($this->default !== null) {
$recursionGuard = RecursionGuard::runOnObjectIdentity($this->default, fn () => $this->default->describe($level));
$default = $this->default;
$recursionGuard = RecursionGuard::runOnObjectIdentity($default, static fn () => $default->describe($level));
if (!$recursionGuard instanceof ErrorType) {
$defaultDescription .= sprintf(' = %s', $recursionGuard);
}
Expand Down
34 changes: 34 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-12912.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php declare(strict_types = 1); // lint >= 8.1

namespace Bug12912;

use function PHPStan\Testing\assertType;

class Foo
{
protected Bar $foo = Bar::Yes;

public function foo(): void
{
if($this->foo === Bar::No) {
return;
}

assertType('Bug12912\Bar::Yes', $this->foo);

$this->wrap(fn() => assertType('Bug12912\Bar', $this->foo));

$this->wrap(function() { assertType('Bug12912\Bar', $this->foo); });
}

public function wrap(callable $callback): void
{
$callback();
}
}

enum Bar: string
{
case Yes = 'yes';
case No = 'no';
}
54 changes: 54 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-13563.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php declare(strict_types = 1);

namespace Bug13563;

use DateTime;
use function PHPStan\Testing\assertType;

class Invoker
{
/**
* @var array<string, \Closure>
*/
private array $callbacks = [];

public function willReturnCallback(string $method, callable $callback): void
{
$this->callbacks[$method] = \Closure::fromCallable($callback);
}
}

class MyTest
{
/**
* @var array<int, DateTime>
*/
private array $dates = [];

/**
* @var array<int, DateTime>
*/
private array $propNotCleared = [];

public function setUp(): void
{
$invoker = new Invoker();
$this->dates = [];

// Arrow function after property reset - should use PHPDoc type, not narrowed empty array
$invoker->willReturnCallback('get1', fn (int $id) => assertType('array<int, DateTime>', $this->dates));

// Closure after property reset - should use PHPDoc type
$invoker->willReturnCallback('get2', function (int $id) {
assertType('array<int, DateTime>', $this->dates);
});

// Arrow function without property reset - should use PHPDoc type
$invoker->willReturnCallback('get3', fn (int $id) => assertType('array<int, DateTime>', $this->propNotCleared));

// Closure without property reset - should use PHPDoc type
$invoker->willReturnCallback('get4', function (int $id) {
assertType('array<int, DateTime>', $this->propNotCleared);
});
}
}
Loading