Wrong behavior of `ReflectionMethod::getPrototype()`

Demo:

interface I
{
    public function foo();
}
abstract class A implements I
{
    abstract public function foo(): array;
}
class B extends A
{
    public function foo(): array
    {
        return [];
    }
}

Native reflection:

$c = new \ReflectionClass(A::class);
$m = $c->getMethod('foo');
var_dump($m->getPrototype()->name); // print: `I`
$c = new \ReflectionClass(B::class);
$m = $c->getMethod('foo');
var_dump($m->getPrototype()->name); // print: `A`

Better reflection:

$c = new \ReflectionClass(A::class);
$m = $c->getMethod('foo');
var_dump($m->getPrototype()->getName()); // print: `I`
$c = new \ReflectionClass(B::class);
$m = $c->getMethod('foo');
var_dump($m->getPrototype()->getName()); // print: `I` ERROR

BetterReflection/src/Reflection/ReflectionMethod.php

Line 125 in b02fa4d

$currentClass = $this->getImplementingClass();

Error: current class is getCurrentClass() and it's not always same to getImplementingClass()

BetterReflection/src/Reflection/ReflectionMethod.php

Line 128 in b02fa4d

foreach ($currentClass->getImmediateInterfaces() as $interface) {

Error: why does getPrototype() at first look for interfaces, not classes?

BetterReflection/src/Reflection/ReflectionMethod.php

Line 160 in b02fa4d

if ($this->isAbstract()) {

Error: abstract method can be declared in interface, A::foo() as in demo

Is this for 4.x or the current 5.0.x branch?

@Ocramius I inserted lines with errors in 5.0

Looking further at this, I must say that I don't understand what the use of getPrototype() is :D

I'd say that for our own purposes, we should drop it from our core reflection API in 6.0.0, only to be kept in the /Adapter code

Yeah I vaguely remember implementing this and I couldn't figure out the exact use for it :/ sorry

@Ocramius @asgrim getDeclaringClass() it's latest class in chain of nested classes who override method, getPrototype() is last/top interface/class in chain of nested interfaces/abstract classes where defined method i.e. last abstraction in chain who re-declare method.

interface I
{
    public function foo();
    public function bar();
}
abstract class A implements I
{
    abstract public function foo();
    abstract public function bar();
}
abstract class A2 extends A
{
    abstract public function bar(): array;
}
class B extends A2
{
    public function foo()
    {
           return 1;
    }
    public function bar(): array
    {
           return [];
    }
}

B is declaring class for foo() and bar()
A is prototype class for foo()
A2 is prototype class for bar()

Note: "re-declare" != "change":

abstract class A
{
   abstract public function foo();
}
abstract class A2 extends A
{
   abstract public function foo();
}
class B extends A2
{
   public function foo()
   {
          return 1;
   }
}

A2 is prototype class, B is declaring class

if useful to find new methods in class or find method comment to replace @inheridDoc for child.

Also, these methods are needed for compatibility with native ReflectionMethod

They're not necessary: they can be moved to the adapter layer :)

@Ocramius they not necessary for you, but it's necessary for code review/static analyze

I'm not able to reproduce it. I think this should be closed. Failing test is needed.

Closing here as per @kukulich's feedback

@Ocramius what's you mean?

Needs a failing automation test case in the test suite of this library, in order to be reconsidered.

@Ocramius it's issue i.e. is reason to send PR with this test case. Closing unresolved issues does not solve the problems described in them.

It's resolved according to @kukulich not being able to reproduce this.

Please consider opening a PR with a failing test case if you don't believe that to be true.