RTFM:

"Members declared protected can be accessed only within the class itself and by inherited and parent classes."

http://php.net/manual/en/language.oop5.visibility.php

I did "RTFM" and I understand that protected members are not SUPPOSED TO BE ABLE to be accessed from anything but "within the class itself and by inherited and parent classes."  The bug is that, when the calling scope resolves and LOOKS LIKE the correct scope because it is an inherited class, but is a _different inherited class_ of the same parent object in which the protected member was declared, the engine allows access to protected members from OUTSIDE THE CLASS.

In the example, both "bar" and "kid" extend foo, but "kid" should NOT have access to bar's protected members, but it does, but its resolving scope looks similar to the correct scope that would need to resolve.

This isn't an RTFM issue, it's a legitimate bug.  Did you read the reproduce code?

Even though it feels odd, it's how the checks are currently designed.

The same applies to i.e. Java. I guess we cannot change things here 
without introducing nasty and hard to track BC breaks.

That's a fundamental part of a class based inheritance system.Changing this won't only affect BC but also the fundamental design.

At a second glance, it really looks like it is wrong.

The visibility check should also be based on the object from which the 
properties are read.

As for my "it works that way in Java" argument, it contradicts the java 
specifications: 
http://java.sun.com/docs/books/jls/third_edition/html/names.html#6.6.2.1
and hence is invalid. (Thanks oorza for pointing that out)

There is no design ground to accept that, it is a bug. The question that 
remains is: is it worth to fix BC-wise?

It causes more problems then the original poster notes.  We've run into problems using __get and __set. Since php see's the member as "in scope" for both classes, the __get and __set methods don't get called.

Our problem differs slightly from this issue, but I think they share a root cause.  Here's a sample of code with the expected outcome.
abstract class A {
    /**
     * undocumented function
     *
     * @return void
     */
    public function __get($propertyName)
    {
        $val = $this->$propertyName;
        echo "Current Value of '{$propertyName}' is '{$val}'\n";
        return $val;
    }
    
    public function __set($propertyName, $propertyValue)
    {
        echo "Setting Property '{$propertyName}'  to '{$propertyValue}'\n";
        $this->$propertyName = $propertyValue;
    }
}

class B extends A {
    protected $name;
    
    public function populateName($val) {
        $a = 'name';
        $this->$a = $val;
    }
    
    public function testit() {
        $b = new B();
        $b->name = 'internal';
        $b->name;
    }
}

$two = new B();
$two->name = 'external';
$two->name;

$two->testit();

Expected Results:
-----------------
Setting Property 'name'  to 'external'
Current Value of 'name' is 'external'
Setting Property 'name'  to 'internal'
Current Value of 'name' is 'internal'

Actual Results:
----------------
Setting Property 'name'  to 'external'
Current Value of 'name' is 'external'

Another one example, even without "extends":

class Page_Element
{
        protected $name = 'name*';

        public function __construct(Page_Element $child = null)
        {
            echo $this->name;
            echo $child->name;
        }

}

new Page_Element(new Page_Element(null));

(from http://youtrack.jetbrains.net/issue/WI-4663)

So why $child->name can be accessed? It's new object!

normandiggs, your sample is irrelevant.
It's ok to access private and protected properties from same type object.
It's bad to access protected properties from different type object.

Why I hate this behavior:
http://3v4l.org/tiOC5 - we receive protected value
http://3v4l.org/uT9PC - we receive fatal error

We should not to rely to implementation of B class (see fiddles)

Another note - this behavior was broken in 5.2 version.

| There is no design ground to accept that, it is a bug. The
| question that remains is: is it worth to fix BC-wise?

IMHO the question is rather: when is the BC break acceptable? 7.1
or 8.0?

A PR was prepared to fix this on the current master (7.2) but it was decided the the potential BC break could possibly be too much for a minor and it was suggested that it should wait for the next major.

This may seem odd, but is actually perfectly reasonable if you consider polymorphism:

- Access is class-based, not instance-based: you can call private members of a different instance of the same class.
- For the same reason, you can access protected members on an instance of a parent class.
- If you ask for an instance of class A, you may at run-time actually receive an instance of some sub-class of A.
- So, if classes B1 and B2 both inherit from A, a method on class B1 might ask for an instance of A, and be given an instance of B2; when it calls a method on that instance, it's the definition in B2 which will be accessed.

Here's a concrete example: https://3v4l.org/LeClo

Note that this example also works in Java: https://tio.run/##vZMxb9swEIVn61fcKKctbWctMhUIuhgomqIdigwn8mIxpUiDPNkpDP92hxJlW4rsIUBRQhP5@O670@MzbvDTs/pzOGAR2KNkkAZDgG9eS4JdBnGtvWOSTAqUqwtDsEFT0@f2LAnirpbpTt7XTDuHZnGpg2h34a7vsD/7nBhGFVfE33H7s7mVT8eVz6p75yvkeLPT9gBmM/iCxgTAnn9FXDoF7gm2pZYlbciDLLVR3Rx0aMFPJp649haWyKXwrrYqb/sa8E1HjXWYG6cVoFJ5mq7jkvy/IGyNLg36w106Exfmt8/2WZYsfuDLvSdKVPTCZNWbBCT@ge7qjw71mnyeto@lLudoQNVz6IZ8bmRE3CL80lxGpCHxbpTIo@x/AsMNLMRtDzvriBL@ErWN1yfdZmDkYzyqeJQ/sNd29fsR0K9CU2gySUPH@HYsbQd/Il/MxTyCHzVFp@k3f9Y8/A1MlXA1i3UswsbmKC48nGvi4qo4fjHAX8nTR0AL2sa2bBPzp2FwttoYQCkpvA16VcRsR3mfPLqiaN5M8W78ZvCHwys


The example from the original report also works in Java: https://tio.run/##fY9BDoMgEEXXcgriShflAqZH6Mpl0zSg1CAKDYy2TePZKQTUpovuZv7/mf@mpzM99K10rhmotfimNX6j7D6xQTS4BiNUh69pPeKc5lXmbaOBN8DbPbEpPrQteYUWhOJlRg3mT@Cq3Vp2U4r210yds/YWcAtFGdQsuMyXKP4IJ4uy8mL9ssBHoicgd88DgyoYSdT/AitnyCyBJpVGqBMV6ovEAoUVaPRWEX8/XzA1nY104Q@Z6Pwc6SSJ/KnEuQ8


I'm of the opinion that this is not a bug.

Sorry, slight mistake in my example: to be equivalent to this report, the Subclass::add(ParentClass $other) method should be declared on one of the child classes. But it still works in both languages:

https://3v4l.org/IInDq

https://tio.run/##vVPBThsxED1vvmKOm5Ya6LXihIS4IFWA2kPVw6w9ZA1eO7JnEyrEt6fe9YbYJDkgVbX2NH5@8@bN20dc4ZdH9bTZYBPYo2SQBkOA715LgpcZxLP0jkkyKVCubwzBCk1P38a7BIhVLdObOsfMJ4bhcKuDGKtwkTO87njeNOx1XBDf4vrH8Kqe73feoa6c75DjywmbCTg9hUs0JgBm/B1x6xS4B1i3Wra0Ig@y1UZNPugwCn8j8cS9t3CD3Arveqvqca5C33w72Otsllju8fnKEyVT6ZnJqncWpzkK3FEnQ78kX6fyttXhRRWyMoZpit1KSpokZuW0AlSqTnIct@T/hZ8j0aFYfL5Id@LAtjMzRzk/NbfRrdLMl700bmH/00v4BOfiayZ7NilK8m9Q2/i8moqBkbdmd/GqvmOv7eLXb0C/CEOjqipigfH3sbQuslKfn4mzqL@qiqmbCZoXd9C7P4GpE65nsYwt2dgaxYFf6Bi4OQqOXwzHNXk6AbSgbRzSDhF6KBO@1sYASknhfYi6JuYmwnPlkRXFkMfmw/KHNWw2fwE

We should not to stand on other language behavior.

One more time: https://3v4l.org/TM6jk

For now it just breaks SOLID.

I agree with arth dot inbox at gmail dot com, the original bug report is wrong but this actually should be filed as a new bug imho:

Reproduce code:
---------------
class C {
  protected $x = 10;
}
class D extends C {
  public function m(C $c) {
    print "x=" . $c->x . "\n";
  }
}
class E extends C {
  protected $x = 20;
}
$d = new D();
$e = new E();
$d->m($d);
$d->m($e);

Expected result:
----------------
x=10
x=10      /* note 10, NOT 20! */

Actual result:
--------------
x=10
PHP Fatal error:  Uncaught Error: Cannot access protected property E::$x in Standard input code:9
Stack trace:
#0 Standard input code(21): D->m(Object(E))
#1 {main}
  thrown in Standard input code on line 9

> https://3v4l.org/TM6jk

This appears to be exactly the opposite complaint from the original: in this case, the protected property *can't* be accessed in a different child.

So we certainly have an inconsistency here - between methods and properties, and based on whether the member is inherited directly or over-ridden / re-declared. The case of a re-declared property is currently the only one that errors, so the simplest change would be to remove that error.

Class C knows that the access is legal when given an A, so assumes it's valid for all sub-classes of A; sub-class B2 is currently able to break this assumption for a property, but not for a method.

@giovanni:

Why would the expected behaviour there be to print 10? Re-declaring a property doesn't create a second property with the same name, it changes it polymorphically just like re-declaring a method does. Either the method can see the value 10, or it cannot see any value.

class C {
  protected $x = 10;
  public function m() {
    print "x=" . $this->x . "\n";
  }
}
class D extends C {
  protected $x = 20;
}
$d = new D();
$d->m(); // "x=20"

This really is a travesty that this scoping issue has been present for at almost 13 years and has gone unaddressed

It's really not something that should be swept under the carpet

I've done a bit of experimentation, and it appears that both C++ and C# correctly handle this, while Java fails to do it correctly.