From f19ecbf0313a1a342ca8595673add2b7c1dba855 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Hansl=C3=ADk?= Date: Wed, 11 Dec 2024 09:27:35 +0100 Subject: [PATCH 1/2] Fixed promoted properties with hooks --- src/Reflection/ReflectionClass.php | 3 ++- test/unit/Fixture/PropertyHooks.php | 7 +++++++ test/unit/Reflection/ReflectionPropertyTest.php | 12 ++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Reflection/ReflectionClass.php b/src/Reflection/ReflectionClass.php index 54d598034..20c9fbb1d 100644 --- a/src/Reflection/ReflectionClass.php +++ b/src/Reflection/ReflectionClass.php @@ -863,7 +863,7 @@ private function createImmediateProperties(ClassNode|InterfaceNode|TraitNode|Enu } foreach ($methodNode->params as $parameterNode) { - if ($parameterNode->flags === 0) { + if ($parameterNode->flags === 0 && $parameterNode->hooks === []) { // No flags, no promotion continue; } @@ -878,6 +878,7 @@ private function createImmediateProperties(ClassNode|InterfaceNode|TraitNode|Enu $parameterNode->getAttributes(), $parameterNode->type, $parameterNode->attrGroups, + $parameterNode->hooks, ); $property = ReflectionProperty::createFromNode( $reflector, diff --git a/test/unit/Fixture/PropertyHooks.php b/test/unit/Fixture/PropertyHooks.php index 81359ce0f..820ff65f6 100644 --- a/test/unit/Fixture/PropertyHooks.php +++ b/test/unit/Fixture/PropertyHooks.php @@ -128,3 +128,10 @@ interface InterfacePropertyHooks public string $readAndWriteHook { get; set; } } + +class PromotedPropertyHooks +{ + public function __construct(string $hook{ get {} }) + { + } +} diff --git a/test/unit/Reflection/ReflectionPropertyTest.php b/test/unit/Reflection/ReflectionPropertyTest.php index 703ad8387..ed8cb3e34 100644 --- a/test/unit/Reflection/ReflectionPropertyTest.php +++ b/test/unit/Reflection/ReflectionPropertyTest.php @@ -1121,4 +1121,16 @@ public function testUseHookFromTrait(): void self::assertFalse($hookProperty->hasHook(ReflectionPropertyHookType::Set)); self::assertSame('Roave\BetterReflectionTest\Fixture\PropertyHookTrait', $hookProperty->getHook(ReflectionPropertyHookType::Get)->getDeclaringClass()->getName()); } + + public function testPromotedPropertyWithHooks(): void + { + $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/PropertyHooks.php', $this->astLocator)); + $getClassInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\PromotedPropertyHooks'); + + $hookProperty = $getClassInfo->getProperty('hook'); + self::assertTrue($hookProperty->isPromoted()); + self::assertCount(1, $hookProperty->getHooks()); + self::assertTrue($hookProperty->hasHook(ReflectionPropertyHookType::Get)); + self::assertFalse($hookProperty->hasHook(ReflectionPropertyHookType::Set)); + } } From f9bad58ee2cd8ee263e469a951a75bd7c1553811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Hansl=C3=ADk?= Date: Wed, 11 Dec 2024 10:15:09 +0100 Subject: [PATCH 2/2] Improved detecting virtual properties --- psalm-baseline.xml | 1 - src/Reflection/ReflectionProperty.php | 46 ++++++++++----------------- 2 files changed, 16 insertions(+), 31 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index e2f9ca57c..30e9b9788 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -220,7 +220,6 @@ - diff --git a/src/Reflection/ReflectionProperty.php b/src/Reflection/ReflectionProperty.php index ecadb99c7..414c75575 100644 --- a/src/Reflection/ReflectionProperty.php +++ b/src/Reflection/ReflectionProperty.php @@ -723,24 +723,7 @@ private function computeImmediateVirtualBasedOnGetHook(PropertyNode $node, Node\ return true; } - $visitor = new FindingVisitor(static fn (Node $node): bool => $node instanceof Node\Expr\PropertyFetch); - $traverser = new NodeTraverser($visitor); - $traverser->traverse($getHookBody); - - foreach ($visitor->getFoundNodes() as $propertyFetchNode) { - assert($propertyFetchNode instanceof Node\Expr\PropertyFetch); - - if ( - $propertyFetchNode->var instanceof Node\Expr\Variable - && $propertyFetchNode->var->name === 'this' - && $propertyFetchNode->name instanceof Node\Identifier - && $propertyFetchNode->name->name === $this->name - ) { - return false; - } - } - - return true; + return ! $this->isHookUsingThisProperty($getHook); } private function computeImmediateVirtualBasedOnSetHook(Node\PropertyHook $setHook): bool @@ -757,26 +740,29 @@ private function computeImmediateVirtualBasedOnSetHook(Node\PropertyHook $setHoo return false; } - $visitor = new FindingVisitor(static fn (Node $node): bool => $node instanceof Node\Expr\Assign); + return ! $this->isHookUsingThisProperty($setHook); + } + + private function isHookUsingThisProperty(Node\PropertyHook $hook): bool + { + $visitor = new FindingVisitor(static fn (Node $node): bool => $node instanceof Node\Expr\PropertyFetch); $traverser = new NodeTraverser($visitor); - $traverser->traverse($setHookBody); + $traverser->traverse([$hook]); - foreach ($visitor->getFoundNodes() as $assigNode) { - assert($assigNode instanceof Node\Expr\Assign); - $variableToAssign = $assigNode->var; + foreach ($visitor->getFoundNodes() as $propertyFetchNode) { + assert($propertyFetchNode instanceof Node\Expr\PropertyFetch); if ( - $variableToAssign instanceof Node\Expr\PropertyFetch - && $variableToAssign->var instanceof Node\Expr\Variable - && $variableToAssign->var->name === 'this' - && $variableToAssign->name instanceof Node\Identifier - && $variableToAssign->name->name === $this->name + $propertyFetchNode->var instanceof Node\Expr\Variable + && $propertyFetchNode->var->name === 'this' + && $propertyFetchNode->name instanceof Node\Identifier + && $propertyFetchNode->name->name === $this->name ) { - return false; + return true; } } - return true; + return false; } /** @return array{get?: ReflectionMethod, set?: ReflectionMethod} */