From e23ee0b8c1eba24616544608ad3a88a229b6a577 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 9 Sep 2025 15:55:29 +0200 Subject: [PATCH 01/10] Remember narrowed readonly types in anonymous functions --- src/Analyser/MutatingScope.php | 45 +++++++++++++++++------ tests/PHPStan/Analyser/nsrt/bug-13321.php | 42 +++++++++++++++++++++ 2 files changed, 75 insertions(+), 12 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-13321.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index ff8b6fdfb0..99e73e5bb3 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3737,19 +3737,40 @@ private function enterAnonymousFunctionWithoutReflection( foreach ($this->invalidateStaticExpressions($this->expressionTypes) as $exprString => $typeHolder) { $expr = $typeHolder->getExpr(); - if ($expr instanceof Variable) { - continue; - } - $variables = (new NodeFinder())->findInstanceOf([$expr], Variable::class); - if ($variables === [] && !$this->expressionTypeIsUnchangeable($typeHolder)) { - continue; - } - foreach ($variables as $variable) { - if (!is_string($variable->name)) { - continue 2; + + if ( + $expr instanceof PropertyFetch + && $expr->name instanceof Node\Identifier + && $expr->var instanceof Variable + && is_string($expr->var->name) + && $expr->var->name === 'this' + && $this->phpVersion->supportsReadOnlyProperties() + ) { + $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this); + if ($propertyReflection === null) { + continue; } - if (!array_key_exists($variable->name, $nonRefVariableNames)) { - continue 2; + $nativePropertyReflection = $propertyReflection->getNativeReflection(); + if ($nativePropertyReflection === null || !$nativePropertyReflection->isReadOnly()) { + continue; + } + } else { + if ($expr instanceof Variable) { + continue; + } + + $variables = (new NodeFinder())->findInstanceOf([$expr], Variable::class); + if ($variables === [] && !$this->expressionTypeIsUnchangeable($typeHolder)) { + continue; + } + + foreach ($variables as $variable) { + if (!is_string($variable->name)) { + continue 2; + } + if (!array_key_exists($variable->name, $nonRefVariableNames)) { + continue 2; + } } } diff --git a/tests/PHPStan/Analyser/nsrt/bug-13321.php b/tests/PHPStan/Analyser/nsrt/bug-13321.php new file mode 100644 index 0000000000..ed4d4e8b29 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-13321.php @@ -0,0 +1,42 @@ +foo === null) { + return; + } + if ($this->writableFoo === null) { + return; + } + + $test = function () { + assertType(Foo::class, $this->foo); + assertType(Foo::class.'|null', $this->writableFoo); + + echo $this->foo->value; + }; + + $test(); + } + +} From 33e8c09b669a00fb09749d8341230e1f1b3af504 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 9 Sep 2025 20:32:16 +0200 Subject: [PATCH 02/10] test readonly class --- tests/PHPStan/Analyser/nsrt/bug-13321.php | 2 +- tests/PHPStan/Analyser/nsrt/bug-13321b.php | 42 ++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-13321b.php diff --git a/tests/PHPStan/Analyser/nsrt/bug-13321.php b/tests/PHPStan/Analyser/nsrt/bug-13321.php index ed4d4e8b29..7bfc163953 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-13321.php +++ b/tests/PHPStan/Analyser/nsrt/bug-13321.php @@ -1,4 +1,4 @@ -= 8.1 namespace Bug13321; diff --git a/tests/PHPStan/Analyser/nsrt/bug-13321b.php b/tests/PHPStan/Analyser/nsrt/bug-13321b.php new file mode 100644 index 0000000000..454e62cb34 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-13321b.php @@ -0,0 +1,42 @@ += 8.2 + +namespace Bug13321b; + +use function PHPStan\Testing\assertType; + +class Foo +{ + public function __construct(public string $value) + { + } +} + +readonly class Bar +{ + public function __construct( + private ?Foo $foo, + ) + { + } + + public function bar(): void + { + if ($this->foo === null) { + return; + } + if ($this->foo->value === '') { + return; + } + + assertType(Foo::class, $this->foo); + assertType('non-empty-string', $this->foo->value); + + $test = function () { + assertType(Foo::class, $this->foo); + assertType('string', $this->foo->value); + }; + + $test(); + } + +} From a830aa6439f3fb374ada960449a884099d4d2b57 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 9 Sep 2025 20:44:32 +0200 Subject: [PATCH 03/10] test static anonyous functions --- src/Analyser/MutatingScope.php | 2 ++ tests/PHPStan/Analyser/nsrt/bug-13321.php | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 99e73e5bb3..845ae8b32c 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3744,6 +3744,8 @@ private function enterAnonymousFunctionWithoutReflection( && $expr->var instanceof Variable && is_string($expr->var->name) && $expr->var->name === 'this' + && !$closure->static + && $this->hasVariableType('this')->yes() && $this->phpVersion->supportsReadOnlyProperties() ) { $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this); diff --git a/tests/PHPStan/Analyser/nsrt/bug-13321.php b/tests/PHPStan/Analyser/nsrt/bug-13321.php index 7bfc163953..ce83351e78 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-13321.php +++ b/tests/PHPStan/Analyser/nsrt/bug-13321.php @@ -37,6 +37,15 @@ public function bar(): void }; $test(); + + $test = static function () { + assertType('mixed', $this->foo); + assertType('mixed', $this->writableFoo); + + echo $this->foo->value; + }; + + $test(); } } From b2342159dd74f0a71850a3ffe5c3d51fe54da7f7 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 9 Sep 2025 21:02:18 +0200 Subject: [PATCH 04/10] only in immediately invoked --- src/Analyser/MutatingScope.php | 2 ++ tests/PHPStan/Analyser/nsrt/bug-13321.php | 9 ++++++++- tests/PHPStan/Analyser/nsrt/bug-13321b.php | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 845ae8b32c..6987f0b324 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -50,6 +50,7 @@ use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Node\PropertyAssignNode; use PHPStan\Parser\ArrayMapArgVisitor; +use PHPStan\Parser\ImmediatelyInvokedClosureVisitor; use PHPStan\Parser\NewAssignedToPropertyVisitor; use PHPStan\Parser\Parser; use PHPStan\Php\PhpVersion; @@ -3744,6 +3745,7 @@ private function enterAnonymousFunctionWithoutReflection( && $expr->var instanceof Variable && is_string($expr->var->name) && $expr->var->name === 'this' + && $closure->getAttribute(ImmediatelyInvokedClosureVisitor::ATTRIBUTE_NAME) === true && !$closure->static && $this->hasVariableType('this')->yes() && $this->phpVersion->supportsReadOnlyProperties() diff --git a/tests/PHPStan/Analyser/nsrt/bug-13321.php b/tests/PHPStan/Analyser/nsrt/bug-13321.php index ce83351e78..e2695881b0 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-13321.php +++ b/tests/PHPStan/Analyser/nsrt/bug-13321.php @@ -29,10 +29,17 @@ public function bar(): void return; } - $test = function () { + (function () { assertType(Foo::class, $this->foo); assertType(Foo::class.'|null', $this->writableFoo); + echo $this->foo->value; + })(); + + $test = function () { + assertType(Foo::class.'|null', $this->foo); + assertType(Foo::class.'|null', $this->writableFoo); + echo $this->foo->value; }; diff --git a/tests/PHPStan/Analyser/nsrt/bug-13321b.php b/tests/PHPStan/Analyser/nsrt/bug-13321b.php index 454e62cb34..b1b0822b0e 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-13321b.php +++ b/tests/PHPStan/Analyser/nsrt/bug-13321b.php @@ -32,7 +32,7 @@ public function bar(): void assertType('non-empty-string', $this->foo->value); $test = function () { - assertType(Foo::class, $this->foo); + assertType(Foo::class.'|null', $this->foo); assertType('string', $this->foo->value); }; From 5f9d68ccad158e4710970baa0d81cbdb85718910 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 9 Sep 2025 21:14:28 +0200 Subject: [PATCH 05/10] fix --- src/Analyser/MutatingScope.php | 2 -- tests/PHPStan/Analyser/nsrt/bug-13321.php | 9 ++++++++- tests/PHPStan/Analyser/nsrt/bug-13321b.php | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 6987f0b324..845ae8b32c 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -50,7 +50,6 @@ use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Node\PropertyAssignNode; use PHPStan\Parser\ArrayMapArgVisitor; -use PHPStan\Parser\ImmediatelyInvokedClosureVisitor; use PHPStan\Parser\NewAssignedToPropertyVisitor; use PHPStan\Parser\Parser; use PHPStan\Php\PhpVersion; @@ -3745,7 +3744,6 @@ private function enterAnonymousFunctionWithoutReflection( && $expr->var instanceof Variable && is_string($expr->var->name) && $expr->var->name === 'this' - && $closure->getAttribute(ImmediatelyInvokedClosureVisitor::ATTRIBUTE_NAME) === true && !$closure->static && $this->hasVariableType('this')->yes() && $this->phpVersion->supportsReadOnlyProperties() diff --git a/tests/PHPStan/Analyser/nsrt/bug-13321.php b/tests/PHPStan/Analyser/nsrt/bug-13321.php index e2695881b0..66a1f724d5 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-13321.php +++ b/tests/PHPStan/Analyser/nsrt/bug-13321.php @@ -22,6 +22,13 @@ public function __construct( public function bar(): void { + (function () { + assertType(Foo::class.'|null', $this->foo); + assertType(Foo::class.'|null', $this->writableFoo); + + echo $this->foo->value; + })(); + if ($this->foo === null) { return; } @@ -37,7 +44,7 @@ public function bar(): void })(); $test = function () { - assertType(Foo::class.'|null', $this->foo); + assertType(Foo::class, $this->foo); assertType(Foo::class.'|null', $this->writableFoo); echo $this->foo->value; diff --git a/tests/PHPStan/Analyser/nsrt/bug-13321b.php b/tests/PHPStan/Analyser/nsrt/bug-13321b.php index b1b0822b0e..454e62cb34 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-13321b.php +++ b/tests/PHPStan/Analyser/nsrt/bug-13321b.php @@ -32,7 +32,7 @@ public function bar(): void assertType('non-empty-string', $this->foo->value); $test = function () { - assertType(Foo::class.'|null', $this->foo); + assertType(Foo::class, $this->foo); assertType('string', $this->foo->value); }; From 98f55585633714ed25a760c14c2d9ba605ef4fac Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 9 Sep 2025 21:57:11 +0200 Subject: [PATCH 06/10] simplify diff --- src/Analyser/MutatingScope.php | 74 +++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 845ae8b32c..3990da732b 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3738,41 +3738,21 @@ private function enterAnonymousFunctionWithoutReflection( foreach ($this->invalidateStaticExpressions($this->expressionTypes) as $exprString => $typeHolder) { $expr = $typeHolder->getExpr(); - if ( - $expr instanceof PropertyFetch - && $expr->name instanceof Node\Identifier - && $expr->var instanceof Variable - && is_string($expr->var->name) - && $expr->var->name === 'this' - && !$closure->static - && $this->hasVariableType('this')->yes() - && $this->phpVersion->supportsReadOnlyProperties() - ) { - $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this); - if ($propertyReflection === null) { - continue; - } - $nativePropertyReflection = $propertyReflection->getNativeReflection(); - if ($nativePropertyReflection === null || !$nativePropertyReflection->isReadOnly()) { - continue; - } - } else { - if ($expr instanceof Variable) { - continue; - } + if ($expr instanceof Variable) { + continue; + } - $variables = (new NodeFinder())->findInstanceOf([$expr], Variable::class); - if ($variables === [] && !$this->expressionTypeIsUnchangeable($typeHolder)) { - continue; - } + $variables = (new NodeFinder())->findInstanceOf([$expr], Variable::class); + if ($variables === [] && !$this->expressionTypeIsUnchangeable($typeHolder)) { + continue; + } - foreach ($variables as $variable) { - if (!is_string($variable->name)) { - continue 2; - } - if (!array_key_exists($variable->name, $nonRefVariableNames)) { - continue 2; - } + foreach ($variables as $variable) { + if (!is_string($variable->name)) { + continue 2; + } + if (!array_key_exists($variable->name, $nonRefVariableNames)) { + continue 2; } } @@ -3783,6 +3763,34 @@ private function enterAnonymousFunctionWithoutReflection( $node = new Variable('this'); $expressionTypes['$this'] = ExpressionTypeHolder::createYes($node, $this->getType($node)); $nativeTypes['$this'] = ExpressionTypeHolder::createYes($node, $this->getNativeType($node)); + + if ($this->phpVersion->supportsReadOnlyProperties()) { + foreach ($this->invalidateStaticExpressions($this->expressionTypes) as $exprString => $typeHolder) { + $expr = $typeHolder->getExpr(); + + if ( + !$expr instanceof PropertyFetch + || !$expr->name instanceof Node\Identifier + || !$expr->var instanceof Variable + || !is_string($expr->var->name) + || $expr->var->name !== 'this' + ) { + continue; + } + + $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this); + if ($propertyReflection === null) { + continue; + } + + $nativePropertyReflection = $propertyReflection->getNativeReflection(); + if ($nativePropertyReflection === null || !$nativePropertyReflection->isReadOnly()) { + continue; + } + + $expressionTypes[$exprString] = $typeHolder; + } + } } return $this->scopeFactory->create( From 54db532754179e4596ae5606d435a0a4ac5c2264 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 10 Sep 2025 10:09:30 +0200 Subject: [PATCH 07/10] prevent duplicated work --- src/Analyser/MutatingScope.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 3990da732b..fc265b2b14 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3735,7 +3735,8 @@ private function enterAnonymousFunctionWithoutReflection( $nativeTypes[$paramExprString] = ExpressionTypeHolder::createYes($use->var, $variableNativeType); } - foreach ($this->invalidateStaticExpressions($this->expressionTypes) as $exprString => $typeHolder) { + $nonStaticExpressions = $this->invalidateStaticExpressions($this->expressionTypes); + foreach ($nonStaticExpressions as $exprString => $typeHolder) { $expr = $typeHolder->getExpr(); if ($expr instanceof Variable) { @@ -3765,7 +3766,7 @@ private function enterAnonymousFunctionWithoutReflection( $nativeTypes['$this'] = ExpressionTypeHolder::createYes($node, $this->getNativeType($node)); if ($this->phpVersion->supportsReadOnlyProperties()) { - foreach ($this->invalidateStaticExpressions($this->expressionTypes) as $exprString => $typeHolder) { + foreach ($nonStaticExpressions as $exprString => $typeHolder) { $expr = $typeHolder->getExpr(); if ( From 2422f94ffefb158a8e27077f7a1039d3277ed5f2 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 10 Sep 2025 14:11:19 +0200 Subject: [PATCH 08/10] fix --- src/Analyser/MutatingScope.php | 34 +++++++++++++++------- tests/PHPStan/Analyser/nsrt/bug-13321b.php | 10 ++++++- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index fc265b2b14..bc99caab19 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3771,22 +3771,34 @@ private function enterAnonymousFunctionWithoutReflection( if ( !$expr instanceof PropertyFetch - || !$expr->name instanceof Node\Identifier - || !$expr->var instanceof Variable - || !is_string($expr->var->name) - || $expr->var->name !== 'this' ) { continue; } - $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this); - if ($propertyReflection === null) { - continue; - } + while ($expr instanceof PropertyFetch) { + if ($expr->var instanceof Variable) { + if ( + ! $expr->name instanceof Node\Identifier + || !is_string($expr->var->name) + || $expr->var->name !== 'this' + ) { + continue 2; + } + } elseif (!$expr->var instanceof PropertyFetch) { + continue 2; + } - $nativePropertyReflection = $propertyReflection->getNativeReflection(); - if ($nativePropertyReflection === null || !$nativePropertyReflection->isReadOnly()) { - continue; + $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this); + if ($propertyReflection === null) { + continue 2; + } + + $nativePropertyReflection = $propertyReflection->getNativeReflection(); + if ($nativePropertyReflection === null || !$nativePropertyReflection->isReadOnly()) { + continue 2; + } + + $expr = $expr->var; } $expressionTypes[$exprString] = $typeHolder; diff --git a/tests/PHPStan/Analyser/nsrt/bug-13321b.php b/tests/PHPStan/Analyser/nsrt/bug-13321b.php index 454e62cb34..27e6a25fe8 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-13321b.php +++ b/tests/PHPStan/Analyser/nsrt/bug-13321b.php @@ -6,7 +6,10 @@ class Foo { - public function __construct(public string $value) + public function __construct( + public string $value, + readonly public string $readonlyValue, + ) { } } @@ -27,13 +30,18 @@ public function bar(): void if ($this->foo->value === '') { return; } + if ($this->foo->readonlyValue === '') { + return; + } assertType(Foo::class, $this->foo); assertType('non-empty-string', $this->foo->value); + assertType('non-empty-string', $this->foo->readonlyValue); $test = function () { assertType(Foo::class, $this->foo); assertType('string', $this->foo->value); + assertType('non-empty-string', $this->foo->readonlyValue); }; $test(); From c540180489bdc45e3ef1320ff53ea148674a5128 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 10 Sep 2025 15:22:18 +0200 Subject: [PATCH 09/10] re-use logic from preparational PR --- src/Analyser/MutatingScope.php | 30 +++--------------------------- 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index bc99caab19..b14c71cc57 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3769,36 +3769,12 @@ private function enterAnonymousFunctionWithoutReflection( foreach ($nonStaticExpressions as $exprString => $typeHolder) { $expr = $typeHolder->getExpr(); - if ( - !$expr instanceof PropertyFetch - ) { + if (!$expr instanceof PropertyFetch) { continue; } - while ($expr instanceof PropertyFetch) { - if ($expr->var instanceof Variable) { - if ( - ! $expr->name instanceof Node\Identifier - || !is_string($expr->var->name) - || $expr->var->name !== 'this' - ) { - continue 2; - } - } elseif (!$expr->var instanceof PropertyFetch) { - continue 2; - } - - $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($expr, $this); - if ($propertyReflection === null) { - continue 2; - } - - $nativePropertyReflection = $propertyReflection->getNativeReflection(); - if ($nativePropertyReflection === null || !$nativePropertyReflection->isReadOnly()) { - continue 2; - } - - $expr = $expr->var; + if (!$this->isReadonlyPropertyFetchOnThis($expr)) { + continue; } $expressionTypes[$exprString] = $typeHolder; From cabf11dd76353e1519edc0def4517f2452156f2a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 10 Sep 2025 15:37:57 +0200 Subject: [PATCH 10/10] rebase --- src/Analyser/MutatingScope.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index b14c71cc57..7b0c7d239c 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3773,7 +3773,7 @@ private function enterAnonymousFunctionWithoutReflection( continue; } - if (!$this->isReadonlyPropertyFetchOnThis($expr)) { + if (!$this->isReadonlyPropertyFetch($expr, true)) { continue; }