Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

debug: Migrate E_USER_ERROR to throw Error in DeprecationHelper

Browse source 
For a long time now, since PHP 7.0.0, access to non-public properties
has thrown Error, not emitted E_ERROR.

Example - https://3v4l.org/dHChU

Our simulation of this in DeprecationHelper is meant to do what PHP
does, so, given we no longer support PHP 5.6 and can thus construct
Error ourselves, we should do the same.

This is identical - https://3v4l.org/koUqu

== Why ==

Referencing the E_USER_ERROR constant causes a deprecation warning
in PHP 8.4+.

== Change ==

The source change is straight-forward.

One of the tests (testSet, dataset 4 "fallbackGetterOnly") was
previously passing by accident. The source called trigger_error twice,
first with E_USER_DEPRECATED (via wfDeprecated) and then again with
E_USER_ERROR. Given that these are asserted via set_error_handler,
an event calback, the callback is run after the callback finished
(it does not interrupt), at which point only the last values are
reported to the event handler. Improve on this by explicitly hiding
the deprecation warning, and focus the case on testing the error.

Bug: T379445
Change-Id: Ia0aff9906102023370f3907e01962a5e1e369125
(cherry picked from commit d0920b8fb5be462a7aba5c21e47b02c2c2f5025f)
This commit is contained in:
Timo Tijhof 2025-06-11 19:13:47 -07:00 committed by Reedy
parent 20d15ad97f
commit 83d4898319
2 changed files with 52 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
includes/debug/DeprecationHelper.php
View file

@ -20,6 +20,7 @@
namespace MediaWiki\Debug;
use Error;
use ReflectionFunction;
use ReflectionProperty;
@ -217,7 +218,7 @@ trait DeprecationHelper {
$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 );
throw new Error( "Cannot access non-public property $qualifiedName" );
} elseif ( property_exists( $this, $name ) ) {
// Normally __get method will not be even called if the property exists,
// but in tests if we mock an object that uses DeprecationHelper,
@ -249,7 +250,7 @@ trait DeprecationHelper {
} elseif ( property_exists( $this, $name ) ) {
$this->$name = $value;
} else {
trigger_error( "Cannot access non-public property $qualifiedName", E_USER_ERROR );
throw new Error( "Cannot access non-public property $qualifiedName" );
}
return;
}
@ -258,7 +259,7 @@ trait DeprecationHelper {
$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 );
throw new Error( "Cannot access non-public property $qualifiedName" );
} else {
if ( $this->dynamicPropertiesAccessDeprecated ) {
[ $version, $class, $component ] = $this->dynamicPropertiesAccessDeprecated;

69
tests/phpunit/includes/debug/DeprecationHelperTest.php
View file

@ -24,11 +24,20 @@ class DeprecationHelperTest extends MediaWikiIntegrationTestCase {
/**
* @dataProvider provideGet
*/
public function testGet( $propName, $expectedLevel, $expectedMessage ) {
if ( $expectedLevel ) {
public function testGet( $propName, $expectError, string $expectedMessage ) {
if ( $expectError === true ) {
try {
$this->testClass->$propName;
} catch ( Throwable $e ) {
$this->assertInstanceOf( Error::class, $e );
$this->assertSame( $expectedMessage, $e->getMessage() );
return;
}
$this->fail( 'Expected an error' );
} elseif ( $expectError ) {
$this->assertErrorTriggered( function () use ( $propName ) {
$this->assertSame( null, $this->testClass->$propName );
}, $expectedLevel, $expectedMessage );
}, $expectError, $expectedMessage );
} else {
$this->assertDeprecationWarningIssued( function () use ( $propName ) {
$expectedValue = $propName === 'fallbackDeprecatedMethodName' ? 'FOO' : 1;
@ -54,9 +63,9 @@ class DeprecationHelperTest extends MediaWikiIntegrationTestCase {
[ 'fallbackGetterOnly', null,
'Use of TestDeprecatedClass::$fallbackGetterOnly was deprecated in MediaWiki 1.25. ' .
'[Called from DeprecationHelperTest::{closure' ],
[ 'protectedNonDeprecated', E_USER_ERROR,
[ 'protectedNonDeprecated', true,
'Cannot access non-public property TestDeprecatedClass::$protectedNonDeprecated' ],
[ 'privateNonDeprecated', E_USER_ERROR,
[ 'privateNonDeprecated', true,
'Cannot access non-public property TestDeprecatedClass::$privateNonDeprecated' ],
[ 'nonExistent', E_USER_NOTICE, 'Undefined property: TestDeprecatedClass::$nonExistent' ],
];
@ -110,13 +119,19 @@ class DeprecationHelperTest extends MediaWikiIntegrationTestCase {
/**
* @dataProvider provideSet
*/
public function testSet( $propName, $expectedLevel, $expectedMessage, $expectedValue = 0 ) {
public function testSet( $propName, $expectedError, $expectedMessage, $expectedValue = 0 ) {
$this->assertPropertySame( 1, $this->testClass, $propName );
if ( $expectedLevel ) {
$this->assertErrorTriggered( function () use ( $propName ) {
if ( $expectedError === true ) {
// Test the hard error, not the deprecation warning in front of it, tested elsewhere
$this->hideDeprecated( 'TestDeprecatedClass::$fallbackGetterOnly' );
try {
$this->testClass->$propName = 0;
$this->assertPropertySame( 1, $this->testClass, $propName );
}, $expectedLevel, $expectedMessage );
} catch ( Throwable $e ) {
$this->assertSame( $expectedMessage, $e->getMessage() );
$this->assertInstanceOf( Error::class, $e );
return;
}
$this->fail( 'Expected an error' );
} else {
if ( $propName === 'nonExistent' ) {
$this->testClass->$propName = 0;
@ -145,11 +160,11 @@ class DeprecationHelperTest extends MediaWikiIntegrationTestCase {
[ 'fallbackDeprecatedMethodName', null,
'Use of TestDeprecatedClass::$fallbackDeprecatedMethodName was deprecated in MediaWiki 1.26. ' .
'[Called from DeprecationHelperTest::{closure' ],
[ 'fallbackGetterOnly', E_USER_ERROR,
[ 'fallbackGetterOnly', true,
'Cannot access non-public property TestDeprecatedClass::$fallbackGetterOnly' ],
[ 'protectedNonDeprecated', E_USER_ERROR,
[ 'protectedNonDeprecated', true,
'Cannot access non-public property TestDeprecatedClass::$protectedNonDeprecated', 1 ],
[ 'privateNonDeprecated', E_USER_ERROR,
[ 'privateNonDeprecated', true,
'Cannot access non-public property TestDeprecatedClass::$privateNonDeprecated', 1 ],
[ 'nonExistent', null,
'Use of TestDeprecatedClass::$nonExistent was deprecated in MediaWiki 1.23. ' .
@ -177,24 +192,36 @@ class DeprecationHelperTest extends MediaWikiIntegrationTestCase {
public function testSubclassGetSet() {
$fullName = 'TestDeprecatedClass::$privateNonDeprecated';
$this->assertErrorTriggered( function () {
$this->assertErrorCaught( function () {
$this->assertSame( null, $this->testSubclass->getNondeprecatedPrivateParentProperty() );
}, E_USER_ERROR, "Cannot access non-public property $fullName" );
$this->assertErrorTriggered( function () {
}, Error::class, "Cannot access non-public property $fullName" );
$this->assertErrorCaught( function () {
$this->testSubclass->setNondeprecatedPrivateParentProperty( 0 );
$wrapper = TestingAccessWrapper::newFromObject( $this->testSubclass );
$this->assertSame( 1, $wrapper->privateNonDeprecated );
}, E_USER_ERROR, "Cannot access non-public property $fullName" );
}, Error::class, "Cannot access non-public property $fullName" );
$fullName = 'TestDeprecatedSubclass::$subclassPrivateNondeprecated';
$this->assertErrorTriggered( function () {
$this->assertErrorCaught( function () {
$this->assertSame( null, $this->testSubclass->subclassPrivateNondeprecated );
}, E_USER_ERROR, "Cannot access non-public property $fullName" );
$this->assertErrorTriggered( function () {
}, Error::class, "Cannot access non-public property $fullName" );
$this->assertErrorCaught( function () {
$this->testSubclass->subclassPrivateNondeprecated = 0;
$wrapper = TestingAccessWrapper::newFromObject( $this->testSubclass );
$this->assertSame( 1, $wrapper->subclassPrivateNondeprecated );
}, E_USER_ERROR, "Cannot access non-public property $fullName" );
}, Error::class, "Cannot access non-public property $fullName" );
}
protected function assertErrorCaught( callable $callback, $name, $message ) {
$actualClass = $actualMessage = null;
try {
$callback();
} catch ( Throwable $e ) {
$actualClass = get_class( $e );
$actualMessage = $e->getMessage();
}
$this->assertSame( $name, $actualClass );
$this->assertSame( $message, $actualMessage );
}
protected function assertErrorTriggered( callable $callback, $level, $message ) {