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

[WordPress Trac]

#61890: Handle WP_Term known, named 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 changes-requested       |  compatibility
-------------------------------------+-------------------------------------
Changes (by hellofromTonya):

 * keywords:  php82 has-patch has-unit-tests => php82 has-patch has-unit-
     tests changes-requested
 * version:   => 4.4


Comment:

 Should `WP_Term` support dynamic properties?

 I've been pondering this question, especially when thinking about how to
 guide extenders with a deprecation notice in `__get()`. In other classes,
 the deprecation said:
 >A property `{$name}` is not declared. Unsetting a dynamic property is
 >deprecated since version 6.4.0! Instead, declare the property on the
 class.

 How can extenders declare their dynamic property on the class? They can't.
 Hmm, this has me thinking about the above question:

 **Should `WP_Term` support dynamic properties?**

 == Start with what's known

 `WP_Term` is a model of a term. Its properties are joined columns from the
 `$wpdb->terms` and `$wpdb->term_taxonomy` db tables.

 {{{
 CREATE TABLE $wpdb->terms (
  term_id bigint(20) unsigned NOT NULL auto_increment,
  name varchar(200) NOT NULL default '',
  slug varchar(200) NOT NULL default '',
  term_group bigint(10) NOT NULL default 0,

 CREATE TABLE $wpdb->term_taxonomy (
  term_taxonomy_id bigint(20) unsigned NOT NULL auto_increment,
  term_id bigint(20) unsigned NOT NULL default 0,
  taxonomy varchar(32) NOT NULL default '',
  description longtext NOT NULL,
  parent bigint(20) unsigned NOT NULL default 0,
  count bigint(20) NOT NULL default 0,
 }}}

 In Core, it's used in different ways, some of which add additional (known,
 named dynamic) properties. Thus, the term object has a base set of
 database column declared properties and possibly additional properties
 used within the scope and context of the functionality processing the
 term.

 When updating a term (handled in `wp_update_term()`), the class handles
 limiting the properties to only base set of declared properties that
 correlate to the database columns.

 `WP_Term`:
 * Has a base set of declared properties that correlate to the columns in
 `$wpdb->terms` and `$wpdb->term_taxonomy` database tables.
 * It's a `final` class.
 * It allows for dynamic properties.
 * Core adds known, named dynamic properties for use within its
 functionality.

 == Using what's known

 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.

 I'm thinking the answer to the first question of this comment is: Yes,
 `WP_Term` should support dynamic properties for Core and extenders.

 Okay, how does it change the solution?

 == Path Forward

 Going back to the epic ticket #56034:

 >Known use of unknown dynamic properties: Declare the full set of magic
 methods on a class (preferred) or let the class extend stdClass
 (discouraged)

 and in the Proposal section:
 >If the class really should support dynamic properties, the magic methods
 should be implemented on the class itself (or its parent).

 I'm currently thinking: continue to allow `WP_Term` to support dynamic
 properties by adding a full set of magic methods.

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