Changeset 56349 for trunk/src/wp-admin/includes/class-wp-list-table.php

Timestamp:

08/02/2023 06:35:24 PM (2 years ago)

Author:

hellofromTonya

Message:

Code Modernization: Deprecate dynamic properties in WP_List_Table magic methods.

The unknown use of unknown dynamic property within the WP_List_Table 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_List_Table.

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.
• Removes returning the value when setting a declared property, as (a) unnecessary and (b) __set() should return void ​per the PHP handbook.
• 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 / defined as a property on the child class. When getting its value, e.g. $list_table->data, the WP_List_Table::__get() magic method is invoked, the following deprecation notice thrown, and null returned:

The property data is not defined. Setting a dynamic (undefined) property is deprecated since version 6.4.0! Instead, define 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.

Several plugins, one of which has over 5M installs, add a property to the $compat_fields array. Removing the property would cause an Undefined property Warning (PHP 8) | Notice (PHP 7) to be thrown. Removing the associated code would change the functionality.

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_List_Table. 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, #22234, #30891.

Follow-up to [15491], [28493], [28521], [28524], [31146].

Props antonvlasenko, jrf, markjaquith, hellofromTonya, SergeyBiryukov, desrosj, peterwilsoncc, audrasjb, costdev, oglekler, jeffpaul.
Fixes #58896.
See #56034.

File:

• trunk/src/wp-admin/includes/class-wp-list-table.php (modified) (7 diffs)

trunk/src/wp-admin/includes/class-wp-list-table.php

@@ -177 +177 @@
      *
      * @since 4.0.0
+     * @since 6.4.0 Getting a dynamic property is deprecated.
      *
      * @param string $name Property to get.
@@ -185 +186 @@
             return $this->$name;
         }
+
+        trigger_error(
+            "The property `{$name}` is not defined. Getting a dynamic (undefined) property is " .
+            'deprecated since version 6.4.0! Instead, define the property on the class.',
+            E_USER_DEPRECATED
+        );
+        return null;
     }
 
@@ -191 +199 @@
      *
      * @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 defined. Setting a dynamic (undefined) property is " .
+            'deprecated since version 6.4.0! Instead, define the property on the class.',
+            E_USER_DEPRECATED
+        );
     }
 
@@ -206 +221 @@
      *
      * @since 4.0.0
+     * @since 6.4.0 Checking a dynamic property is deprecated.
      *
      * @param string $name Property to check if set.
@@ -215 +231 @@
         }
 
+        trigger_error(
+            "The property `{$name}` is not defined. Checking `isset()` on a dynamic (undefined) property " .
+            'is deprecated since version 6.4.0! Instead, define the property on the class.',
+            E_USER_DEPRECATED
+        );
         return false;
     }
@@ -222 +243 @@
      *
      * @since 4.0.0
+     * @since 6.4.0 Unsetting a dynamic property is deprecated.
      *
      * @param string $name Property to unset.
@@ -228 +250 @@
         if ( in_array( $name, $this->compat_fields, true ) ) {
             unset( $this->$name );
-        }
+            return;
+        }
+
+        trigger_error(
+            "A property `{$name}` is not defined. Unsetting a dynamic (undefined) property is " .
+            'deprecated since version 6.4.0! Instead, define the property on the class.',
+            E_USER_DEPRECATED
+        );
     }