Merge pull request #1485 from ondrejmirtes/trait-method-alias-test

Fixed modifiers of trait method when method does not exist in current class
Roave · Feb 9, 2025 · 87fc501 · 87fc501
2 parents 8d26665 + c3fa1cb
commit 87fc501
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 13 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.26.1@d747f6500b38ac4f7dfc5edbcae6e4b637d7add0">
+<files psalm-version="6.4.0@04f312ac6ea48ba1c3e5db4d815bf6d74641c0ee">
   <file src="src/Reflection/Attribute/ReflectionAttributeHelper.php">
     <ImpureMethodCall>
       <code><![CDATA[toLowerString]]></code>
@@ -32,8 +32,8 @@
             [$valueParameter],
             new Node\NullableType(new Node\Identifier('static')),
         )]]></code>
-      <code><![CDATA[$createMethod($method->getAliasName())]]></code>
-      <code><![CDATA[$createMethod($traitAliasDefinition['alias'])]]></code>
+      <code><![CDATA[$createMethod($method->getAliasName(), $modifiersUsedWithAlias ? $method->getModifiers() : $methodModifiers)]]></code>
+      <code><![CDATA[$createMethod($traitAliasDefinition['alias'], $methodModifiers)]]></code>
       <code><![CDATA[$createMethod('cases', [], new Node\Identifier('array'))]]></code>
       <code><![CDATA[$createProperty('name', new Node\Identifier('string'))]]></code>
       <code><![CDATA[$createProperty('name', new Node\Identifier('string'))]]></code>

diff --git a/src/Reflection/ReflectionClass.php b/src/Reflection/ReflectionClass.php
@@ -360,8 +360,12 @@ public function getExtensionName(): string|null
         return $this->locatedSource->getExtensionName();
     }
 
-    /** @return list<ReflectionMethod> */
-    private function createMethodsFromTrait(ReflectionMethod $method): array
+    /**
+     * @param array<lowercase-string, ReflectionMethod> $currentMethods
+     *
+     * @return list<ReflectionMethod>
+     */
+    private function createMethodsFromTrait(ReflectionMethod $method, array $currentMethods): array
     {
         $methodModifiers      = $method->getModifiers();
         $lowerCasedMethodHash = $this->lowerCasedMethodHash($method->getImplementingClass()->getName(), $method->getName());
@@ -377,17 +381,35 @@ private function createMethodsFromTrait(ReflectionMethod $method): array
             }
         }
 
-        $createMethod = function (string|null $aliasMethodName) use ($method, $methodModifiers): ReflectionMethod {
+        $createMethod = function (string|null $aliasMethodName, int $methodModifiers) use ($method): ReflectionMethod {
             assert($aliasMethodName === null || $aliasMethodName !== '');
 
-            /** @phpstan-ignore argument.type */
+            /** @var int-mask-of<ReflectionMethodAdapter::IS_*> $methodModifiers */
+            $methodModifiers = $methodModifiers;
+
             return $method->withImplementingClass($this, $aliasMethodName, $methodModifiers);
         };
 
         $methods = [];
 
-        if (! array_key_exists($lowerCasedMethodHash, $this->traitsData['precedences'])) {
-            $methods[] = $createMethod($method->getAliasName());
+        if (
+            ! array_key_exists($lowerCasedMethodHash, $this->traitsData['precedences'])
+            && ! array_key_exists($lowerCasedMethodHash, $currentMethods)
+        ) {
+            $modifiersUsedWithAlias = false;
+
+            foreach ($this->traitsData['aliases'] as $traitAliasDefinitions) {
+                foreach ($traitAliasDefinitions as $traitAliasDefinition) {
+                    if ($lowerCasedMethodHash === $traitAliasDefinition['hash']) {
+                        $modifiersUsedWithAlias = true;
+                        break;
+                    }
+                }
+            }
+
+            // Modifiers used with alias -> copy method with original modifiers (will be added later with the alias name and new modifiers)
+            // Modifiers not used with alias -> add method with new modifiers
+            $methods[] = $createMethod($method->getAliasName(), $modifiersUsedWithAlias ? $method->getModifiers() : $methodModifiers);
         }
 
         if ($this->traitsData['aliases'] !== []) {
@@ -403,7 +425,7 @@ private function createMethodsFromTrait(ReflectionMethod $method): array
                         continue;
                     }
 
-                    $methods[] = $createMethod($traitAliasDefinition['alias']);
+                    $methods[] = $createMethod($traitAliasDefinition['alias'], $methodModifiers);
                 }
             }
         }
@@ -455,7 +477,7 @@ private function getMethodsIndexedByLowercasedName(AlreadyVisitedClasses $alread
         foreach ($this->getTraits() as $trait) {
             $alreadyVisitedClassesCopy = clone $alreadyVisitedClasses;
             foreach ($trait->getMethodsIndexedByLowercasedName($alreadyVisitedClassesCopy) as $method) {
-                foreach ($this->createMethodsFromTrait($method) as $traitMethod) {
+                foreach ($this->createMethodsFromTrait($method, $methods) as $traitMethod) {
                     $lowercasedMethodName = strtolower($traitMethod->getName());
 
                     if (! array_key_exists($lowercasedMethodName, $methods)) {

diff --git a/test/unit/Reflection/ReflectionClassTest.php b/test/unit/Reflection/ReflectionClassTest.php
@@ -2646,8 +2646,8 @@ class Foo
         self::assertTrue($protectedMethodFromTrait->isProtected());
 
         $privateMethodFromClass = $classReflection->getMethod('privateMethod');
-        self::assertTrue($privateMethodFromClass->isProtected());
-        self::assertFalse($privateMethodFromClass->isPrivate());
+        self::assertFalse($privateMethodFromClass->isProtected());
+        self::assertTrue($privateMethodFromClass->isPrivate());
 
         $privateMethodFromTrait = $traitReflection->getMethod('privateMethod');
         self::assertFalse($privateMethodFromTrait->isProtected());
@@ -3105,4 +3105,68 @@ interface EntityInterface extends AccessibleInterface, CacheableDependencyInterf
 
         self::assertSame($expectedInterfaceNames, $classReflection->getInterfaceNames());
     }
+
+    public function testTraitAliasCopiesMethod(): void
+    {
+        // runtime proof https://3v4l.org/7R74v
+
+        $php = <<<'PHP'
+<?php
+trait Foo {
+    public function hello(): string
+    {
+        return "Hello from Foo!";
+    }
+}
+
+class Bar {
+    use Foo {
+        hello as private somethingElse;
+    }
+}
+PHP;
+
+        $classInfo = (new DefaultReflector(new StringSourceLocator($php, $this->astLocator)))->reflectClass('Bar');
+
+        self::assertTrue($classInfo->hasMethod('hello'));
+        $hello = $classInfo->getMethod('hello');
+        self::assertTrue($hello->isPublic());
+
+        self::assertTrue($classInfo->hasMethod('somethingElse'));
+        $somethingElse = $classInfo->getMethod('somethingElse');
+        self::assertTrue($somethingElse->isPrivate());
+    }
+
+    public function testTraitAliasCopiesMethodUnlessMethodWithSameNameAlreadyInClass(): void
+    {
+        // runtime proof https://3v4l.org/WbpSi
+        $php = <<<'PHP'
+<?php
+trait Foo {
+    public function hello(): string
+    {
+        return "Hello from Foo!";
+    }
+}
+
+class Bar {
+    use Foo {
+        hello as private somethingElse;
+    }
+
+    protected function hello(int $i): void
+    {
+    }
+}
+PHP;
+
+        $classInfo = (new DefaultReflector(new StringSourceLocator($php, $this->astLocator)))->reflectClass('Bar');
+
+        self::assertTrue($classInfo->hasMethod('hello'));
+        self::assertTrue($classInfo->hasMethod('somethingElse'));
+
+        $hello = $classInfo->getMethod('hello');
+        self::assertTrue($hello->isProtected());
+        self::assertCount(1, $hello->getParameters());
+    }
 }