diff --git a/CLAUDE.md b/CLAUDE.md index f48abb644d..c353c8b988 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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`. 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`). 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. diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index b81f89088c..e0135ae12b 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -2852,6 +2852,29 @@ private function invalidateStaticExpressions(array $expressionTypes): array return $filteredExpressionTypes; } + /** + * @param array $expressionTypes + * @return array + */ + 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 @@ -2920,13 +2943,21 @@ private function enterArrowFunctionWithoutReflection(Expr\ArrowFunction $arrowFu $arrowFunctionScope = $arrowFunctionScope->invalidateExpression(new Variable('this')); } + $expressionTypes = $this->invalidateStaticExpressions($arrowFunctionScope->expressionTypes); + $nativeExpressionTypes = $arrowFunctionScope->nativeExpressionTypes; + + if (!$arrowFunction->static && $this->hasVariableType('this')->yes()) { + $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(), diff --git a/src/Type/Generic/TemplateTypeTrait.php b/src/Type/Generic/TemplateTypeTrait.php index a1f381e6f5..f61f43d2d0 100644 --- a/src/Type/Generic/TemplateTypeTrait.php +++ b/src/Type/Generic/TemplateTypeTrait.php @@ -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); } diff --git a/tests/PHPStan/Analyser/nsrt/bug-12912.php b/tests/PHPStan/Analyser/nsrt/bug-12912.php new file mode 100644 index 0000000000..52a0492478 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-12912.php @@ -0,0 +1,34 @@ += 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'; +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-13563.php b/tests/PHPStan/Analyser/nsrt/bug-13563.php new file mode 100644 index 0000000000..ac2fda99a7 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-13563.php @@ -0,0 +1,54 @@ + + */ + private array $callbacks = []; + + public function willReturnCallback(string $method, callable $callback): void + { + $this->callbacks[$method] = \Closure::fromCallable($callback); + } +} + +class MyTest +{ + /** + * @var array + */ + private array $dates = []; + + /** + * @var array + */ + 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', $this->dates)); + + // Closure after property reset - should use PHPDoc type + $invoker->willReturnCallback('get2', function (int $id) { + assertType('array', $this->dates); + }); + + // Arrow function without property reset - should use PHPDoc type + $invoker->willReturnCallback('get3', fn (int $id) => assertType('array', $this->propNotCleared)); + + // Closure without property reset - should use PHPDoc type + $invoker->willReturnCallback('get4', function (int $id) { + assertType('array', $this->propNotCleared); + }); + } +}