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

WordPress Trac noreply at wordpress.org
Mon Jul 24 22:34:28 UTC 2023


#58898: Fix WP_Text_Diff_Renderer_Table magic methods for PHP 8.2 dynamic
properties
--------------------------+------------------------------------------------
 Reporter:                |      Owner:  hellofromTonya
  antonvlasenko           |
     Type:  defect (bug)  |     Status:  assigned
 Priority:  normal        |  Milestone:  6.4
Component:                |    Version:  4.0
  Administration          |
 Severity:  normal        |   Keywords:  needs-patch needs-unit-tests php82
  Focuses:                |
--------------------------+------------------------------------------------
 `WP_Text_Diff_Renderer_Table` has a list of compatible fields
 (`compat_fields`), which was introduced at the same time as the magic
 methods. The fields in `compat_fields` list are then not dynamic
 properties, as the magic methods properly handle each. But fields not in
 that list are dynamic properties.

 What happens if a field is not in the list?

 * `__get()` falls through the method.
 * `__set()` nothing happens as it falls through the method.
 * `__isset()` returns `false`, which is correct.
 * `__unset()` nothing happens as it falls through the method.

 The problem:
 When a dynamic property is called on this class, there's no information
 given to the developer. The notice of `_doing_it_wrong()` will provide the
 alert to developers to modify their code.

 Also notice: the `compat_fields` are essentially public properties through
 the magic methods.

 === Proposed Solutions

 As proposed in #58896, 2 ways to fix this class to be compatible with PHP
 8.2's dynamic properties:

 1. **Option 1**: Fix the magic methods:

 * Change 1: Add a notice or `_doing_it_wrong()` to each magic method, i.e.
 to alert and inform developers when attempting to get/set/isset/unset a
 dynamic property.

 * Change 2: Add `return null` in `__get()` when attempting to get a
 dynamic property that is not in the `compat_fields`.

 2. **Option 2**: Add each `compat_fields` as a `public` property and
 remove the magic methods.

 Props to @jrf for identifying the issue.

 References:
 * These changes were
 [https://www.youtube.com/live/vDZWepDQQVE?feature=share&t=10097 discussed
 and agreed to in a livestream] by @jrf @markjaquith and @hellofromTonya.
 * The changes are part of #56034, which is the larger Dynamic Properties
 proposal and initiative.

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


More information about the wp-trac mailing list