Fixed hooks parameters and return types

Roave · Dec 12, 2024 · 766643c · 766643c
1 parent e574da5
commit 766643c
Show file tree

Hide file tree

Showing 8 changed files with 225 additions and 15 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -53,7 +53,7 @@
       <code><![CDATA[createEmpty]]></code>
       <code><![CDATA[createEmpty]]></code>
       <code><![CDATA[createEmpty]]></code>
-      <code><![CDATA[createFromNode]]></code>
+      <code><![CDATA[createFromMethodNode]]></code>
       <code><![CDATA[createFromNode]]></code>
       <code><![CDATA[createFromNode]]></code>
       <code><![CDATA[createFromNode]]></code>
@@ -205,7 +205,7 @@
     <ImpureMethodCall>
       <code><![CDATA[__invoke]]></code>
       <code><![CDATA[classExists]]></code>
-      <code><![CDATA[createFromNode]]></code>
+      <code><![CDATA[createFromPropertyHook]]></code>
       <code><![CDATA[getEndLine]]></code>
       <code><![CDATA[getStartLine]]></code>
       <code><![CDATA[getStmts]]></code>

diff --git a/src/Reflection/ReflectionClass.php b/src/Reflection/ReflectionClass.php
@@ -567,11 +567,10 @@ private function createImmediateMethods(ClassNode|InterfaceNode|TraitNode|EnumNo
         $methods = [];
 
         foreach ($node->getMethods() as $methodNode) {
-            $method = ReflectionMethod::createFromNode(
+            $method = ReflectionMethod::createFromMethodNode(
                 $reflector,
                 $methodNode,
                 $this->locatedSource,
-                $methodNode->name->name,
                 $this->getNamespaceName(),
                 $this,
                 $this,
@@ -606,14 +605,13 @@ private function addEnumMethods(EnumNode $node, array $methods): array
                 'returnType' => $returnType,
             ];
 
-            return ReflectionMethod::createFromNode(
+            return ReflectionMethod::createFromMethodNode(
                 $this->reflector,
                 new ClassMethod(
                     new Node\Identifier($name),
                     $classMethodSubnodes,
                 ),
                 $internalLocatedSource,
-                $name,
                 $this->getNamespaceName(),
                 $this,
                 $this,

diff --git a/src/Reflection/ReflectionFunctionAbstract.php b/src/Reflection/ReflectionFunctionAbstract.php
@@ -39,13 +39,13 @@ trait ReflectionFunctionAbstract
      * @var array<non-empty-string, ReflectionParameter>
      * @psalm-allow-private-mutation
      */
-    private array $parameters;
+    protected array $parameters;
 
     /** @psalm-allow-private-mutation */
     private bool $returnsReference;
 
     /** @psalm-allow-private-mutation */
-    private ReflectionNamedType|ReflectionUnionType|ReflectionIntersectionType|null $returnType;
+    protected ReflectionNamedType|ReflectionUnionType|ReflectionIntersectionType|null $returnType;
 
     /**
      * @var list<ReflectionAttribute>

diff --git a/src/Reflection/ReflectionMethod.php b/src/Reflection/ReflectionMethod.php
@@ -60,15 +60,13 @@ private function __construct(
     /**
      * @internal
      *
-     * @param non-empty-string      $name
      * @param non-empty-string|null $aliasName
      * @param non-empty-string|null $namespace
      */
-    public static function createFromNode(
+    public static function createFromMethodNode(
         Reflector $reflector,
-        MethodNode|Node\PropertyHook $node,
+        MethodNode $node,
         LocatedSource $locatedSource,
-        string $name,
         string|null $namespace,
         ReflectionClass $declaringClass,
         ReflectionClass $implementingClass,
@@ -79,7 +77,7 @@ public static function createFromNode(
             $reflector,
             $node,
             $locatedSource,
-            $name,
+            $node->name->name,
             $namespace,
             $declaringClass,
             $implementingClass,
@@ -88,6 +86,56 @@ public static function createFromNode(
         );
     }
 
+    /**
+     * @internal
+     *
+     * @param non-empty-string $name
+     */
+    public static function createFromPropertyHook(
+        Reflector $reflector,
+        Node\PropertyHook $node,
+        LocatedSource $locatedSource,
+        string $name,
+        Node\Identifier|Node\Name|Node\NullableType|Node\UnionType|Node\IntersectionType|null $type,
+        ReflectionClass $declaringClass,
+        ReflectionClass $implementingClass,
+        ReflectionClass $currentClass,
+    ): self {
+        $method = new self(
+            $reflector,
+            $node,
+            $locatedSource,
+            $name,
+            null,
+            $declaringClass,
+            $implementingClass,
+            $currentClass,
+            null,
+        );
+
+        if ($node->name->name === 'set') {
+            $method->returnType = ReflectionType::createFromNode($reflector, $method, new Node\Identifier('void'));
+
+            if ($method->parameters === []) {
+                $parameter = ReflectionParameter::createFromNode(
+                    $reflector,
+                    new Node\Param(new Node\Expr\Variable('value'), type: $type),
+                    $method,
+                    0,
+                    false,
+                );
+
+                $method->parameters['value'] = $parameter;
+            }
+        } elseif ($node->name->name === 'get') {
+            $method->returnType = $type !== null
+                ? ReflectionType::createFromNode($reflector, $method, $type)
+                : null;
+        }
+
+        return $method;
+    }
+
     /**
      * Create a reflection of a method by it's name using a named class
      *

diff --git a/src/Reflection/ReflectionProperty.php b/src/Reflection/ReflectionProperty.php
@@ -774,12 +774,15 @@ private function createImmediateHooks(PropertyNode $node): array
             $hookName = $hook->name->name;
             assert($hookName === 'get' || $hookName === 'set');
 
-            $hooks[$hookName] = ReflectionMethod::createFromNode(
+            $hookType = $node->type;
+            assert($hookType === null || $hookType instanceof Node\Identifier || $hookType instanceof Node\Name || $hookType instanceof Node\NullableType || $hookType instanceof Node\UnionType || $hookType instanceof Node\IntersectionType);
+
+            $hooks[$hookName] = ReflectionMethod::createFromPropertyHook(
                 $this->reflector,
                 $hook,
                 $this->getDeclaringClass()->getLocatedSource(),
                 sprintf('$%s::%s', $this->name, $hookName),
-                null,
+                $hookType,
                 $this->getDeclaringClass(),
                 $this->getImplementingClass(),
                 $this->getDeclaringClass(),

diff --git a/test/unit/Fixture/PropertyHooks.php b/test/unit/Fixture/PropertyHooks.php
@@ -135,3 +135,27 @@ public function __construct(string $hook{ get {} })
     {
     }
 }
+
+class SetPropertyHooksParameters
+{
+    public string $hookWithImplicitParameter { set => strtolower($value); }
+
+    public int $hookWithExplicitParameter {
+        set (int $value) {
+            $this->hookWithExplicitParameter = $value;
+        }
+     }
+
+}
+
+class GetPropertyHooksReturnTypes
+{
+    public string $hookWithString { get => 'string'; }
+
+    public int $hookWithInteger { get => 42; }
+
+    public string|int $hookWithUnion { get => 'string'; }
+
+    public $hookWithoutType { get => 'string'; }
+
+}
diff --git a/test/unit/Reflection/ReflectionMethodTest.php b/test/unit/Reflection/ReflectionMethodTest.php
@@ -21,6 +21,7 @@
 use Roave\BetterReflection\Reflection\ReflectionClass;
 use Roave\BetterReflection\Reflection\ReflectionMethod;
 use Roave\BetterReflection\Reflection\ReflectionParameter;
+use Roave\BetterReflection\Reflection\ReflectionPropertyHookType;
 use Roave\BetterReflection\Reflection\ReflectionType;
 use Roave\BetterReflection\Reflection\ReflectionUnionType;
 use Roave\BetterReflection\Reflector\DefaultReflector;
@@ -810,4 +811,71 @@ public function testWithCurrentClass(): void
         self::assertCount(2, $cloneAttributes);
         self::assertSame($attributes[0], $cloneAttributes[0]);
     }
+
+    public function testSetPropertyHookHasImplicitParameter(): void
+    {
+        $reflector    = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/PropertyHooks.php', $this->astLocator));
+        $getClassInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\SetPropertyHooksParameters');
+
+        $hookProperty = $getClassInfo->getProperty('hookWithImplicitParameter');
+        self::assertTrue($hookProperty->hasHook(ReflectionPropertyHookType::Set));
+
+        $hookReflection = $hookProperty->getHook(ReflectionPropertyHookType::Set);
+        self::assertInstanceOf(ReflectionType::class, $hookReflection->getReturnType());
+        self::assertSame('void', $hookReflection->getReturnType()->getName());
+
+        self::assertSame(1, $hookReflection->getNumberOfParameters());
+        self::assertInstanceOf(ReflectionParameter::class, $hookReflection->getParameter('value'));
+
+        $hookParameterReflection = $hookReflection->getParameter('value');
+        self::assertSame(0, $hookParameterReflection->getPosition());
+        self::assertFalse($hookParameterReflection->isOptional());
+        self::assertInstanceOf(ReflectionType::class, $hookParameterReflection->getType());
+        self::assertSame('string', $hookParameterReflection->getType()->getName());
+    }
+
+    public function testSetPropertyHookHasExplicitParameter(): void
+    {
+        $reflector    = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/PropertyHooks.php', $this->astLocator));
+        $getClassInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\SetPropertyHooksParameters');
+
+        $hookProperty = $getClassInfo->getProperty('hookWithExplicitParameter');
+        self::assertTrue($hookProperty->hasHook(ReflectionPropertyHookType::Set));
+
+        $hookReflection = $hookProperty->getHook(ReflectionPropertyHookType::Set);
+        self::assertInstanceOf(ReflectionType::class, $hookReflection->getReturnType());
+        self::assertSame('void', $hookReflection->getReturnType()->getName());
+
+        self::assertSame(1, $hookReflection->getNumberOfParameters());
+        self::assertInstanceOf(ReflectionParameter::class, $hookReflection->getParameter('value'));
+
+        $hookParameterReflection = $hookReflection->getParameter('value');
+        self::assertSame(0, $hookParameterReflection->getPosition());
+        self::assertFalse($hookParameterReflection->isOptional());
+        self::assertInstanceOf(ReflectionType::class, $hookParameterReflection->getType());
+        self::assertSame('int', $hookParameterReflection->getType()->getName());
+    }
+
+    /** @return list<array{0: non-empty-string, 1: string|null}> */
+    public static function getPropertyHookReturnTypeProvider(): array
+    {
+        return [
+            ['hookWithString', 'string'],
+            ['hookWithInteger', 'int'],
+            ['hookWithUnion', 'string|int'],
+            ['hookWithoutType', null],
+        ];
+    }
+
+    #[DataProvider('getPropertyHookReturnTypeProvider')]
+    public function testGetPropertyHookReturnType(string $propertyName, string|null $returnType): void
+    {
+        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/PropertyHooks.php', $this->astLocator));
+        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\GetPropertyHooksReturnTypes');
+
+        $hookProperty      = $classInfo->getProperty($propertyName);
+        $getHookReflection = $hookProperty->getHook(ReflectionPropertyHookType::Get);
+        self::assertNotNull($getHookReflection);
+        self::assertSame($returnType, $getHookReflection->getReturnType()?->__toString());
+    }
 }
diff --git a/test/unit/Reflection/ReflectionPropertyTest.php b/test/unit/Reflection/ReflectionPropertyTest.php
@@ -25,8 +25,10 @@
 use Roave\BetterReflection\Reflection\Exception\ObjectNotInstanceOfClass;
 use Roave\BetterReflection\Reflection\ReflectionClass;
 use Roave\BetterReflection\Reflection\ReflectionMethod;
+use Roave\BetterReflection\Reflection\ReflectionParameter;
 use Roave\BetterReflection\Reflection\ReflectionProperty;
 use Roave\BetterReflection\Reflection\ReflectionPropertyHookType;
+use Roave\BetterReflection\Reflection\ReflectionType;
 use Roave\BetterReflection\Reflector\DefaultReflector;
 use Roave\BetterReflection\Reflector\Reflector;
 use Roave\BetterReflection\SourceLocator\Ast\Locator;
@@ -1133,4 +1135,71 @@ public function testPromotedPropertyWithHooks(): void
         self::assertTrue($hookProperty->hasHook(ReflectionPropertyHookType::Get));
         self::assertFalse($hookProperty->hasHook(ReflectionPropertyHookType::Set));
     }
+
+    public function testSetPropertyHookHasImplicitParameter(): void
+    {
+        $reflector    = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/PropertyHooks.php', $this->astLocator));
+        $getClassInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\SetPropertyHooksParameters');
+
+        $hookProperty = $getClassInfo->getProperty('hookWithImplicitParameter');
+        self::assertTrue($hookProperty->hasHook(ReflectionPropertyHookType::Set));
+
+        $hookReflection = $hookProperty->getHook(ReflectionPropertyHookType::Set);
+        self::assertInstanceOf(ReflectionType::class, $hookReflection->getReturnType());
+        self::assertSame('void', $hookReflection->getReturnType()->getName());
+
+        self::assertSame(1, $hookReflection->getNumberOfParameters());
+        self::assertInstanceOf(ReflectionParameter::class, $hookReflection->getParameter('value'));
+
+        $hookParameterReflection = $hookReflection->getParameter('value');
+        self::assertSame(0, $hookParameterReflection->getPosition());
+        self::assertFalse($hookParameterReflection->isOptional());
+        self::assertInstanceOf(ReflectionType::class, $hookParameterReflection->getType());
+        self::assertSame('string', $hookParameterReflection->getType()->getName());
+    }
+
+    public function testSetPropertyHookHasExplicitParameter(): void
+    {
+        $reflector    = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/PropertyHooks.php', $this->astLocator));
+        $getClassInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\SetPropertyHooksParameters');
+
+        $hookProperty = $getClassInfo->getProperty('hookWithExplicitParameter');
+        self::assertTrue($hookProperty->hasHook(ReflectionPropertyHookType::Set));
+
+        $hookReflection = $hookProperty->getHook(ReflectionPropertyHookType::Set);
+        self::assertInstanceOf(ReflectionType::class, $hookReflection->getReturnType());
+        self::assertSame('void', $hookReflection->getReturnType()->getName());
+
+        self::assertSame(1, $hookReflection->getNumberOfParameters());
+        self::assertInstanceOf(ReflectionParameter::class, $hookReflection->getParameter('value'));
+
+        $hookParameterReflection = $hookReflection->getParameter('value');
+        self::assertSame(0, $hookParameterReflection->getPosition());
+        self::assertFalse($hookParameterReflection->isOptional());
+        self::assertInstanceOf(ReflectionType::class, $hookParameterReflection->getType());
+        self::assertSame('int', $hookParameterReflection->getType()->getName());
+    }
+
+    /** @return list<array{0: non-empty-string, 1: string|null}> */
+    public static function getPropertyHookReturnTypeProvider(): array
+    {
+        return [
+            ['hookWithString', 'string'],
+            ['hookWithInteger', 'int'],
+            ['hookWithUnion', 'string|int'],
+            ['hookWithoutType', null],
+        ];
+    }
+
+    #[DataProvider('getPropertyHookReturnTypeProvider')]
+    public function testGetPropertyHookReturnType(string $propertyName, string|null $returnType): void
+    {
+        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/PropertyHooks.php', $this->astLocator));
+        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\GetPropertyHooksReturnTypes');
+
+        $hookProperty      = $classInfo->getProperty($propertyName);
+        $getHookReflection = $hookProperty->getHook(ReflectionPropertyHookType::Get);
+        self::assertNotNull($getHookReflection);
+        self::assertSame($returnType, $getHookReflection->getReturnType()?->__toString());
+    }
 }