[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          |
-------------------------------------+-------------------------------------

Comment (by adrianduffell):

 Replying to @hellofromTonya

 Thanks for the info on what to audit.

 > If the goal is to guide / nudge extenders to remove their dynamic
 properties, what guidance can be given to them to change their approach?
 How can they handle their extra data needs (which are currently dynamic
 properties)?

 In many cases I think term meta can be used instead:

 {{{#!php
 $term->my_custom_property = 'the_value';
 }}}

 becomes:

 {{{#!php
 add_term_meta( $term->term_id, 'my_custom_property', 'the_value' );
 }}}

 If storing the data in term meta is undesirable, and there is a genuine
 need for extra properties, OOP-wise I think this is a strong  sign a new
 class should be created to properly represent the data. I see an example
 of this in WP's in `wp_tag_cloud` function. It currently adds dynamic
 properties to a `WP_Term` object:

 {{{#!php
 $tags[ $key ]->link = $link;
 $tags[ $key ]->id   = $tag->term_id;
 }}}

 At this point, the object resembles a `WP_Term`, but has morphed into
 something slightly different. A problem arises in the
 `wp_generate_tag_cloud` function, which states in its documentation that
 an array of `WP_Term` objects should be passed to it, but actually expects
 the dynamic properties to be present in the object as well. The problem is
 developers won't know from the documentation to add the dynamic properties
 when using `wp_generate_tag_cloud`. I think it would be better OOP-wise to
 handle the data in a new class, e.g. `WP_Tag`, that explicitly includes
 the extra properties in it's definition along with full documentation
 about them. This way, `wp_generate_tag_cloud` could clearly document its
 requirement for the additional properties just by stating `WP_Tag` objects
 are passed to it.

 > And how might adding Core's known, named (previously dynamic) properties
 affect plugins?

 I don't think it changes anything from the status quo. A plugin might be
 adding a dynamic property with an identical name and creating a clash with
 Core, but this is possible with Core using dynamic properties too. Ideally
 I think Core would benefit from migrating to the approaches above too.

 > what downsides do you anticipate with Approach 2?

 It looks like there is a backwards compatibility break in a scenario where
 dynamic properties are added to `WP_Term`objects and those are being cast
 with `(array)`. However this seems really difficult for plugin devs to
 audit their codebase for instances of it. For example, it doesn't seem
 possible to inspect and  create a log every time (array) is performed on
 WP_Term objects. So while the backwards compatibility break might be
 necessary, it looks difficult to find instances where it could occur.

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