Repository: Roave/BetterReflection (php)

Issue:
ReflectionClass - abstract trait method should not override concrete method from parent class
(Yeah, I want to kill myself because of PHP and traits too.)

Consider this code (https://3v4l.org/GNnML):

```php
abstract class AbstractFoo
{

	public function myFoo(): void
	{
            var_dump('foo');
	}
	
	public function myBar(): void
	{
	    var_dump('bar from class');
	}

}

trait TraitFoo
{

	abstract public function myFoo(): void;
	
	public function myBar(): void
	{
	    var_dump('bar from trait');
	}

}

final class ClassFoo extends AbstractFoo
{

	use TraitFoo;

}

$foo = new ClassFoo();
$foo->myFoo(); // prints "foo"
$foo->myBar(); // prints "bar from trait"
```

PHP reflection matches that:

```php
$reflectionClass = new \ReflectionClass(\ClassFoo::class);
var_dump($reflectionClass->getMethod('myFoo')->getDeclaringClass()->getName()); // AbstractFoo
var_dump($reflectionClass->getMethod('myBar')->getDeclaringClass()->getName()); // ClassFoo
```

Unfortunately BetterReflection thinks that the implementing class of `myFoo` (declaring class from the Adapter point of view) is ClassFoo and that the method is abstract and therefore PHPStan considered it not implemented.

PoC fix: https://github.com/ondrejmirtes/BetterReflection/commit/aac16f0448261958468f0b3be803cb86ddb6d7a3

Original issue: https://github.com/phpstan/phpstan/issues/3406
Trait method aliases don't work in a simple scenario without insteadof
See this example:

```php
trait Foo {
	public function bar(): void {

	}
}

class SomeClass {
	use Foo {
		bar as baz;
	}
}
```

BetterReflection reports that `SomeClass::baz()` does not exist.

Additionally, `SomeClass::bar()` also exists and it doesn't seem like a PHP bug. The manual says (https://www.php.net/manual/en/language.oop5.traits.php):

> The Aliased_Talker makes use of the as operator to be able to use B's bigTalk implementation under an additional alias talk. 

Runtime proof: https://3v4l.org/kBjVh
Reproduction: https://gist.github.com/ondrejmirtes/a18fc61b29d2319beb792ec5252b1939

_Silly me 2 days ago thought that https://github.com/Roave/BetterReflection/issues/647 was bad enough 🤦_ 

Pull request:
Corrected trait analysis: method ordering, abstract trait methods do not override concrete ones, method aliases now recognized
Fixes #647 
Fixes #659

Also fixes methods order: https://3v4l.org/LiTi4

Patch:
diff --git a/src/Reflection/ReflectionClass.php b/src/Reflection/ReflectionClass.php
index 62ad0acb..2f81d120 100644
--- a/src/Reflection/ReflectionClass.php
+++ b/src/Reflection/ReflectionClass.php
@@ -81,6 +81,8 @@ class ReflectionClass implements Reflection
     /** @var array<string, string>|null */
     private ?array $cachedTraitPrecedences = null;
 
+    private ?ReflectionClass $cachedParentClass = null;
+
     private function __construct()
     {
     }
@@ -206,27 +208,67 @@ class ReflectionClass implements Reflection
     }
 
     /**
-     * Construct a flat list of all methods from current class, traits,
-     * parent classes and interfaces in this precise order.
-     *
      * @return ReflectionMethod[]
      */
-    private function getAllMethods() : array
+    private function createMethodsFromTrait(ReflectionMethod $method) : array
+    {
+        $traitAliases     = $this->getTraitAliases();
+        $traitPrecedences = $this->getTraitPrecedences();
+
+        $methodAst = $method->getAst();
+        assert($methodAst instanceof ClassMethod);
+
+        $methodHash   = $this->methodHash($method->getImplementingClass()->getName(), $method->getName());
+        $createMethod = function (?string $aliasMethodName) use ($method, $methodAst) : ReflectionMethod {
+            return ReflectionMethod::createFromNode(
+                $this->reflector,
+                $methodAst,
+                $method->getDeclaringClass()->getDeclaringNamespaceAst(),
+                $method->getDeclaringClass(),
+                $this,
+                $aliasMethodName,
+            );
+        };
+
+        $methods = [];
+        foreach ($traitAliases as $aliasMethodName => $traitAliasDefinition) {
+            if ($methodHash !== $traitAliasDefinition) {
+                continue;
+            }
+
+            $methods[] = $createMethod($aliasMethodName);
+        }
+
+        if (! array_key_exists($methodHash, $traitPrecedences)) {
+            $methods[] = $createMethod($method->getAliasName());
+        }
+
+        return $methods;
+    }
+
+    /**
+     * @return list<ReflectionMethod>
+     */
+    private function getParentMethods() : array
     {
         return array_merge(
             [],
-            array_map(
-                function (ClassMethod $methodNode) : ReflectionMethod {
-                    return ReflectionMethod::createFromNode(
-                        $this->reflector,
-                        $methodNode,
-                        $this->declaringNamespace,
-                        $this,
-                        $this,
-                    );
+            ...array_map(
+                static function (ReflectionClass $ancestor) : array {
+                    return $ancestor->getMethods();
                 },
-                $this->node->getMethods(),
+                array_filter([$this->getParentClass()]),
             ),
+        );
+    }
+
+    /**
+     * @return list<ReflectionMethod>
+     */
+    private function getMethodsFromTraits() : array
+    {
+        return array_merge(
+            [],
             ...array_map(
                 function (ReflectionClass $trait) : array {
                     return array_merge(
@@ -241,70 +283,33 @@ class ReflectionClass implements Reflection
                 },
                 $this->getTraits(),
             ),
-            ...array_map(
-                static function (ReflectionClass $ancestor) : array {
-                    return $ancestor->getMethods();
-                },
-                array_values(array_merge(
-                    array_filter([$this->getParentClass()]),
-                    $this->getInterfaces(),
-                )),
-            ),
         );
     }
 
     /**
-     * @return ReflectionMethod[]
+     * @return list<ReflectionMethod>
      */
-    private function createMethodsFromTrait(ReflectionMethod $method) : array
+    private function getMethodsFromInterfaces() : 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 array_merge(
+            [],
+            ...array_map(
+                static function (ReflectionClass $ancestor) : array {
+                    return $ancestor->getMethods();
+                },
+                array_values($this->getInterfaces()),
+            ),
         );
-
-        return [$newMethod];
     }
 
     /**
-     * Construct a flat list of methods that are available. This will search up
-     * all parent classes/traits/interfaces/current scope for methods.
+     * Construct a flat list of all methods in this precise order from:
+     *  - current class
+     *  - parent class
+     *  - traits used in parent class
+     *  - interfaces implemented in parent class
+     *  - traits used in current class
+     *  - interfaces implemented in current class
      *
      * Methods are not merged via their name as array index, since internal PHP method
      * sorting does not follow `\array_merge()` semantics.
@@ -317,19 +322,51 @@ class ReflectionClass implements Reflection
             return $this->cachedMethods;
         }
 
-        $cachedMethods = [];
+        $classMethods     = $this->getImmediateMethods();
+        $parentClass      = $this->getParentClass();
+        $parentClassName  = $parentClass !== null ? $parentClass->getName() : null;
+        $parentMethods    = $this->getParentMethods();
+        $traitsMethods    = $this->getMethodsFromTraits();
+        $interfaceMethods = $this->getMethodsFromInterfaces();
 
-        foreach ($this->getAllMethods() as $method) {
-            $methodName = strtolower($method->getName());
+        $methods = [];
 
-            if (isset($cachedMethods[$methodName])) {
-                continue;
-            }
+        foreach ([$classMethods, $parentMethods, 'traits' => $traitsMethods, $interfaceMethods] as $type => $typeMethods) {
+            foreach ($typeMethods as $method) {
+                $methodName = strtolower($method->getName());
 
-            $cachedMethods[$methodName] = $method;
+                if (! array_key_exists($methodName, $methods)) {
+                    $methods[$methodName] = $method;
+                    continue;
+                }
+
+                if ($type !== 'traits') {
+                    continue;
+                }
+
+                $existingMethod = $methods[$methodName];
+
+                // Non-abstract trait method can overwrite existing methods:
+                // - when existing method comes from parent class
+                // - when existing method comes from trait and is abstract
+                if (! (
+                    ! $method->isAbstract()
+                    && (
+                        $existingMethod->getDeclaringClass()->getName() === $parentClassName
+                        || (
+                            $existingMethod->isAbstract()
+                            && $existingMethod->getDeclaringClass()->isTrait()
+                        )
+                    )
+                )) {
+                    continue;
+                }
+
+                $methods[$methodName] = $method;
+            }
         }
 
-        $this->cachedMethods = $cachedMethods;
+        $this->cachedMethods = $methods;
 
         return $this->cachedMethods;
     }
@@ -801,15 +838,19 @@ class ReflectionClass implements Reflection
             return null;
         }
 
-        $parent = $this->reflector->reflect($this->node->extends->toString());
-        // @TODO use actual `ClassReflector` or `FunctionReflector`?
-        assert($parent instanceof self);
+        if ($this->cachedParentClass === null) {
+            $parent = $this->reflector->reflect($this->node->extends->toString());
+            // @TODO use actual `ClassReflector` or `FunctionReflector`?
+            assert($parent instanceof self);
+
+            $this->cachedParentClass = $parent;
+        }
 
-        if ($parent->isInterface() || $parent->isTrait()) {
-            throw NotAClassReflection::fromReflectionClass($parent);
+        if ($this->cachedParentClass->isInterface() || $this->cachedParentClass->isTrait()) {
+            throw NotAClassReflection::fromReflectionClass($this->cachedParentClass);
         }
 
-        return $parent;
+        return $this->cachedParentClass;
     }
 
     /**


Test patch:
diff --git a/test/unit/Fixture/MethodsOrder.php b/test/unit/Fixture/MethodsOrder.php
index f84ad94d..68244ff8 100644
--- a/test/unit/Fixture/MethodsOrder.php
+++ b/test/unit/Fixture/MethodsOrder.php
@@ -2,34 +2,79 @@
 
 namespace Roave\BetterReflectionTest\Fixture;
 
-interface MethodsOrderInterface
+interface MethodsOrderParentInterface
 {
-    public function forth();
+    public function f10();
 }
 
-class MethodsOrderParent
+interface MethodsOrderInterface extends MethodsOrderParentInterface
 {
-    public function third()
+    public function f9();
+}
+
+interface InterfaceForMethodsOrderParent
+{
+    public function f6();
+}
+
+trait TraitForMethodsOrderParent
+{
+    public function f5()
+    {
+        // Not used
+    }
+}
+
+abstract class MethodsOrderParent implements InterfaceForMethodsOrderParent
+{
+    use TraitForMethodsOrderParent;
+
+    public function f3()
+    {
+    }
+
+    public function f1()
     {
+        // Not used
     }
 
-    public function first()
+    public function f4()
+    {
+    }
+}
+
+trait TraitForMethodsOrderTrait
+{
+    public function f8()
     {
     }
 }
 
 trait MethodsOrderTrait
 {
-    public function second()
+    use TraitForMethodsOrderTrait;
+
+    public function f2()
     {
+        // Not used
     }
+
+    public function f7()
+    {
+    }
+
+    abstract function f4(); // Not used
 }
 
 abstract class MethodsOrder extends MethodsOrderParent implements MethodsOrderInterface
 {
     use MethodsOrderTrait;
 
-    public function first()
+    public function f1()
+    {
+    }
+
+    public function f2()
     {
     }
 }
diff --git a/test/unit/Fixture/TraitFixture.php b/test/unit/Fixture/TraitFixture.php
index b3d8b665..9f1daafe 100644
--- a/test/unit/Fixture/TraitFixture.php
+++ b/test/unit/Fixture/TraitFixture.php
@@ -31,6 +31,14 @@ trait TraitFixtureTraitC
     public function b() {}
     public function c() {}
 }
+trait TraitFixtureTraitC2
+{
+    public function d() {}
+}
+trait TraitFixtureTraitC3
+{
+    use TraitFixtureTraitC2;
+}
 class TraitFixtureC
 {
     use TraitFixtureTraitC {
@@ -38,6 +46,9 @@ class TraitFixtureC
         b as b_renamed;
         c as private;
     }
+    use TraitFixtureTraitC3 {
+        d as d_renamed;
+    }
 }
 
 // Conflict resolution
diff --git a/test/unit/Fixture/TraitWithAbstractMethod.php b/test/unit/Fixture/TraitWithAbstractMethod.php
new file mode 100644
index 00000000..db6d0548
--- /dev/null
+++ b/test/unit/Fixture/TraitWithAbstractMethod.php
@@ -0,0 +1,62 @@
+<?php
+
+namespace Roave\BetterReflectionTest\Fixture;
+
+abstract class AbstractClassImplementingMethodFromTrait
+{
+    public function foo(): void
+    {
+    }
+
+    public function bar(): void
+    {
+    }
+}
+
+trait TraitWithAbstractMethod
+{
+    abstract public function foo(): void;
+
+    public function bar(): void
+    {
+    }
+}
+
+trait TraitWithBoo
+{
+    public function boo(): void
+    {
+    }
+}
+
+class ClassUsingTraitWithAbstractMethod extends AbstractClassImplementingMethodFromTrait
+{
+    use TraitWithAbstractMethod;
+
+    public function boo(): void
+    {
+    }
+}
+
+class ClassExtendingNonAbstractClass extends ClassUsingTraitWithAbstractMethod
+{
+    use TraitWithBoo;
+}
+
+trait AbstractTrait
+{
+    abstract public function bar(): void;
+}
+
+trait ImplementationTrait
+{
+    public function bar(): void
+    {
+    }
+}
+
+class ClassUsesTwoTraitsWithSameMethodNameOneIsAbstract
+{
+    use AbstractTrait;
+    use ImplementationTrait;
+}
diff --git a/test/unit/Reflection/ReflectionClassTest.php b/test/unit/Reflection/ReflectionClassTest.php
index 34eaa3ee..108c1ee3 100644
--- a/test/unit/Reflection/ReflectionClassTest.php
+++ b/test/unit/Reflection/ReflectionClassTest.php
@@ -45,7 +45,10 @@ use Roave\BetterReflectionTest\ClassWithInterfacesExtendingInterfaces;
 use Roave\BetterReflectionTest\ClassWithInterfacesOther;
 use Roave\BetterReflectionTest\Fixture;
 use Roave\BetterReflectionTest\Fixture\AbstractClass;
+use Roave\BetterReflectionTest\Fixture\ClassExtendingNonAbstractClass;
 use Roave\BetterReflectionTest\Fixture\ClassForHinting;
+use Roave\BetterReflectionTest\Fixture\ClassUsesTwoTraitsWithSameMethodNameOneIsAbstract;
+use Roave\BetterReflectionTest\Fixture\ClassUsingTraitWithAbstractMethod;
 use Roave\BetterReflectionTest\Fixture\ClassWithCaseInsensitiveMethods;
 use Roave\BetterReflectionTest\Fixture\ClassWithMissingParent;
 use Roave\BetterReflectionTest\Fixture\ExampleClass;
@@ -270,10 +273,16 @@ class ReflectionClassTest extends TestCase
         }, $classInfo->getMethods());
 
         $expectedMethodNames = [
-            'first',
-            'second',
-            'third',
-            'forth',
+            'f1',
+            'f2',
+            'f3',
+            'f4',
+            'f5',
+            'f6',
+            'f7',
+            'f8',
+            'f9',
+            'f10',
         ];
 
         self::assertSame($expectedMethodNames, $actualMethodNames);
@@ -927,6 +936,60 @@ PHP;
         self::assertSame('TraitFixtureTraitA', $classInfo->getMethod('foo')->getDeclaringClass()->getName());
     }
 
+    public function declaringClassProvider() : array
+    {
+        return [
+            [
+                ClassUsingTraitWithAbstractMethod::class,
+                'foo',
+                'AbstractClassImplementingMethodFromTrait',
+                'AbstractClassImplementingMethodFromTrait',
+            ],
+            [
+                ClassUsingTraitWithAbstractMethod::class,
+                'bar',
+                'TraitWithAbstractMethod',
+                'ClassUsingTraitWithAbstractMethod',
+            ],
+            [
+                ClassExtendingNonAbstractClass::class,
+                'boo',
+                'TraitWithBoo',
+                'ClassExtendingNonAbstractClass',
+            ],
+            [
+                ClassUsesTwoTraitsWithSameMethodNameOneIsAbstract::class,
+                'bar',
+                'ImplementationTrait',
+                'ClassUsesTwoTraitsWithSameMethodNameOneIsAbstract',
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider declaringClassProvider
+     */
+    public function testGetDeclaringClassWithTraitAndParent(
+        string $className,
+        string $methodName,
+        string $declaringClassShortName,
+        string $implementingClassShortName
+    ) : void {
+        $reflector = new ClassReflector(new SingleFileSourceLocator(
+            __DIR__ . '/../Fixture/TraitWithAbstractMethod.php',
+            $this->astLocator,
+        ));
+
+        $classInfo = $reflector->reflect($className);
+
+        self::assertTrue($classInfo->hasMethod($methodName));
+
+        $fooMethodInfo = $classInfo->getMethod($methodName);
+
+        self::assertSame($declaringClassShortName, $fooMethodInfo->getDeclaringClass()->getShortName());
+        self::assertSame($implementingClassShortName, $fooMethodInfo->getImplementingClass()->getShortName());
+    }
+
     public function testGetTraitsReturnsEmptyArrayWhenNoTraitsUsed() : void
     {
         $reflector = new ClassReflector(new SingleFileSourceLocator(
@@ -965,6 +1028,7 @@ PHP;
         self::assertSame([
             'a_protected' => 'TraitFixtureTraitC::a',
             'b_renamed' => 'TraitFixtureTraitC::b',
+            'd_renamed' => 'TraitFixtureTraitC3::d',
         ], $classInfo->getTraitAliases());
     }
 
@@ -977,14 +1041,14 @@ PHP;
 
         $classInfo = $reflector->reflect('TraitFixtureC');
 
-        self::assertFalse($classInfo->hasMethod('a'));
+        self::assertTrue($classInfo->hasMethod('a'));
         self::assertTrue($classInfo->hasMethod('a_protected'));
 
         $aProtected = $classInfo->getMethod('a_protected');
 
         self::assertSame('TraitFixtureTraitC', $aProtected->getDeclaringClass()->getName());
 
-        self::assertFalse($classInfo->hasMethod('b'));
+        self::assertTrue($classInfo->hasMethod('b'));
         self::assertTrue($classInfo->hasMethod('b_renamed'));
 
         $bRenamed = $classInfo->getMethod('b_renamed');
@@ -997,6 +1061,12 @@ PHP;
 
         self::assertSame('c', $c->getName());
         self::assertSame('TraitFixtureTraitC', $c->getDeclaringClass()->getName());
+
+        self::assertTrue($classInfo->hasMethod('d'));
+        self::assertTrue($classInfo->hasMethod('d_renamed'));
+
+        self::assertSame('TraitFixtureTraitC2', $classInfo->getMethod('d')->getDeclaringClass()->getName());
+        self::assertSame('TraitFixtureTraitC2', $classInfo->getMethod('d_renamed')->getDeclaringClass()->getName());
     }
 
     public function testMethodsFromTraitsWithConflicts() : void
@@ -1019,7 +1089,7 @@ PHP;
         self::assertSame('TraitFixtureTraitD1', $foo->getDeclaringClass()->getName());
         self::assertSame('TraitFixtureD', $foo->getImplementingClass()->getName());
 
-        self::assertFalse($classInfo->hasMethod('hoo'));
+        self::assertTrue($classInfo->hasMethod('hoo'));
         self::assertTrue($classInfo->hasMethod('hooFirstAlias'));
         self::assertTrue($classInfo->hasMethod('hooSecondAlias'));