Changeset 56353

Timestamp:

08/03/2023 04:25:25 PM (14 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:
  • ​`__isset()` in the PHP manual
  • ​`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 ​live open public working session where these changes were discussed and agreed to.
• ​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.

Location:

trunk

Files:

• src/wp-includes/class-wp-user-query.php (modified) (7 diffs)
• tests/phpunit/tests/user/query.php (modified) (1 diff)

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

@@ -1110 +1110 @@
      *
      * @since 4.0.0
+     * @since 6.4.0 Getting a dynamic property is deprecated.
      *
      * @param string $name Property to get.
@@ -1118 +1119 @@
             return $this->$name;
         }
+
+        trigger_error(
+            "The property `{$name}` is not declared. Getting a dynamic property is " .
+            'deprecated since version 6.4.0! Instead, declare the property on the class.',
+            E_USER_DEPRECATED
+        );
+        return null;
     }
 
@@ -1124 +1132 @@
      *
      * @since 4.0.0
+     * @since 6.4.0 Setting a dynamic property is deprecated.
      *
      * @param string $name  Property to check if set.
      * @param mixed  $value Property value.
-     * @return mixed Newly-set property.
      */
     public function __set( $name, $value ) {
         if ( in_array( $name, $this->compat_fields, true ) ) {
-            return $this->$name = $value;
-        }
+            $this->$name = $value;
+            return;
+        }
+
+        trigger_error(
+            "The property `{$name}` is not declared. Setting a dynamic property is " .
+            'deprecated since version 6.4.0! Instead, declare the property on the class.',
+            E_USER_DEPRECATED
+        );
     }
 
@@ -1139 +1154 @@
      *
      * @since 4.0.0
+     * @since 6.4.0 Checking a dynamic property is deprecated.
      *
      * @param string $name Property to check if set.
@@ -1147 +1163 @@
             return isset( $this->$name );
         }
+
+        trigger_error(
+            "The property `{$name}` is not declared. Checking `isset()` on a dynamic property " .
+            'is deprecated since version 6.4.0! Instead, declare the property on the class.',
+            E_USER_DEPRECATED
+        );
+        return false;
     }
 
@@ -1153 +1176 @@
      *
      * @since 4.0.0
+     * @since 6.4.0 Unsetting a dynamic property is deprecated.
      *
      * @param string $name Property to unset.
@@ -1159 +1183 @@
         if ( in_array( $name, $this->compat_fields, true ) ) {
             unset( $this->$name );
-        }
+            return;
+        }
+
+        trigger_error(
+            "A property `{$name}` is not declared. Unsetting a dynamic property is " .
+            'deprecated since version 6.4.0! Instead, declare the property on the class.',
+            E_USER_DEPRECATED
+        );
     }
 

trunk/tests/phpunit/tests/user/query.php

@@ -2228 +2228 @@
         $this->assertNotFalse( DateTime::createFromFormat( 'Y-m-d H:i:s', $results[0] ) );
     }
+
+    /**
+     * @dataProvider data_compat_fields
+     * @ticket 58897
+     *
+     * @covers WP_User_Query::__get()
+     *
+     * @param string $property_name Property name to get.
+     * @param mixed $expected       Expected value.
+     */
+    public function test_should_get_compat_fields( $property_name, $expected ) {
+        $user_query = new WP_User_Query();
+
+        $this->assertSame( $expected, $user_query->$property_name );
+    }
+
+    /**
+     * @ticket 58897
+     *
+     * @covers WP_User_Query::__get()
+     */
+    public function test_should_throw_deprecation_when_getting_dynamic_property() {
+        $user_query = new WP_User_Query();
+
+        $this->expectDeprecation();
+        $this->expectDeprecationMessage(
+            'The property `undefined_property` is not declared. Getting a dynamic property is ' .
+            'deprecated since version 6.4.0! Instead, declare the property on the class.'
+        );
+        $this->assertNull( $user_query->undefined_property, 'Getting a dynamic property should return null from WP_User_Query::__get()' );
+    }
+
+    /**
+     * @dataProvider data_compat_fields
+     * @ticket 58897
+     *
+     * @covers WP_User_Query::__set()
+     *
+     * @param string $property_name Property name to set.
+     */
+    public function test_should_set_compat_fields( $property_name ) {
+        $user_query = new WP_User_Query();
+        $value      = uniqid();
+
+        $user_query->$property_name = $value;
+        $this->assertSame( $value, $user_query->$property_name );
+    }
+
+    /**
+     * @ticket 58897
+     *
+     * @covers WP_User_Query::__set()
+     */
+    public function test_should_throw_deprecation_when_setting_dynamic_property() {
+        $user_query = new WP_User_Query();
+
+        $this->expectDeprecation();
+        $this->expectDeprecationMessage(
+            'The property `undefined_property` is not declared. Setting a dynamic property is ' .
+            'deprecated since version 6.4.0! Instead, declare the property on the class.'
+        );
+        $user_query->undefined_property = 'some value';
+    }
+
+    /**
+     * @dataProvider data_compat_fields
+     * @ticket 58897
+     *
+     * @covers WP_User_Query::__isset()
+     *
+     * @param string $property_name Property name to check.
+     * @param mixed $expected       Expected value.
+     */
+    public function test_should_isset_compat_fields( $property_name, $expected ) {
+        $user_query = new WP_User_Query();
+
+        $actual = isset( $user_query->$property_name );
+        if ( is_null( $expected ) ) {
+            $this->assertFalse( $actual );
+        } else {
+            $this->assertTrue( $actual );
+        }
+    }
+
+    /**
+     * @ticket 58897
+     *
+     * @covers WP_User_Query::__isset()
+     */
+    public function test_should_throw_deprecation_when_isset_of_dynamic_property() {
+        $user_query = new WP_User_Query();
+
+        $this->expectDeprecation();
+        $this->expectDeprecationMessage(
+            'The property `undefined_property` is not declared. Checking `isset()` on a dynamic property ' .
+            'is deprecated since version 6.4.0! Instead, declare the property on the class.'
+        );
+        $this->assertFalse( isset( $user_query->undefined_property ), 'Checking a dynamic property should return false from WP_User_Query::__isset()' );
+    }
+
+    /**
+     * @dataProvider data_compat_fields
+     * @ticket 58897
+     *
+     * @covers WP_User_Query::__unset()
+     *
+     * @param string $property_name Property name to unset.
+     */
+    public function test_should_unset_compat_fields( $property_name ) {
+        $user_query = new WP_User_Query();
+
+        unset( $user_query->$property_name );
+        $this->assertFalse( isset( $user_query->$property_name ) );
+    }
+
+    /**
+     * @ticket 58897
+     *
+     * @covers WP_User_Query::__unset()
+     */
+    public function test_should_throw_deprecation_when_unset_of_dynamic_property() {
+        $user_query = new WP_User_Query();
+
+        $this->expectDeprecation();
+        $this->expectDeprecationMessage(
+            'A property `undefined_property` is not declared. Unsetting a dynamic property is ' .
+            'deprecated since version 6.4.0! Instead, declare the property on the class.'
+        );
+        unset( $user_query->undefined_property );
+    }
+
+    /**
+     * Data provider.
+     *
+     * @return array
+     */
+    public function data_compat_fields() {
+        return array(
+            'results'     => array(
+                'property_name' => 'results',
+                'expected'      => null,
+            ),
+            'total_users' => array(
+                'property_name' => 'total_users',
+                'expected'      => 0,
+            ),
+        );
+    }
 }