Merge pull request #1474 from kukulich/final-hook

Property hooks can be marked `final` - expose that in reflection API

Author: Ocramius

composer.json

@@ -6,7 +6,7 @@
         "php": "~8.2.0 || ~8.3.2 || ~8.4.1",
         "ext-json": "*",
         "jetbrains/phpstorm-stubs": "2024.3",
-        "nikic/php-parser": "^5.3.1"
+        "nikic/php-parser": "^5.4.0"
     },
     "authors": [
         {

composer.lock

@@ -166,6 +166,7 @@
       <code><![CDATA[classExists]]></code>
       <code><![CDATA[isAbstract]]></code>
       <code><![CDATA[isFinal]]></code>
+      <code><![CDATA[isFinal]]></code>
       <code><![CDATA[isPrivate]]></code>
       <code><![CDATA[isProtected]]></code>
       <code><![CDATA[isPublic]]></code>
@@ -209,7 +210,8 @@
       <code><![CDATA[getEndLine]]></code>
       <code><![CDATA[getStartLine]]></code>
       <code><![CDATA[getStmts]]></code>
-      <code><![CDATA[getStmts]]></code>
+      <code><![CDATA[isAbstract]]></code>
+      <code><![CDATA[isFinal]]></code>
       <code><![CDATA[isPrivate]]></code>
       <code><![CDATA[isPrivateSet]]></code>
       <code><![CDATA[isProtected]]></code>

psalm-baseline.xml

@@ -53,7 +53,7 @@ private function __construct(
         assert($node instanceof MethodNode || $node instanceof Node\PropertyHook);
 
         $this->name      = $name;
-        $this->modifiers = $node instanceof MethodNode ? $this->computeModifiers($node) : 0;
+        $this->modifiers = $this->computeModifiers($node);
 
         $this->fillFromNode($node);
     }
@@ -305,13 +305,18 @@ public function getModifiers(): int
     }
 
     /** @return int-mask-of<ReflectionMethodAdapter::IS_*> */
-    private function computeModifiers(MethodNode $node): int
+    private function computeModifiers(MethodNode|Node\PropertyHook $node): int
     {
-        $modifiers  = $node->isStatic() ? CoreReflectionMethod::IS_STATIC : 0;
-        $modifiers += $node->isPublic() ? CoreReflectionMethod::IS_PUBLIC : 0;
-        $modifiers += $node->isProtected() ? CoreReflectionMethod::IS_PROTECTED : 0;
-        $modifiers += $node->isPrivate() ? CoreReflectionMethod::IS_PRIVATE : 0;
-        $modifiers += $node->isAbstract() ? CoreReflectionMethod::IS_ABSTRACT : 0;
+        $modifiers = 0;
+
+        if ($node instanceof MethodNode) {
+            $modifiers += $node->isStatic() ? CoreReflectionMethod::IS_STATIC : 0;
+            $modifiers += $node->isPublic() ? CoreReflectionMethod::IS_PUBLIC : 0;
+            $modifiers += $node->isProtected() ? CoreReflectionMethod::IS_PROTECTED : 0;
+            $modifiers += $node->isPrivate() ? CoreReflectionMethod::IS_PRIVATE : 0;
+            $modifiers += $node->isAbstract() ? CoreReflectionMethod::IS_ABSTRACT : 0;
+        }
+
         $modifiers += $node->isFinal() ? CoreReflectionMethod::IS_FINAL : 0;
 
         return $modifiers;

src/Reflection/ReflectionMethod.php

@@ -7,7 +7,6 @@
 use Closure;
 use Error;
 use OutOfBoundsException;
-use PhpParser\Modifiers;
 use PhpParser\Node;
 use PhpParser\Node\Stmt\Property as PropertyNode;
 use PhpParser\NodeTraverser;
@@ -35,7 +34,6 @@
 
 use function array_map;
 use function assert;
-use function count;
 use function func_num_args;
 use function is_object;
 use function sprintf;
@@ -675,8 +673,8 @@ private function computeModifiers(PropertyNode $node): int
         $modifiers += $node->isProtected() ? CoreReflectionProperty::IS_PROTECTED : 0;
         $modifiers += $node->isProtectedSet() ? ReflectionPropertyAdapter::IS_PROTECTED_SET_COMPATIBILITY : 0;
         $modifiers += $node->isPublic() ? CoreReflectionProperty::IS_PUBLIC : 0;
-        $modifiers += ($node->flags & ReflectionPropertyAdapter::IS_FINAL_COMPATIBILITY) === ReflectionPropertyAdapter::IS_FINAL_COMPATIBILITY ? ReflectionPropertyAdapter::IS_FINAL_COMPATIBILITY : 0;
-        $modifiers += ($node->flags & Modifiers::ABSTRACT) === Modifiers::ABSTRACT ? ReflectionPropertyAdapter::IS_ABSTRACT_COMPATIBILITY : 0;
+        $modifiers += $node->isFinal() ? ReflectionPropertyAdapter::IS_FINAL_COMPATIBILITY : 0;
+        $modifiers += $node->isAbstract() ? ReflectionPropertyAdapter::IS_ABSTRACT_COMPATIBILITY : 0;
 
         /** @phpstan-ignore return.type */
         return $modifiers;
@@ -699,51 +697,29 @@ private function computeImmediateVirtual(PropertyNode $node): bool
             }
         }
 
-        if ($setHook !== null && ! $this->computeImmediateVirtualBasedOnSetHook($setHook)) {
+        if ($setHook !== null && ! $this->computeImmediateVirtualBasedOnHook($setHook)) {
             return false;
         }
 
         if ($getHook === null) {
             return true;
         }
 
-        return $this->computeImmediateVirtualBasedOnGetHook($getHook);
+        return $this->computeImmediateVirtualBasedOnHook($getHook);
     }
 
-    private function computeImmediateVirtualBasedOnGetHook(Node\PropertyHook $getHook): bool
+    private function computeImmediateVirtualBasedOnHook(Node\PropertyHook $hook): bool
     {
-        $getHookBody = $getHook->getStmts();
+        $hookBody = $hook->getStmts();
 
         // Abstract property or property in interface
-        if ($getHookBody === null) {
+        if ($hookBody === null) {
             return true;
         }
 
-        return ! $this->isHookUsingThisProperty($getHook);
-    }
-
-    private function computeImmediateVirtualBasedOnSetHook(Node\PropertyHook $setHook): bool
-    {
-        $setHookBody = $setHook->getStmts();
-
-        // Abstract property or property in interface
-        if ($setHookBody === null) {
-            return true;
-        }
-
-        // Short syntax
-        if (count($setHookBody) === 1 && $setHookBody[0] instanceof Node\Stmt\Return_) {
-            return false;
-        }
-
-        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([$hook]);
+        $traverser->traverse($hookBody);
 
         foreach ($visitor->getFoundNodes() as $propertyFetchNode) {
             assert($propertyFetchNode instanceof Node\Expr\PropertyFetch);
@@ -754,11 +730,11 @@ private function isHookUsingThisProperty(Node\PropertyHook $hook): bool
                 && $propertyFetchNode->name instanceof Node\Identifier
                 && $propertyFetchNode->name->name === $this->name
             ) {
-                return true;
+                return false;
             }
         }
 
-        return false;
+        return true;
     }
 
     /** @return array{get?: ReflectionMethod, set?: ReflectionMethod} */

src/Reflection/ReflectionProperty.php

@@ -153,3 +153,14 @@ class GetPropertyHooksReturnTypes
     public $hookWithoutType { get => 'string'; }
 
 }
+
+class FinalPropertyHooks
+{
+    public string $notFinalHook {
+        set => strtolower($value);
+    }
+
+    public string $finalHook {
+        final set => strtolower($value);
+    }
+}

test/unit/Fixture/PropertyHooks.php

@@ -882,4 +882,25 @@ public function testGetPropertyHookReturnType(string $propertyName, string|null
         self::assertNotNull($getHookReflection);
         self::assertSame($returnType, $getHookReflection->getReturnType()?->__toString());
     }
+
+    /** @return list<array{0: non-empty-string, 1: bool}> */
+    public static function finalPropertyHookProvider(): array
+    {
+        return [
+            ['notFinalHook', false],
+            ['finalHook', true],
+        ];
+    }
+
+    #[DataProvider('finalPropertyHookProvider')]
+    public function testFinalPropertyHook(string $propertyName, bool $isFinal): void
+    {
+        $reflector = new DefaultReflector(new SingleFileSourceLocator(__DIR__ . '/../Fixture/PropertyHooks.php', $this->astLocator));
+        $classInfo = $reflector->reflectClass('Roave\BetterReflectionTest\Fixture\FinalPropertyHooks');
+
+        $hookProperty   = $classInfo->getProperty($propertyName);
+        $hookReflection = $hookProperty->getHook(ReflectionPropertyHookType::Set);
+        self::assertNotNull($hookReflection);
+        self::assertSame($isFinal, $hookReflection->isFinal());
+    }
 }