Repository: Roave/BetterReflection (php)

Issue:
Trait method aliases broken
Hi,
consider this example: https://3v4l.org/a2hQf

Two observations from that:

1) Aliased trait method fetched on another trait should have the aliased name, not the original name.
2) Class that uses a trait with an aliased method should also know the aliased method.

Unfortunately, BetterReflection breaks both of those assumptions. Here's a reproducing example: https://gist.github.com/ondrejmirtes/a8481b8e03880aebd4e0bae5c6d87e86

Pull request:
Fixed many traits bugs
Fixes #577 

Patch:
diff --git a/src/Reflection/ReflectionClass.php b/src/Reflection/ReflectionClass.php
index 1f07b38b..1d36c699 100644
--- a/src/Reflection/ReflectionClass.php
+++ b/src/Reflection/ReflectionClass.php
@@ -38,6 +38,7 @@ use Roave\BetterReflection\Util\GetFirstDocComment;
 use Traversable;
 use function array_combine;
 use function array_filter;
+use function array_key_exists;
 use function array_map;
 use function array_merge;
 use function array_reverse;
@@ -92,6 +93,12 @@ class ReflectionClass implements Reflection
      */
     private $cachedMethods;
 
+    /** @var array<string, string>|null */
+    private $cachedTraitAliases;
+
+    /** @var array<string, string>|null */
+    private $cachedTraitPrecedences;
+
     private function __construct()
     {
     }
@@ -252,18 +259,15 @@ class ReflectionClass implements Reflection
             ),
             ...array_map(
                 function (ReflectionClass $trait) : array {
-                    return array_map(function (ReflectionMethod $method) use ($trait) : ReflectionMethod {
-                        $methodAst = $method->getAst();
-                        assert($methodAst instanceof ClassMethod);
-
-                        return ReflectionMethod::createFromNode(
-                            $this->reflector,
-                            $methodAst,
-                            $this->declaringNamespace,
-                            $trait,
-                            $this
-                        );
-                    }, $trait->getMethods());
+                    return array_merge(
+                        [],
+                        ...array_map(
+                            function (ReflectionMethod $method) : array {
+                                return $this->createMethodsFromTrait($method);
+                            },
+                            $trait->getMethods()
+                        )
+                    );
                 },
                 $this->getTraits()
             ),
@@ -279,6 +283,55 @@ class ReflectionClass implements Reflection
         );
     }
 
+    /**
+     * @return ReflectionMethod[]
+     */
+    private function createMethodsFromTrait(ReflectionMethod $method) : array
+    {
+        $traitAliases     = $this->getTraitAliases();
+        $traitPrecedences = $this->getTraitPrecedences();
+
+        $methodAst = $method->getAst();
+        assert($methodAst instanceof ClassMethod);
+
+        $methodHash = $this->methodHash($method->getDeclaringClass()->getName(), $method->getName());
+
+        $aliases = [];
+        foreach ($traitAliases as $aliasMethodName => $traitAliasDefinition) {
+            if ($methodHash !== $traitAliasDefinition) {
+                continue;
+            }
+
+            $aliases[] = ReflectionMethod::createFromNode(
+                $this->reflector,
+                $methodAst,
+                $method->getDeclaringClass()->getDeclaringNamespaceAst(),
+                $method->getDeclaringClass(),
+                $this,
+                $aliasMethodName
+            );
+        }
+
+        if ($aliases !== []) {
+            return $aliases;
+        }
+
+        if (array_key_exists($methodHash, $traitPrecedences)) {
+            return [];
+        }
+
+        $newMethod = ReflectionMethod::createFromNode(
+            $this->reflector,
+            $methodAst,
+            $method->getDeclaringClass()->getDeclaringNamespaceAst(),
+            $method->getDeclaringClass(),
+            $this,
+            $method->getAliasName()
+        );
+
+        return [$newMethod];
+    }
+
     /**
      * Construct a flat list of methods that are available. This will search up
      * all parent classes/traits/interfaces/current scope for methods.
@@ -298,23 +351,8 @@ class ReflectionClass implements Reflection
 
         $cachedMethods = [];
 
-        $traitAliases = $this->getTraitAliases();
-
         foreach ($this->getAllMethods() as $method) {
-            $methodName              = $method->getName();
-            $methodNameWithClassName = sprintf('%s::%s', $method->getDeclaringClass()->getName(), $methodName);
-
-            foreach ($traitAliases as $methodAlias => $traitMethodNameWithTraitName) {
-                if ($methodNameWithClassName !== $traitMethodNameWithTraitName) {
-                    continue;
-                }
-
-                if (isset($cachedMethods[$methodAlias])) {
-                    continue;
-                }
-
-                $cachedMethods[$methodAlias] = $method;
-            }
+            $methodName = $method->getName();
 
             if (isset($cachedMethods[$methodName])) {
                 continue;
@@ -984,12 +1022,52 @@ class ReflectionClass implements Reflection
      */
     public function getTraitAliases() : array
     {
+        $this->parseTraitUsages();
+
+        /** @return array<string, string> */
+        return $this->cachedTraitAliases;
+    }
+
+    /**
+     * Return a list of the precedences used when importing traits for this class.
+     * The returned array is in key/value pair in this format:.
+     *
+     *   'Class::method' => 'Class::method'
+     *
+     * @return array<string, string>
+     *
+     * @example
+     * // When reflecting a class such as:
+     * class Foo
+     * {
+     *     use MyTrait, MyTrait2 {
+     *         MyTrait2::foo insteadof MyTrait1;
+     *     }
+     * }
+     * // This method would return
+     * //   ['MyTrait1::foo' => 'MyTrait2::foo']
+     */
+    private function getTraitPrecedences() : array
+    {
+        $this->parseTraitUsages();
+
+        /** @return array<string, string> */
+        return $this->cachedTraitPrecedences;
+    }
+
+    private function parseTraitUsages() : void
+    {
+        if ($this->cachedTraitAliases !== null && $this->cachedTraitPrecedences !== null) {
+            return;
+        }
+
         /** @var Node\Stmt\TraitUse[] $traitUsages */
         $traitUsages = array_filter($this->node->stmts, static function (Node $node) : bool {
             return $node instanceof TraitUse;
         });
 
-        $resolvedAliases = [];
+        $this->cachedTraitAliases     = [];
+        $this->cachedTraitPrecedences = [];
 
         foreach ($traitUsages as $traitUsage) {
             $traitNames  = $traitUsage->traits;
@@ -1001,23 +1079,35 @@ class ReflectionClass implements Reflection
                     $usedTrait = $traitNames[0];
                 }
 
-                if (! ($adaptation instanceof Node\Stmt\TraitUseAdaptation\Alias)) {
+                if ($adaptation instanceof Node\Stmt\TraitUseAdaptation\Alias && $adaptation->newName) {
+                    $this->cachedTraitAliases[$adaptation->newName->name] = $this->methodHash($usedTrait->toString(), $adaptation->method->toString());
                     continue;
                 }
 
-                if (! $adaptation->newName) {
+                if (! $adaptation instanceof Node\Stmt\TraitUseAdaptation\Precedence || ! $adaptation->insteadof) {
                     continue;
                 }
 
-                $resolvedAliases[$adaptation->newName->name] = sprintf(
-                    '%s::%s',
-                    $usedTrait->toString(),
-                    $adaptation->method->toString()
-                );
+                foreach ($adaptation->insteadof as $insteadof) {
+                    $adaptationNameHash = $this->methodHash($insteadof->toString(), $adaptation->method->toString());
+                    $originalNameHash   = $this->methodHash($usedTrait->toString(), $adaptation->method->toString());
+
+                    $this->cachedTraitPrecedences[$adaptationNameHash] = $originalNameHash;
+                }
             }
         }
+    }
 
-        return $resolvedAliases;
+    /**
+     * @psalm-pure
+     */
+    private function methodHash(string $className, string $methodName) : string
+    {
+        return sprintf(
+            '%s::%s',
+            $className,
+            $methodName
+        );
     }
 
     /**
diff --git a/src/Reflection/ReflectionMethod.php b/src/Reflection/ReflectionMethod.php
index 323a8217..687fec33 100644
--- a/src/Reflection/ReflectionMethod.php
+++ b/src/Reflection/ReflectionMethod.php
@@ -36,6 +36,9 @@ class ReflectionMethod extends ReflectionFunctionAbstract
     /** @var MethodNode */
     private $methodNode;
 
+    /** @var string|null */
+    private $aliasName;
+
     /**
      * @internal
      *
@@ -48,12 +51,14 @@ class ReflectionMethod extends ReflectionFunctionAbstract
         MethodNode $node,
         ?Namespace_ $namespace,
         ReflectionClass $declaringClass,
-        ReflectionClass $implementingClass
+        ReflectionClass $implementingClass,
+        ?string $aliasName = null
     ) : self {
         $method                    = new self();
         $method->declaringClass    = $declaringClass;
         $method->implementingClass = $implementingClass;
         $method->methodNode        = $node;
+        $method->aliasName         = $aliasName;
 
         $method->populateFunctionAbstract($reflector, $node, $declaringClass->getLocatedSource(), $namespace);
 
@@ -86,6 +91,20 @@ class ReflectionMethod extends ReflectionFunctionAbstract
         return ReflectionClass::createFromInstance($instance)->getMethod($methodName);
     }
 
+    public function getShortName() : string
+    {
+        if ($this->aliasName !== null) {
+            return $this->aliasName;
+        }
+
+        return parent::getShortName();
+    }
+
+    public function getAliasName() : ?string
+    {
+        return $this->aliasName;
+    }
+
     /**
      * Find the prototype for this method, if it exists. If it does not exist
      * it will throw a MethodPrototypeNotFound exception.


Test patch:
diff --git a/test/unit/Fixture/TraitFixture.php b/test/unit/Fixture/TraitFixture.php
index d34d2e11..b3d8b665 100644
--- a/test/unit/Fixture/TraitFixture.php
+++ b/test/unit/Fixture/TraitFixture.php
@@ -10,6 +10,16 @@ class TraitFixtureA
     use TraitFixtureTraitA;
 }
 
+trait TraitFixtureTraitAA
+{
+    use TraitFixtureTraitA;
+}
+
+class TraitFixtureAA
+{
+    use TraitFixtureTraitAA;
+}
+
 // No trait usage
 class TraitFixtureB
 {
@@ -43,7 +53,7 @@ trait TraitFixtureTraitD2
 }
 class TraitFixtureD
 {
-    use TraitFixtureTraitD1, TraitFixtureTraitD2 {
+    use TraitFixtureTraitD2, TraitFixtureTraitD1 {
         TraitFixtureTraitD1::foo insteadof TraitFixtureTraitD2;
         TraitFixtureTraitD1::hoo as hooFirstAlias;
         TraitFixtureTraitD1::hoo as hooSecondAlias;
@@ -51,3 +61,64 @@ class TraitFixtureD
 
     public function boo() {}
 }
+
+trait FirstTraitForFixtureE
+{
+    public function foo(): void
+    {
+    }
+}
+
+trait SecondTraitForFixtureE
+{
+    use FirstTraitForFixtureE {
+        foo as parentFoo;
+    }
+
+    public function foo(): void
+    {
+    }
+}
+
+class TraitFixtureE
+{
+    use SecondTraitForFixtureE;
+}
+
+trait FirstTraitForFixtureF
+{
+
+    public function a()
+    {
+    }
+
+    public function b()
+    {
+    }
+
+}
+
+trait SecondTraitForFixtureF
+{
+
+    public function a()
+    {
+    }
+
+    public function b()
+    {
+    }
+
+}
+
+class TraitFixtureF
+{
+
+    use FirstTraitForFixtureF, SecondTraitForFixtureF {
+        FirstTraitForFixtureF::a insteadof SecondTraitForFixtureF;
+        SecondTraitForFixtureF::b insteadof FirstTraitForFixtureF;
+        SecondTraitForFixtureF::a as aliasedA;
+        FirstTraitForFixtureF::b as aliasedB;
+    }
+
+}
diff --git a/test/unit/Reflection/ReflectionClassTest.php b/test/unit/Reflection/ReflectionClassTest.php
index c637c77e..9a02929a 100644
--- a/test/unit/Reflection/ReflectionClassTest.php
+++ b/test/unit/Reflection/ReflectionClassTest.php
@@ -891,6 +891,19 @@ PHP;
         self::assertTrue($traits[0]->isTrait());
     }
 
+    public function testGetDeclaringClassForTraits() : void
+    {
+        $reflector = new ClassReflector(new SingleFileSourceLocator(
+            __DIR__ . '/../Fixture/TraitFixture.php',
+            $this->astLocator
+        ));
+
+        $classInfo = $reflector->reflect('TraitFixtureAA');
+
+        self::assertTrue($classInfo->hasMethod('foo'));
+        self::assertSame('TraitFixtureTraitA', $classInfo->getMethod('foo')->getDeclaringClass()->getName());
+    }
+
     public function testGetTraitsReturnsEmptyArrayWhenNoTraitsUsed() : void
     {
         $reflector = new ClassReflector(new SingleFileSourceLocator(
@@ -941,26 +954,18 @@ PHP;
 
         $classInfo = $reflector->reflect('TraitFixtureC');
 
-        self::assertTrue($classInfo->hasMethod('a'));
+        self::assertFalse($classInfo->hasMethod('a'));
         self::assertTrue($classInfo->hasMethod('a_protected'));
 
-        $a          = $classInfo->getMethod('a');
         $aProtected = $classInfo->getMethod('a_protected');
 
-        self::assertSame('a', $a->getName());
-        self::assertSame($a->getName(), $aProtected->getName());
-        self::assertSame('TraitFixtureTraitC', $a->getDeclaringClass()->getName());
         self::assertSame('TraitFixtureTraitC', $aProtected->getDeclaringClass()->getName());
 
-        self::assertTrue($classInfo->hasMethod('b'));
+        self::assertFalse($classInfo->hasMethod('b'));
         self::assertTrue($classInfo->hasMethod('b_renamed'));
 
-        $b        = $classInfo->getMethod('b');
         $bRenamed = $classInfo->getMethod('b_renamed');
 
-        self::assertSame('b', $b->getName());
-        self::assertSame($b->getName(), $bRenamed->getName());
-        self::assertSame('TraitFixtureTraitC', $b->getDeclaringClass()->getName());
         self::assertSame('TraitFixtureTraitC', $bRenamed->getDeclaringClass()->getName());
 
         self::assertTrue($classInfo->hasMethod('c'));
@@ -984,28 +989,76 @@ PHP;
         self::assertSame('TraitFixtureD', $classInfo->getMethod('boo')->getDeclaringClass()->getName());
 
         self::assertTrue($classInfo->hasMethod('foo'));
+        self::assertSame('TraitFixtureTraitD1', $classInfo->getMethod('foo')->getDeclaringClass()->getName());
 
         $foo = $classInfo->getMethod('foo');
 
         self::assertSame('TraitFixtureTraitD1', $foo->getDeclaringClass()->getName());
         self::assertSame('TraitFixtureD', $foo->getImplementingClass()->getName());
 
-        self::assertTrue($classInfo->hasMethod('hoo'));
+        self::assertFalse($classInfo->hasMethod('hoo'));
         self::assertTrue($classInfo->hasMethod('hooFirstAlias'));
         self::assertTrue($classInfo->hasMethod('hooSecondAlias'));
 
-        $hoo            = $classInfo->getMethod('hoo');
         $hooFirstAlias  = $classInfo->getMethod('hooFirstAlias');
         $hooSecondAlias = $classInfo->getMethod('hooSecondAlias');
 
-        self::assertSame('TraitFixtureTraitD1', $hoo->getDeclaringClass()->getName());
         self::assertSame('TraitFixtureTraitD1', $hooFirstAlias->getDeclaringClass()->getName());
         self::assertSame('TraitFixtureTraitD1', $hooSecondAlias->getDeclaringClass()->getName());
-        self::assertSame('TraitFixtureD', $hoo->getImplementingClass()->getName());
         self::assertSame('TraitFixtureD', $hooFirstAlias->getImplementingClass()->getName());
         self::assertSame('TraitFixtureD', $hooSecondAlias->getImplementingClass()->getName());
     }
 
+    public function testMethodsFromTraitsWithAliases() : void
+    {
+        $reflector = new ClassReflector(new SingleFileSourceLocator(
+            __DIR__ . '/../Fixture/TraitFixture.php',
+            $this->astLocator
+        ));
+
+        $classInfo = $reflector->reflect('TraitFixtureE');
+
+        self::assertTrue($classInfo->hasMethod('foo'));
+        self::assertSame('foo', $classInfo->getMethod('foo')->getName());
+        self::assertTrue($classInfo->hasMethod('parentFoo'));
+        self::assertSame('parentFoo', $classInfo->getMethod('parentFoo')->getName());
+
+        $traitInfo = $reflector->reflect('SecondTraitForFixtureE');
+
+        self::assertTrue($traitInfo->hasMethod('foo'));
+        self::assertSame('foo', $traitInfo->getMethod('foo')->getName());
+        self::assertTrue($traitInfo->hasMethod('parentFoo'));
+        self::assertSame('parentFoo', $traitInfo->getMethod('parentFoo')->getName());
+    }
+
+    public function testMethodsFromTraitsWithAliasesAndConflicts() : void
+    {
+        $reflector = new ClassReflector(new SingleFileSourceLocator(
+            __DIR__ . '/../Fixture/TraitFixture.php',
+            $this->astLocator
+        ));
+
+        $classInfo = $reflector->reflect('TraitFixtureF');
+
+        self::assertTrue($classInfo->hasMethod('a'));
+        self::assertTrue($classInfo->hasMethod('aliasedA'));
+
+        $a        = $classInfo->getMethod('a');
+        $aliasedA = $classInfo->getMethod('aliasedA');
+
+        self::assertSame('FirstTraitForFixtureF', $a->getDeclaringClass()->getName());
+        self::assertSame('SecondTraitForFixtureF', $aliasedA->getDeclaringClass()->getName());
+
+        self::assertTrue($classInfo->hasMethod('b'));
+        self::assertTrue($classInfo->hasMethod('aliasedB'));
+
+        $b        = $classInfo->getMethod('b');
+        $aliasedB = $classInfo->getMethod('aliasedB');
+
+        self::assertSame('SecondTraitForFixtureF', $b->getDeclaringClass()->getName());
+        self::assertSame('FirstTraitForFixtureF', $aliasedB->getDeclaringClass()->getName());
+    }
+
     public function testGetInterfaceNames() : void
     {
         $reflector = new ClassReflector(new SingleFileSourceLocator(