ReflectionClass not return private elements of parent classes

Description

The following code:

<?php
class A
{
    private $privateProperty = 1;
    private function privateMethod() {}
}
class B extends A {}
$class = new ReflectionClass('B');
echo (string) $class;

Resulted in this output:

Class [ <user> class B extends A ] {
  ...

  - Properties [0] {
  }

  - Methods [0] {
  }
}

But I expected this output instead:

Class [ <user> class B extends A ] {
  ...

  - Properties [1] {
    Property [ private $privateProperty = 1 ]
  }

  - Methods [1] {
    Method [ <user, inherits A> private method privateMethod ] {
      @@ ...
    }
  }
}

PHP Version

PHP 8.0.22

Operating System

No response

Private members are private to the class that defined them and not inherited by child classes. If you want to see that information then you need an instance of ReflectionClass that focuses on the A class.

@damianwadley

@damianwadley yes, but class still contains that's methods.

Also, ReflectionClass return private methods of parent trait:

<?php
trait T {
    private function privateTraitMethod() {}
}
trait T2 {
    use T;
}
class A {
    private function privateMethod() {}
}
class B extends A {
    use T2;
}
$class = new ReflectionClass('B');
echo (string) $class;

Result:

Class [ <user> class B extends A ] {
  ...
  - Methods [1] {
    Method [ <user> private method privateTraitMethod ] {
      ...
    }
  }
}

Traits don't use inheritance like classes do.

Traits are language-assisted copy and paste. What your code is doing right is now copying and pasting "private function privateTraitMethod() { }" into the B class.

@damianwadley

@damianwadley I see inconsistency in the other direction: https://3v4l.org/1AWCJ

new ReflectionMethod('B', 'privateMethod') shouldn't be allowed, but It's successful and when converted to string it's showing highly incorrect: inherits A: Method [ <user, inherits A> private method privateMethod ]

Also when child class defines own method with that name ReflectionMethod again reports overwrites A: Method [ <user, overwrites A> private method privateMethod ]

Related: https://bugs.php.net/bug.php?id=64592

It's not like the private method doesn't exist at all but more that there is one albeit as part of an ancestor class. And I'm also thinking that rules are rules even if sometimes reflection might break them.

Consider a child class, a parent class with a private privateMethod, and also a grandparent class with its own private privateMethod. Both of those methods exist at the same time and they do so separately from each other: if the grandparent calls $this->privateMethod() then it gets the grandparent's method, and if the parent does too then it gets the parent's method, and if the child does too then PHP complains. They're so separate that they can even have completely different and conflicting signatures.

Now, let's say you had an instance of the child class and wanted to list all of its methods. Should you see 0 privateMethods because there aren't any on the child, 1 method because there can only be one by name and presumably that one should be the "closest" method (which is on the parent), or 2 methods because there are actually two different ones that can be invoked?

The viewpoint taken for bug 64592 was that yes, in fact, you shouldn't be able to see either of those private method(s) by listing the things specifically on the child class. And I do think that creating a ReflectionMethod based on the child class B and an ancestor method privateMethod should fail, but that would be another backwards-incompatible change for the sake of consistency.
(I also think it's unlikely that someone would intentionally try to refer to the method as "B::privateMethod" so it could be an edge case to begin with.)

@damianwadley

@damianwadley

or 2 methods because there are actually two different ones that can be invoked?

ReflectionMethod::getPrototype() solves this problem very well

Reflection classes are designed to make code exploration easier, not to confuse developers.
Using crutches like as

while (!$reflectionClass->hasMethod('privateMethod')) {
    $reflectionClass = $reflectionClass->getParentClass();
}

not having obvious advantages.

Reflection isn't about code exploration. Reflection is about giving runtime access to internal structures and data; if you use that for some sort of exploration or discovery process then that's your choice (and a very logical one). But having that reflection lie about the nature of those internal structures and data, such as by making it look like a class has a private method when it does not, will cause confusion - if not at that time then later when the mistaken beliefs are challenged by more explicit truths.

So what you call a "crutch" I call an accurate description of the PHP internals, in that if you wanted to use ReflectionClass to find a method named "privateMethod" then yes, use a loop to traverse the hierarchy until you find what you want. Though that process involves a couple more lines of code, it's extremely self-documenting about what it represents: the literal act of searching the hierarchy for a named method.
But if I were a code reviewer on your pull request that introduces such code then I'd have some rather big questions about why you'd be doing so.

From where I'm sitting, the original problem reported is not so much that reflection did not behave as was expected, but that there was an incorrect expectation of a certain behavior and that reflection did not support the expectation. B does not, in fact, have a privateMethod and almost* any attempt to call it as if it did will fail. Is there some confusion about this? Apparently so, but the way to address that is by correcting the mistaken belief.

Question: is there a honest use case here where one needs to (1) reflect on a class or object and (2) identify an arbitrary "privateMethod" that is located somewhere in the class/object's inheritance hierarchy? I don't really have one myself.

If there was one, consider the fragility of doing so:

Since private methods have no inheritance rules, any class in the hierarchy is free to implement its own particular "privateMethod" with any signature it so desires.
Adding such a method could easily change where in the inheritance traversal the named method is found, and cause the code responsible to locate something it did not want.

Let's give this method a better name for the moment: toArray.

abstract class GrandparentClass {
    private function toArray(): array { ... }
}

abstract class ParentClass extends GrandparentClass {
    private function toArray(int $options): array { ... }
}

class ChildClass extends ParentClass { }

Two different methods with presumably a similar purpose - a helper method that collects internal data into an array - that go about it two different ways. If you use reflection to search the hierarchy for a "toArray" starting from a ChildClass then you'll find ParentClass's implementation first. If that's what you wanted then great, now you can call the method with your $options value (or whatever).

But let's say ChildClass updates:

class ChildClass extends ParentClass {
    private function toArray(ToArrayVisitor $visitor): array { ... }
}

This completely breaks the reflective code because now you're going to find ChildClass's toArray first, and if you call it with your $options integer then PHP will complain.
Instead, what you should be doing** is specifically targetting ParentClass's implementation, and you do so with a new ReflectionMethod(ParentClass::class, "toArray"). That's a very precise implementation that won't randomly fail because someone added a new method somewhere. And there's no loops.

I've lost my train of thought, but this reply is long enough so whatever.

* The scope resolution operator :: (please don't think it's a "static method call operator") is flexible in how it uses the LHS class name to resolve a method. For example, you can write B::privateMethod within A and it will work. But that's a whole different discussion so please check out https://3v4l.org/j21Sh for a teaser instead.
** What you really should be doing is using an interface and avoiding reflection entirely.

Reflection is about giving runtime access to internal structures and data;

Reflection is "exploration", this classes not focused on "access" like as runkit/uopz.

why you'd be doing so

packages like as BetterReflection bases on crutches like this, I want fix it in native classes.

This completely breaks the reflective code because now you're going to find ChildClass's toArray first, and if you call it with your $options integer then PHP will complain.

i don't vote to add all declarations of method, I'm vote to add latest declaration i.e. if private method declared in parent class and not declared in current class, then reflection returns information about parent method. if private method declared in parent class and in current class, then reflection returns information about this method.

Shouldn't this thinking apply to method_exists() as well?

The changes may break code that use functions like this to prevent errors for non-existent methods during reflection: https://github.com/search?q=+%22method_exists%22+%22reflectionmethod%22&type=code (notably, reflection may be used to determine visibility)

Navigation Menu

Repository navigation

ReflectionClass not return private elements of parent classes #9405

Description

PHP Version

Operating System

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

ReflectionClass not return private elements of parent classes #9405

Description

Description

PHP Version

Operating System

Activity

damianwadley commented on Aug 23, 2022

WinterSilence commented on Aug 23, 2022

damianwadley commented on Aug 23, 2022

KapitanOczywisty commented on Aug 23, 2022

damianwadley commented on Aug 23, 2022

WinterSilence commented on Aug 24, 2022

damianwadley commented on Aug 24, 2022

WinterSilence commented on Aug 24, 2022

jqazsy commented on Jun 24, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions