[wp-trac] [WordPress Trac] #58898: Fix WP_Text_Diff_Renderer_Table magic methods for PHP 8.2 dynamic properties

[WordPress Trac]

#58898: Fix WP_Text_Diff_Renderer_Table magic methods for PHP 8.2 dynamic
properties
-------------------------------------------------+-------------------------
 Reporter:  antonvlasenko                        |       Owner:
                                                 |  hellofromTonya
     Type:  defect (bug)                         |      Status:  closed
 Priority:  normal                               |   Milestone:  6.4
Component:  Administration                       |     Version:  4.0
 Severity:  normal                               |  Resolution:  fixed
 Keywords:  php82 has-patch has-unit-tests       |     Focuses:  php-
  commit                                         |  compatibility
-------------------------------------------------+-------------------------
Changes (by hellofromTonya):

 * status:  reviewing => closed
 * resolution:   => fixed


Comment:

 In [changeset:"56354" 56354]:
 {{{
 #!CommitTicketReference repository="" revision="56354"
 Code Modernization: Deprecate dynamic properties in
 WP_Text_Diff_Renderer_Table magic methods.

 The unknown use of unknown dynamic property within the
 `WP_Text_Diff_Renderer_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_Text_Diff_Renderer_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.
 * 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 a test class with happy and unhappy paths for these changes.

 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_Text_Diff_Renderer_Table::__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_Text_Diff_Renderer_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 [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.]

 Follow-up to [28525], [31135].

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

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/58898#comment:9>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform