From: Gergő Tisza <gtisza@wikimedia.org>
Date: Wed, 11 Jul 2018 17:34:26 +0000 (+0000)
Subject: Add helper trait for deprecating properties
X-Git-Tag: 1.34.0-rc.0~4659^2
X-Git-Url: https://git.cyclocoop.org/%27.WWW_URL.%27admin/?a=commitdiff_plain;h=4aedefdbfd193f323097354bf581de1c93f02715;p=lhc%2Fweb%2Fwiklou.git

Add helper trait for deprecating properties

Change-Id: I83e6ee4e8eedd49acef2b5d92132d37af715bff3
---

diff --git a/autoload.php b/autoload.php
index 40b8acfee9..ac34efbaf1 100644
--- a/autoload.php
+++ b/autoload.php
@@ -387,6 +387,7 @@ $wgAutoloadLocalClasses = [
 	'DependencyWrapper' => __DIR__ . '/includes/cache/CacheDependency.php',
 	'DeprecatedGlobal' => __DIR__ . '/includes/DeprecatedGlobal.php',
 	'DeprecatedInterfaceFinder' => __DIR__ . '/maintenance/findDeprecated.php',
+	'DeprecationHelper' => __DIR__ . '/includes/debug/DeprecationHelper.php',
 	'DerivativeContext' => __DIR__ . '/includes/context/DerivativeContext.php',
 	'DerivativeRequest' => __DIR__ . '/includes/DerivativeRequest.php',
 	'DerivativeResourceLoaderContext' => __DIR__ . '/includes/resourceloader/DerivativeResourceLoaderContext.php',
diff --git a/includes/debug/DeprecationHelper.php b/includes/debug/DeprecationHelper.php
new file mode 100644
index 0000000000..cd78005a65
--- /dev/null
+++ b/includes/debug/DeprecationHelper.php
@@ -0,0 +1,148 @@
+<?php
+/**
+ * Trait for issuing warnings on deprecated access.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * Use this trait in classes which have properties for which public access
+ * is deprecated. Set the list of properties in $deprecatedPublicProperties
+ * and make the properties non-public. The trait will preserve public access
+ * but issue deprecation warnings when it is needed.
+ *
+ * Example usage:
+ *     class Foo {
+ *         use DeprecationHelper;
+ *         protected $bar;
+ *         public function __construct() {
+ *             $this->deprecatePublicProperty( 'bar', '1.21', __CLASS__ );
+ *         }
+ *     }
+ *
+ *     $foo = new Foo;
+ *     $foo->bar; // works but logs a warning
+ *
+ * Cannot be used with classes that have their own __get/__set methods.
+ *
+ * @since 1.32
+ */
+trait DeprecationHelper {
+
+	/**
+	 * List of deprecated properties, in <property name> => [<version>, <class>, <component>] format
+	 * where <version> is the MediaWiki version where the property got deprecated, <class> is the
+	 * the name of the class defining the property, <component> is the MediaWiki component
+	 * (extension, skin etc.) for use in the deprecation warning) or null if it is MediaWiki.
+	 * E.g. [ 'mNewRev' => [ '1.32', 'DifferenceEngine', null ]
+	 * @var string[]
+	 */
+	protected $deprecatedPublicProperties = [];
+
+	/**
+	 * Mark a property as deprecated. Only use this for properties that used to be public and only
+	 *   call it in the constructor.
+	 * @param string $property The name of the property.
+	 * @param string $version MediaWiki version where the property became deprecated.
+	 * @param string|null $class The class which has the deprecated property. This can usually be
+	 *   guessed, but PHP can get confused when both the parent class and the subclass use the
+	 *   trait, so it should be specified in classes meant for subclassing.
+	 * @param string|null $component
+	 * @see wfDeprecated()
+	 */
+	protected function deprecatePublicProperty(
+		$property, $version, $class = null, $component = null
+	) {
+		$this->deprecatedPublicProperties[$property] = [ $version, $class ?: get_class(), $component ];
+	}
+
+	public function __get( $name ) {
+		if ( isset( $this->deprecatedPublicProperties[$name] ) ) {
+			list( $version, $class, $component ) = $this->deprecatedPublicProperties[$name];
+			$qualifiedName = $class . '::$' . $name;
+			wfDeprecated( $qualifiedName, $version, $component, 3 );
+			return $this->$name;
+		}
+
+		$qualifiedName = get_class() . '::$' . $name;
+		if ( $this->deprecationHelperGetPropertyOwner( $name ) ) {
+			// 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 {
+			// Non-existing property. Try to behave like PHP would.
+			trigger_error( "Undefined property: $qualifiedName", E_USER_NOTICE );
+		}
+		return null;
+	}
+
+	public function __set( $name, $value ) {
+		if ( isset( $this->deprecatedPublicProperties[$name] ) ) {
+			list( $version, $class, $component ) = $this->deprecatedPublicProperties[$name];
+			$qualifiedName = $class . '::$' . $name;
+			wfDeprecated( $qualifiedName, $version, $component, 3 );
+			$this->$name = $value;
+			return;
+		}
+
+		$qualifiedName = get_class() . '::$' . $name;
+		if ( $this->deprecationHelperGetPropertyOwner( $name ) ) {
+			// 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 {
+			// Non-existing property. Try to behave like PHP would.
+			$this->$name = $value;
+		}
+	}
+
+	/**
+	 * 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.
+	 */
+	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:
+		$obfuscatedProps = array_keys( (array)$this );
+		$obfuscatedPropTail = "\0$property";
+		foreach ( $obfuscatedProps as $obfuscatedProp ) {
+			// private props are in the form \0<classname>\0<propname>
+			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__;
+				}
+				return $classname;
+			}
+		}
+		return false;
+	}
+
+}
diff --git a/tests/common/TestsAutoLoader.php b/tests/common/TestsAutoLoader.php
index 06d789b09e..0cb042a553 100644
--- a/tests/common/TestsAutoLoader.php
+++ b/tests/common/TestsAutoLoader.php
@@ -109,6 +109,10 @@ $wgAutoloadClasses += [
 	# tests/phpunit/includes/db
 	'DatabaseTestHelper' => "$testDir/phpunit/includes/db/DatabaseTestHelper.php",
 
+	# tests/phpunit/includes/debug
+	'TestDeprecatedClass' => "$testDir/phpunit/includes/debug/TestDeprecatedClass.php",
+	'TestDeprecatedSubclass' => "$testDir/phpunit/includes/debug/TestDeprecatedSubclass.php",
+
 	# tests/phpunit/includes/diff
 	'FakeDiffOp' => "$testDir/phpunit/includes/diff/FakeDiffOp.php",
 
diff --git a/tests/phpunit/includes/debug/DeprecationHelperTest.php b/tests/phpunit/includes/debug/DeprecationHelperTest.php
new file mode 100644
index 0000000000..55e5bbf28b
--- /dev/null
+++ b/tests/phpunit/includes/debug/DeprecationHelperTest.php
@@ -0,0 +1,163 @@
+<?php
+
+use Wikimedia\TestingAccessWrapper;
+
+/**
+ * @covers DeprecationHelper
+ */
+class DeprecationHelperTest extends MediaWikiTestCase {
+
+	/** @var TestDeprecatedClass */
+	private $testClass;
+
+	/** @var TestDeprecatedSubclass */
+	private $testSubclass;
+
+	public function setUp() {
+		parent::setUp();
+		$this->testClass = new TestDeprecatedClass();
+		$this->testSubclass = new TestDeprecatedSubclass();
+		$this->setMwGlobals( 'wgDevelopmentWarnings', false );
+	}
+
+	public function tearDown() {
+		parent::tearDown();
+		MWDebug::clearLog();
+	}
+
+	/**
+	 * @dataProvider provideGet
+	 */
+	public function testGet( $propName, $expectedLevel, $expectedMessage ) {
+		if ( $expectedLevel ) {
+			$this->assertErrorTriggered( function () use ( $propName ) {
+				$this->assertSame( null, $this->testClass->$propName );
+			}, $expectedLevel, $expectedMessage );
+		} else {
+			$this->assertDeprecationWarningIssued( function () use ( $propName ) {
+				$this->assertSame( 1, $this->testClass->$propName );
+			} );
+		}
+	}
+
+	public function provideGet() {
+		return [
+			[ 'protectedDeprecated', null, null ],
+			[ 'protectedNonDeprecated', E_USER_ERROR,
+				'Cannot access non-public property TestDeprecatedClass::$protectedNonDeprecated' ],
+			[ 'privateDeprecated', null, null ],
+			[ 'privateNonDeprecated', E_USER_ERROR,
+			  'Cannot access non-public property TestDeprecatedClass::$privateNonDeprecated' ],
+			[ 'nonExistent', E_USER_NOTICE, 'Undefined property: TestDeprecatedClass::$nonExistent' ],
+		];
+	}
+
+	/**
+	 * @dataProvider provideSet
+	 */
+	public function testSet( $propName, $expectedLevel, $expectedMessage ) {
+		$this->assertPropertySame( 1, $this->testClass, $propName );
+		if ( $expectedLevel ) {
+			$this->assertErrorTriggered( function () use ( $propName ) {
+				$this->testClass->$propName = 0;
+				$this->assertPropertySame( 1, $this->testClass, $propName );
+			}, $expectedLevel, $expectedMessage );
+		} else {
+			if ( $propName === 'nonExistent' ) {
+				$this->testClass->$propName = 0;
+			} else {
+				$this->assertDeprecationWarningIssued( function () use ( $propName ) {
+					$this->testClass->$propName = 0;
+				} );
+			}
+			$this->assertPropertySame( 0, $this->testClass, $propName );
+		}
+	}
+
+	public function provideSet() {
+		return [
+			[ 'protectedDeprecated', null, null ],
+			[ 'protectedNonDeprecated', E_USER_ERROR,
+			  'Cannot access non-public property TestDeprecatedClass::$protectedNonDeprecated' ],
+			[ 'privateDeprecated', null, null ],
+			[ 'privateNonDeprecated', E_USER_ERROR,
+			  'Cannot access non-public property TestDeprecatedClass::$privateNonDeprecated' ],
+			[ 'nonExistent', null, null ],
+		];
+	}
+
+	public function testInternalGet() {
+		$this->assertSame( [
+			'prod' => 1,
+			'prond' => 1,
+			'prid' => 1,
+			'prind' => 1,
+		], $this->testClass->getThings() );
+	}
+
+	public function testInternalSet() {
+		$this->testClass->setThings( 2, 2, 2, 2 );
+		$wrapper = TestingAccessWrapper::newFromObject( $this->testClass );
+		$this->assertSame( 2, $wrapper->protectedDeprecated );
+		$this->assertSame( 2, $wrapper->protectedNonDeprecated );
+		$this->assertSame( 2, $wrapper->privateDeprecated );
+		$this->assertSame( 2, $wrapper->privateNonDeprecated );
+	}
+
+	public function testSubclassGetSet() {
+		$this->assertDeprecationWarningIssued( function () {
+			$this->assertSame( 1, $this->testSubclass->getDeprecatedPrivateParentProperty() );
+		} );
+		$this->assertDeprecationWarningIssued( function () {
+			$this->testSubclass->setDeprecatedPrivateParentProperty( 0 );
+		} );
+		$wrapper = TestingAccessWrapper::newFromObject( $this->testSubclass );
+		$this->assertSame( 0, $wrapper->privateDeprecated );
+
+		$fullName = 'TestDeprecatedClass::$privateNonDeprecated';
+		$this->assertErrorTriggered( function () {
+			$this->assertSame( null, $this->testSubclass->getNonDeprecatedPrivateParentProperty() );
+		}, E_USER_ERROR, "Cannot access non-public property $fullName" );
+		$this->assertErrorTriggered( function () {
+			$this->testSubclass->setNonDeprecatedPrivateParentProperty( 0 );
+			$wrapper = TestingAccessWrapper::newFromObject( $this->testSubclass );
+			$this->assertSame( 1, $wrapper->privateNonDeprecated );
+		}, E_USER_ERROR, "Cannot access non-public property $fullName" );
+	}
+
+	protected function assertErrorTriggered( callable $callback, $level, $message ) {
+		$actualLevel = $actualMessage = null;
+		set_error_handler( function ( $errorCode, $errorStr ) use ( &$actualLevel, &$actualMessage ) {
+			$actualLevel = $errorCode;
+			$actualMessage = $errorStr;
+		} );
+		$callback();
+		restore_error_handler();
+		$this->assertSame( $level, $actualLevel );
+		$this->assertSame( $message, $actualMessage );
+	}
+
+	protected function assertPropertySame( $expected, $object, $propName ) {
+		try {
+			$this->assertSame( $expected, TestingAccessWrapper::newFromObject( $object )->$propName );
+		} catch ( ReflectionException $e ) {
+			if ( !preg_match( "/Property (TestDeprecated(Class|Subclass)::)?$propName does not exist/",
+				$e->getMessage() )
+			) {
+				throw $e;
+			}
+			// property_exists accepts monkey-patching, Reflection / TestingAccessWrapper doesn't
+			if ( property_exists( $object, $propName ) ) {
+				$this->assertSame( $expected, $object->$propName );
+			}
+		}
+	}
+
+	protected function assertDeprecationWarningIssued( callable $callback ) {
+		MWDebug::clearLog();
+		$callback();
+		$wrapper = TestingAccessWrapper::newFromClass( MWDebug::class );
+		$this->assertNotEmpty( $wrapper->deprecationWarnings );
+	}
+
+}
diff --git a/tests/phpunit/includes/debug/TestDeprecatedClass.php b/tests/phpunit/includes/debug/TestDeprecatedClass.php
new file mode 100644
index 0000000000..5a9e645b82
--- /dev/null
+++ b/tests/phpunit/includes/debug/TestDeprecatedClass.php
@@ -0,0 +1,35 @@
+<?php
+
+class TestDeprecatedClass {
+
+	use DeprecationHelper;
+
+	protected $protectedDeprecated = 1;
+	protected $protectedNonDeprecated = 1;
+	private $privateDeprecated = 1;
+	private $privateNonDeprecated = 1;
+
+	public function __construct() {
+		$this->deprecatedPublicProperties = [
+			'protectedDeprecated' => '1.23',
+			'privateDeprecated' => '1.24',
+		];
+	}
+
+	public function setThings( $prod, $prond, $prid, $prind ) {
+		$this->protectedDeprecated = $prod;
+		$this->protectedNonDeprecated = $prond;
+		$this->privateDeprecated = $prid;
+		$this->privateNonDeprecated = $prind;
+	}
+
+	public function getThings() {
+		return [
+			'prod' => $this->protectedDeprecated,
+			'prond' => $this->protectedNonDeprecated,
+			'prid' => $this->privateDeprecated,
+			'prind' => $this->privateNonDeprecated,
+		];
+	}
+
+}
diff --git a/tests/phpunit/includes/debug/TestDeprecatedSubclass.php b/tests/phpunit/includes/debug/TestDeprecatedSubclass.php
new file mode 100644
index 0000000000..0b6c8cf6f1
--- /dev/null
+++ b/tests/phpunit/includes/debug/TestDeprecatedSubclass.php
@@ -0,0 +1,21 @@
+<?php
+
+class TestDeprecatedSubclass extends TestDeprecatedClass {
+
+	public function getDeprecatedPrivateParentProperty() {
+		return $this->privateDeprecated;
+	}
+
+	public function setDeprecatedPrivateParentProperty( $value ) {
+		$this->privateDeprecated = $value;
+	}
+
+	public function getNondeprecatedPrivateParentProperty() {
+		return $this->privateNonDeprecated;
+	}
+
+	public function setNondeprecatedPrivateParentProperty( $value ) {
+		$this->privateNonDeprecated = $value;
+	}
+
+}