[wp-trac] [WordPress Trac] #58897: Fix WP_User_Query magic methods for PHP 8.2 dynamic property

[WordPress Trac]

#58897: Fix WP_User_Query magic methods for PHP 8.2 dynamic property
------------------------------------------------+--------------------------
 Reporter:  antonvlasenko                       |       Owner:
                                                |  hellofromTonya
     Type:  defect (bug)                        |      Status:  assigned
 Priority:  normal                              |   Milestone:  6.4
Component:  Users                               |     Version:  4.0
 Severity:  normal                              |  Resolution:
 Keywords:  needs-patch needs-unit-tests php82  |     Focuses:
------------------------------------------------+--------------------------
Description changed by hellofromTonya:

Old description:

> Changes requested are (which are the same changes requested in #58896):
>
> 1. Change 1: Add a notice to each of the `WP_User_Query` magic methods to
> alert and inform developers when attempting to get/set/isset/unset a
> dynamic property.
>
> `WP_User_Query` 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.
>
> 2. Change 2: Add `return null` in `__get()` when attempting to get a
> dynamic property that is not in the `compat_fields`.
>
> 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.

New description:

 `WP_User_Query` 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/58897#comment:1>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform