[wp-trac] [WordPress Trac] #61890: Handle WP_Term dynamic properties for PHP 8.2

[WordPress Trac]

#61890: Handle WP_Term dynamic properties for PHP 8.2
-------------------------------------+-------------------------------------
 Reporter:  hellofromTonya           |       Owner:  (none)
     Type:  defect (bug)             |      Status:  new
 Priority:  normal                   |   Milestone:  6.7
Component:  Taxonomy                 |     Version:  4.4
 Severity:  minor                    |  Resolution:
 Keywords:  php82 has-patch has-     |     Focuses:  coding-standards, php-
  unit-tests needs-testing needs-    |  compatibility
  dev-note has-testing-info          |
-------------------------------------+-------------------------------------

Old description:

> For PHP 8.2+ compatibility, the scope of this ticket is to handle
> `WP_Term`'s dynamic properties and its incomplete set of magic methods.
>
> As noted in epic ticket #56034:
> >Dynamic (non-explicitly declared) properties are deprecated as of PHP
> 8.2 and are expected to become a fatal error in PHP 9.0, though this last
> part is not 100% certain yet.
> >
> >RFC: https://wiki.php.net/rfc/deprecate_dynamic_properties
>
> The epic ticket provides recommendations for how to handle dynamic
> properties:
>

> == Mitigation Approaches
>
> This ticket offers 2 different approaches. It uses the epic ticket #56034
> as the guide for mitigation.
>
> === **Approach 1: Declare Core's known, named and deprecate dynamic
> properties.**
>
> Per #56034:
> >Situation: Known, named, dynamic property
> >Resolution: Declare the property on the (parent) class
>
> This approach proposes to:
> * Soft remove support of dynamic properties by deprecating getting
> dynamic properties.
> * Identify and declare all of Core's known, named dynamic properties on
> the `WP_Term` class.
>
> Goals:
> * Remove Core's dynamic properties.
> * Alert extenders via deprecation notice and dev note.
> * Extenders to remove their `WP_Term` dynamic properties and replace with
> a different custom handling approach. (Note: need to figure out what to
> recommend to extenders for this migration process.)
>
> Tasks:
>
> 1. Identify all of the known, named dynamic properties Core adds and
> uses.
>
> Beyond the `'data'` dynamic property (previously discussed in #58087),
> Core adds and uses a lot of other known, named dynamic properties. For
> example:
>
> *  `wp_tag_cloud()` adds `link` and `id` dynamic properties.
> * `_make_cat_compat()` adds `cat_ID`, `category_count`,
> `category_description`, `cat_name`, `category_nicename`,
> `category_parent`.
>
> A first step is to identify all of the dynamic properties that Core adds
> and uses.
>
> 2. Determine affects / impacts of the preferred solution
>
> #56034 provides guidance for known, named dynamic properties:
> >Declare the property on the (parent) class
>
> Consideration will be needed for how declaring these properties on the
> class will affect the usage of the `WP_Term::to_array()` method.
>
> If found to have an unwanted affect, the compatible fields approach (e.g.
> `WP_User_Query`) can be explored (see #58897).
>
> === **Approach 2: Add full support for dynamic properties on `WP_Term`**.
>
> Per #56034:
> >Situation: Known use of unknown dynamic properties
> >Solution: Declare the full set of magic methods on a class
> >...
> >If the class really ''**should**'' support dynamic properties, the magic
> methods should be implemented on the class itself (or its parent).
>
> This approach proposes to:
> * Fully support dynamic properties on `WP_Term`.
> * Add a full set of magic methods on the `WP_Term` class.
>
> What if extenders are also adding and using dynamic properties?
> It's been possible since WP_Term was introduced in 2015 in WP 4.4.0 via
> [34997].
>
> Has it been supported since introduction?
> Yes, as Core itself needed it supported.
>
> Since its introduction, this class has supported and actively added /
> used dynamic properties. Core itself active uses and adds dynamic
> properties to `WP_Term` objects. Following its example, extenders also
> add dynamic properties.
>
> With `WP_Term` being marked as `final`, there's not a way for extenders
> to declare their dynamic property needs on a custom child class.
>
> It's reasonable to determine that `WP_Term` should fully support dynamic
> properties. Doing so maintains backward compatibility (BC) with the
> (hopefully) the lowest risks of breaking things for users.
>
> **References**:
> * The changes are part of #56034, which is the larger Dynamic Properties
> proposal and initiative.
> * See #58087 for an extensive discussion and contextual history of the
> `WP_Term::$data` dynamic property.

New description:

 For PHP 8.2+ compatibility, the scope of this ticket is to handle
 `WP_Term`'s dynamic properties and its incomplete set of magic methods.

 As noted in epic ticket #56034:
 >Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2
 and are expected to become a fatal error in PHP 9.0, though this last part
 is not 100% certain yet.
 >
 >RFC: https://wiki.php.net/rfc/deprecate_dynamic_properties

 The epic ticket provides recommendations for how to handle dynamic
 properties:


 == Mitigation Approaches

 This ticket offers 2 different approaches. It uses the epic ticket #56034
 as the guide for mitigation.

 === **Approach 1: Declare Core's known, named and deprecate dynamic
 properties.**

 Per #56034:
 >Situation: Known, named, dynamic property
 >Resolution: Declare the property on the (parent) class

 This approach proposes to:
 * Soft remove support of dynamic properties by deprecating getting dynamic
 properties.
 * Identify and declare all of Core's known, named dynamic properties on
 the `WP_Term` class.

 Goals:
 * Remove Core's dynamic properties.
 * Alert extenders via deprecation notice and dev note.
 * Extenders to remove their `WP_Term` dynamic properties and replace with
 a different custom handling approach. (Note: need to figure out what to
 recommend to extenders for this migration process.)

 Tasks:

 1. Identify all of the known, named dynamic properties Core adds and uses.

 Beyond the `'data'` dynamic property (previously discussed in #58087),
 Core adds and uses a lot of other known, named dynamic properties. For
 example:

 *  `wp_tag_cloud()` adds `link` and `id` dynamic properties.
 * `_make_cat_compat()` adds `cat_ID`, `category_count`,
 `category_description`, `cat_name`, `category_nicename`,
 `category_parent`.

 A first step is to identify all of the dynamic properties that Core adds
 and uses.

 2. Determine affects / impacts of the preferred solution

 #56034 provides guidance for known, named dynamic properties:
 >Declare the property on the (parent) class

 Consideration will be needed for how declaring these properties on the
 class will affect the usage of the `WP_Term::to_array()` method.

 If found to have an unwanted affect, the compatible fields approach (e.g.
 `WP_User_Query`) can be explored (see #58897).

 === **Approach 2: Add full support for dynamic properties on `WP_Term`**.

 Update: [comment:30 Approach 2 was found to not be viable] as it will not
 support native PHP `(array)` type casting.

 Per #56034:
 >Situation: Known use of unknown dynamic properties
 >Solution: Declare the full set of magic methods on a class
 >...
 >If the class really ''**should**'' support dynamic properties, the magic
 methods should be implemented on the class itself (or its parent).

 This approach proposes to:
 * Fully support dynamic properties on `WP_Term`.
 * Add a full set of magic methods on the `WP_Term` class.

 What if extenders are also adding and using dynamic properties?
 It's been possible since WP_Term was introduced in 2015 in WP 4.4.0 via
 [34997].

 Has it been supported since introduction?
 Yes, as Core itself needed it supported.

 Since its introduction, this class has supported and actively added / used
 dynamic properties. Core itself active uses and adds dynamic properties to
 `WP_Term` objects. Following its example, extenders also add dynamic
 properties.

 With `WP_Term` being marked as `final`, there's not a way for extenders to
 declare their dynamic property needs on a custom child class.

 It's reasonable to determine that `WP_Term` should fully support dynamic
 properties. Doing so maintains backward compatibility (BC) with the
 (hopefully) the lowest risks of breaking things for users.

 **References**:
 * The changes are part of #56034, which is the larger Dynamic Properties
 proposal and initiative.
 * See #58087 for an extensive discussion and contextual history of the
 `WP_Term::$data` dynamic property.

--

Comment (by hellofromTonya):

 Description updated to denote Approach 2 was found to not be a viable
 solution.

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