Make WordPress Core


Ignore:
Timestamp:
08/03/2023 04:25:25 PM (15 months ago)
Author:
hellofromTonya
Message:

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 per the PHP handbook.
  • Fixes __isset() to return false if not in the $compat_fields, as isset() and __isset() should always return bool:
  • 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:

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.

File:
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/tests/phpunit/tests/user/query.php

    r55745 r56353  
    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}
Note: See TracChangeset for help on using the changeset viewer.