Merge "Fix class name handling in DeprecationHelper"

diff --git a/includes/debug/DeprecationHelper.php b/includes/debug/DeprecationHelper.php
index dc73ac9..0e1ee6b 100644 (file)
--- a/includes/debug/DeprecationHelper.php
+++ b/includes/debug/DeprecationHelper.php
@@ -79,8 +79,9 @@ trait DeprecationHelper {
                        return $this->$name;
                }
 
-               $qualifiedName = __CLASS__ . '::$' . $name;
-               if ( $this->deprecationHelperGetPropertyOwner( $name ) ) {
+               $ownerClass = $this->deprecationHelperGetPropertyOwner( $name );
+               $qualifiedName = ( $ownerClass ?: get_class( $this ) ) . '::$' . $name;
+               if ( $ownerClass ) {
                        // Someone tried to access a normal non-public property. Try to behave like PHP would.
                        trigger_error( "Cannot access non-public property $qualifiedName", E_USER_ERROR );
                } else {
@@ -99,8 +100,9 @@ trait DeprecationHelper {
                        return;
                }
 
-               $qualifiedName = __CLASS__ . '::$' . $name;
-               if ( $this->deprecationHelperGetPropertyOwner( $name ) ) {
+               $ownerClass = $this->deprecationHelperGetPropertyOwner( $name );
+               $qualifiedName = ( $ownerClass ?: get_class( $this ) ) . '::$' . $name;
+               if ( $ownerClass ) {
                        // Someone tried to access a normal non-public property. Try to behave like PHP would.
                        trigger_error( "Cannot access non-public property $qualifiedName", E_USER_ERROR );
                } else {
@@ -113,22 +115,12 @@ trait DeprecationHelper {
         * Like property_exists but also check for non-visible private properties and returns which
         * class in the inheritance chain declared the property.
         * @param string $property
-        * @return string|bool Best guess for the class in which the property is defined.
+        * @return string|bool Best guess for the class in which the property is defined. False if
+        *   the object does not have such a property.
         */
        private function deprecationHelperGetPropertyOwner( $property ) {
-               // Easy branch: check for protected property / private property of the current class.
-               if ( property_exists( $this, $property ) ) {
-                       // The class name is not necessarily correct here but getting the correct class
-                       // name would be expensive, this will work most of the time and getting it
-                       // wrong is not a big deal.
-                       return __CLASS__;
-               }
-               // property_exists() returns false when the property does exist but is private (and not
-               // defined by the current class, for some value of "current" that differs slightly
-               // between engines).
-               // Since PHP triggers an error on public access of non-public properties but happily
-               // allows public access to undefined properties, we need to detect this case as well.
-               // Reflection is slow so use array cast hack to check for that:
+               // Returning false is a non-error path and should avoid slow checks like reflection.
+               // Use array cast hack instead.
                $obfuscatedProps = array_keys( (array)$this );
                $obfuscatedPropTail = "\0$property";
                foreach ( $obfuscatedProps as $obfuscatedProp ) {
@@ -136,8 +128,9 @@ trait DeprecationHelper {
                        if ( strpos( $obfuscatedProp, $obfuscatedPropTail, 1 ) !== false ) {
                                $classname = substr( $obfuscatedProp, 1, -strlen( $obfuscatedPropTail ) );
                                if ( $classname === '*' ) {
-                                       // sanity; this shouldn't be possible as protected properties were handled earlier
-                                       $classname = __CLASS__;
+                                       // protected property; we didn't get the name, but we are on an error path
+                                       // now so it's fine to use reflection
+                                       return ( new ReflectionProperty( $this, $property ) )->getDeclaringClass()->getName();
                                }
                                return $classname;
                        }

diff --git a/tests/phpunit/includes/debug/DeprecationHelperTest.php b/tests/phpunit/includes/debug/DeprecationHelperTest.php
index 6b977a3..b14d89c 100644 (file)
--- a/tests/phpunit/includes/debug/DeprecationHelperTest.php
+++ b/tests/phpunit/includes/debug/DeprecationHelperTest.php
@@ -118,6 +118,16 @@ class DeprecationHelperTest extends MediaWikiTestCase {
                        $wrapper = TestingAccessWrapper::newFromObject( $this->testSubclass );
                        $this->assertSame( 1, $wrapper->privateNonDeprecated );
                }, E_USER_ERROR, "Cannot access non-public property $fullName" );
+
+               $fullName = 'TestDeprecatedSubclass::$subclassPrivateNondeprecated';
+               $this->assertErrorTriggered( function () {
+                       $this->assertSame( null, $this->testSubclass->subclassPrivateNondeprecated );
+               }, E_USER_ERROR, "Cannot access non-public property $fullName" );
+               $this->assertErrorTriggered( function () {
+                       $this->testSubclass->subclassPrivateNondeprecated = 0;
+                       $wrapper = TestingAccessWrapper::newFromObject( $this->testSubclass );
+                       $this->assertSame( 1, $wrapper->subclassPrivateNondeprecated );
+               }, E_USER_ERROR, "Cannot access non-public property $fullName" );
        }
 
        protected function assertErrorTriggered( callable $callback, $level, $message ) {

diff --git a/tests/phpunit/includes/debug/TestDeprecatedSubclass.php b/tests/phpunit/includes/debug/TestDeprecatedSubclass.php
index 0b6c8cf..28f8fa2 100644 (file)
--- a/tests/phpunit/includes/debug/TestDeprecatedSubclass.php
+++ b/tests/phpunit/includes/debug/TestDeprecatedSubclass.php
@@ -2,6 +2,8 @@
 
 class TestDeprecatedSubclass extends TestDeprecatedClass {
 
+       private $subclassPrivateNondeprecated = 1;
+
        public function getDeprecatedPrivateParentProperty() {
                return $this->privateDeprecated;
        }