Skip to content

Commit 6fe193c

Browse files
Code Modernization: Deprecate dynamic properties in WP_User_Query magic methods.
The unknown use of unknown dynamic property within the `WP_User_Query` property magic methods is now deprecated. A descriptive deprecation notice is provided to alert developers to declare the property on the child class extending `WP_User_Query`. Changes in this commit: * Adds a deprecation notice to the `__get()`, `__set()`, `__isset()`, `__unset()` magic methods, i.e. to alert and inform developers when attempting to get/set/isset/unset a dynamic property. * Fixes `__get()` to explicitly returns `null` when attempting to get a dynamic property. * Fixes `__set()` by removing the value return after setting a declared property, as (a) unnecessary and (b) `__set()` should return `void` [https://www.php.net/manual/en/language.oop5.overloading.php#object.set per the PHP handbook]. * Fixes `__isset()` to return `false` if not in the `$compat_fields`, as `isset()` and `__isset()` should always return `bool`: * [https://www.php.net/manual/en/language.oop5.overloading.php#object.isset `__isset()` in the PHP manual] * [https://www.php.net/manual/en/function.isset.php `isset()` in the PHP manual] * Adds unit tests for happy and unhappy paths. For backward compatibility, no changes are made to the internal declared properties listed in `$compat_fields` and accessed through the magic methods. For example: A child class uses a property named `$data` that is not declared as a property on the child class. When getting its value, e.g. `$user_query->data`, the `WP_User_Query::__get()` magic method is invoked, the following deprecation notice thrown, and `null` returned: >The property `data` is not declared. Setting a dynamic property is deprecated since version 6.4.0! Instead, declare the property on the class. === Why not remove the magic methods, remove the `$compat_fields` property, and restore the properties `public`? tl;dr Backward compatibility. If a plugin adds a property to `$compat_fields` array, then sites using that plugin would experience (a) an `Undefined property` `Warning` (PHP 8) | `Notice` (PHP 7) and (b) a possible change in behavior. === Why not limit the deprecation for PHP versions >= 8.2? tl;dr original design intent and inform The magic methods and `$compat_fields` property were added for one purpose: to continue providing external access to internal properties declared on `WP_User_Query`. They were not intended to be used for dynamic properties. Deprecating that unintended usage both alerts developers a change is needed in their child class and informs them what to change. References: * Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0. * A [https://www.youtube.com/live/vDZWepDQQVE?feature=share&t=10097 live open public working session] where these changes were discussed and agreed to. * [https://wiki.php.net/rfc/deprecate_dynamic_properties PHP RFC: Deprecate dynamic properties.] Related to #14579, #27881, #30891. Follow-up to [15491], [28528], [31144]. Props antonvlasenko, rajinsharwar, jrf, markjaquith, hellofromTonya, SergeyBiryukov, desrosj, peterwilsoncc, audrasjb, costdev, oglekler, jeffpaul. Fixes #58897. See #56034. git-svn-id: https://develop.svn.wordpress.org/trunk@56353 602fd350-edb4-49c9-b593-d223f7449a82
1 parent b92491b commit 6fe193c

2 files changed

Lines changed: 181 additions & 2 deletions

File tree

src/wp-includes/class-wp-user-query.php

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,6 +1109,7 @@ protected function parse_order( $order ) {
11091109
* Makes private properties readable for backward compatibility.
11101110
*
11111111
* @since 4.0.0
1112+
* @since 6.4.0 Getting a dynamic property is deprecated.
11121113
*
11131114
* @param string $name Property to get.
11141115
* @return mixed Property.
@@ -1117,27 +1118,42 @@ public function __get( $name ) {
11171118
if ( in_array( $name, $this->compat_fields, true ) ) {
11181119
return $this->$name;
11191120
}
1121+
1122+
trigger_error(
1123+
"The property `{$name}` is not declared. Getting a dynamic property is " .
1124+
'deprecated since version 6.4.0! Instead, declare the property on the class.',
1125+
E_USER_DEPRECATED
1126+
);
1127+
return null;
11201128
}
11211129

11221130
/**
11231131
* Makes private properties settable for backward compatibility.
11241132
*
11251133
* @since 4.0.0
1134+
* @since 6.4.0 Setting a dynamic property is deprecated.
11261135
*
11271136
* @param string $name Property to check if set.
11281137
* @param mixed $value Property value.
1129-
* @return mixed Newly-set property.
11301138
*/
11311139
public function __set( $name, $value ) {
11321140
if ( in_array( $name, $this->compat_fields, true ) ) {
1133-
return $this->$name = $value;
1141+
$this->$name = $value;
1142+
return;
11341143
}
1144+
1145+
trigger_error(
1146+
"The property `{$name}` is not declared. Setting a dynamic property is " .
1147+
'deprecated since version 6.4.0! Instead, declare the property on the class.',
1148+
E_USER_DEPRECATED
1149+
);
11351150
}
11361151

11371152
/**
11381153
* Makes private properties checkable for backward compatibility.
11391154
*
11401155
* @since 4.0.0
1156+
* @since 6.4.0 Checking a dynamic property is deprecated.
11411157
*
11421158
* @param string $name Property to check if set.
11431159
* @return bool Whether the property is set.
@@ -1146,19 +1162,34 @@ public function __isset( $name ) {
11461162
if ( in_array( $name, $this->compat_fields, true ) ) {
11471163
return isset( $this->$name );
11481164
}
1165+
1166+
trigger_error(
1167+
"The property `{$name}` is not declared. Checking `isset()` on a dynamic property " .
1168+
'is deprecated since version 6.4.0! Instead, declare the property on the class.',
1169+
E_USER_DEPRECATED
1170+
);
1171+
return false;
11491172
}
11501173

11511174
/**
11521175
* Makes private properties un-settable for backward compatibility.
11531176
*
11541177
* @since 4.0.0
1178+
* @since 6.4.0 Unsetting a dynamic property is deprecated.
11551179
*
11561180
* @param string $name Property to unset.
11571181
*/
11581182
public function __unset( $name ) {
11591183
if ( in_array( $name, $this->compat_fields, true ) ) {
11601184
unset( $this->$name );
1185+
return;
11611186
}
1187+
1188+
trigger_error(
1189+
"A property `{$name}` is not declared. Unsetting a dynamic property is " .
1190+
'deprecated since version 6.4.0! Instead, declare the property on the class.',
1191+
E_USER_DEPRECATED
1192+
);
11621193
}
11631194

11641195
/**

tests/phpunit/tests/user/query.php

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2227,4 +2227,152 @@ public function test_returning_field_user_registered() {
22272227
$results = $q->get_results();
22282228
$this->assertNotFalse( DateTime::createFromFormat( 'Y-m-d H:i:s', $results[0] ) );
22292229
}
2230+
2231+
/**
2232+
* @dataProvider data_compat_fields
2233+
* @ticket 58897
2234+
*
2235+
* @covers WP_User_Query::__get()
2236+
*
2237+
* @param string $property_name Property name to get.
2238+
* @param mixed $expected Expected value.
2239+
*/
2240+
public function test_should_get_compat_fields( $property_name, $expected ) {
2241+
$user_query = new WP_User_Query();
2242+
2243+
$this->assertSame( $expected, $user_query->$property_name );
2244+
}
2245+
2246+
/**
2247+
* @ticket 58897
2248+
*
2249+
* @covers WP_User_Query::__get()
2250+
*/
2251+
public function test_should_throw_deprecation_when_getting_dynamic_property() {
2252+
$user_query = new WP_User_Query();
2253+
2254+
$this->expectDeprecation();
2255+
$this->expectDeprecationMessage(
2256+
'The property `undefined_property` is not declared. Getting a dynamic property is ' .
2257+
'deprecated since version 6.4.0! Instead, declare the property on the class.'
2258+
);
2259+
$this->assertNull( $user_query->undefined_property, 'Getting a dynamic property should return null from WP_User_Query::__get()' );
2260+
}
2261+
2262+
/**
2263+
* @dataProvider data_compat_fields
2264+
* @ticket 58897
2265+
*
2266+
* @covers WP_User_Query::__set()
2267+
*
2268+
* @param string $property_name Property name to set.
2269+
*/
2270+
public function test_should_set_compat_fields( $property_name ) {
2271+
$user_query = new WP_User_Query();
2272+
$value = uniqid();
2273+
2274+
$user_query->$property_name = $value;
2275+
$this->assertSame( $value, $user_query->$property_name );
2276+
}
2277+
2278+
/**
2279+
* @ticket 58897
2280+
*
2281+
* @covers WP_User_Query::__set()
2282+
*/
2283+
public function test_should_throw_deprecation_when_setting_dynamic_property() {
2284+
$user_query = new WP_User_Query();
2285+
2286+
$this->expectDeprecation();
2287+
$this->expectDeprecationMessage(
2288+
'The property `undefined_property` is not declared. Setting a dynamic property is ' .
2289+
'deprecated since version 6.4.0! Instead, declare the property on the class.'
2290+
);
2291+
$user_query->undefined_property = 'some value';
2292+
}
2293+
2294+
/**
2295+
* @dataProvider data_compat_fields
2296+
* @ticket 58897
2297+
*
2298+
* @covers WP_User_Query::__isset()
2299+
*
2300+
* @param string $property_name Property name to check.
2301+
* @param mixed $expected Expected value.
2302+
*/
2303+
public function test_should_isset_compat_fields( $property_name, $expected ) {
2304+
$user_query = new WP_User_Query();
2305+
2306+
$actual = isset( $user_query->$property_name );
2307+
if ( is_null( $expected ) ) {
2308+
$this->assertFalse( $actual );
2309+
} else {
2310+
$this->assertTrue( $actual );
2311+
}
2312+
}
2313+
2314+
/**
2315+
* @ticket 58897
2316+
*
2317+
* @covers WP_User_Query::__isset()
2318+
*/
2319+
public function test_should_throw_deprecation_when_isset_of_dynamic_property() {
2320+
$user_query = new WP_User_Query();
2321+
2322+
$this->expectDeprecation();
2323+
$this->expectDeprecationMessage(
2324+
'The property `undefined_property` is not declared. Checking `isset()` on a dynamic property ' .
2325+
'is deprecated since version 6.4.0! Instead, declare the property on the class.'
2326+
);
2327+
$this->assertFalse( isset( $user_query->undefined_property ), 'Checking a dynamic property should return false from WP_User_Query::__isset()' );
2328+
}
2329+
2330+
/**
2331+
* @dataProvider data_compat_fields
2332+
* @ticket 58897
2333+
*
2334+
* @covers WP_User_Query::__unset()
2335+
*
2336+
* @param string $property_name Property name to unset.
2337+
*/
2338+
public function test_should_unset_compat_fields( $property_name ) {
2339+
$user_query = new WP_User_Query();
2340+
2341+
unset( $user_query->$property_name );
2342+
$this->assertFalse( isset( $user_query->$property_name ) );
2343+
}
2344+
2345+
/**
2346+
* @ticket 58897
2347+
*
2348+
* @covers WP_User_Query::__unset()
2349+
*/
2350+
public function test_should_throw_deprecation_when_unset_of_dynamic_property() {
2351+
$user_query = new WP_User_Query();
2352+
2353+
$this->expectDeprecation();
2354+
$this->expectDeprecationMessage(
2355+
'A property `undefined_property` is not declared. Unsetting a dynamic property is ' .
2356+
'deprecated since version 6.4.0! Instead, declare the property on the class.'
2357+
);
2358+
unset( $user_query->undefined_property );
2359+
}
2360+
2361+
/**
2362+
* Data provider.
2363+
*
2364+
* @return array
2365+
*/
2366+
public function data_compat_fields() {
2367+
return array(
2368+
'results' => array(
2369+
'property_name' => 'results',
2370+
'expected' => null,
2371+
),
2372+
'total_users' => array(
2373+
'property_name' => 'total_users',
2374+
'expected' => 0,
2375+
),
2376+
);
2377+
}
22302378
}

0 commit comments

Comments
 (0)