[wp-trac] [WordPress Trac] #56034: PHP 8.2: proposal for handling unknown dynamic properties deprecations

[WordPress Trac]

#56034: PHP 8.2: proposal for handling unknown dynamic properties deprecations
-------------------------------------+---------------------
 Reporter:  jrf                      |       Owner:  (none)
     Type:  task (blessed)           |      Status:  new
 Priority:  normal                   |   Milestone:  6.1
Component:  General                  |     Version:
 Severity:  normal                   |  Resolution:
 Keywords:  php82 early 2nd-opinion  |     Focuses:
-------------------------------------+---------------------

Comment (by jrf):

 Replying to [comment:9 peterwilsoncc]:
 > I agree with introducing traits to WP, they were previously avoided due
 to PHP version support but that is no longer an issue.

 @peterwilsoncc Happy to have your support!


 > - is it possible for WPCS to be set up to require all classes use one of
 the traits?

 It is and as you probably already guessed, that is parts of the wider
 /follow-up plan, but I'd like to make four caveats to that:
 1. Not all classes should implement one of these traits. Classes which
 implement the magic methods - `__isset()`, `__get()`, `__set()`,
 `__unset()` - should ''not'' get these traits and adding the trait to a
 class implementing the magic methods should be flagged.
 2. The rule/sniff should only apply to the `src` directory, not to the
 tests.
 3. The sniff will not be able to determine whether something is a ''new''
 class, so it will not be able to enforce that new classes introduced to WP
 Core implement use of the `ForbidDynamicProperties` trait, only that one
 of the two traits ''or'' the full set of magic methods, is implemented.
 4. The sniff should ''only'' apply to WordPress Core, and be explicitly
 excluded for `WordPress-Extra` as plugins/themes should solve this in the
 context of their own code.


 > - in some cases I think it would be wise to continue to support dynamic
 properties so `WpAllowDynamicProperties` would be helpful for forward
 compatibility for situation four.

 I understand where you are coming from, but in my opinion, a generic
 "Allow dynamic properties" trait is **not** the right solution for that.

 Those cases should be "solved" by implementing the magic methods ''on the
 class itself'', preferably in combination with an "allow list" and/or
 "block list" of properties which are allowed to be set/accessed.

 A generic solution via a trait would be problematic as, in that case,
 `private` and `protected` properties would no longer have any meaning as
 everything would become `public`.

 I have a couple of PRs lined up to fix some of the currently existing
 magic methods in Core, which I will pull soonish.

 If the above explanation isn't clear, I think if you look those over, you
 will understand what I mean.


 > The specific circumstance I am thinking of for the latter situation is
 `$wpdb`, some plugin set dynamic properties for table names
 `$wpdb->56034_some_table_name` to allow them to use the DB Security sniffs
 without false positives. (See
 [https://wpdirectory.net/search/01G69FMWBMBKE91SX9PSNQQNC3 search results]
 with a mix of true and false positives.)

 The `wpdb` class actually already contains the magic methods, though
 incorrectly implemented, and fixing those is one of the PRs I have lined
 up already.

 To quote myself from the above ticket:

 > >  It will also give plugin and theme authors plenty of time to notify
 WP Core of Core classes which use the `DeprecateDynamicProperties` trait,
 but should, in all reality, fully support dynamic properties.
 > >  For those cases, after evaluating the merit of the specific use-case
 and finding it valid, the full set of magic methods could then be added to
 the WP Core class and the use of the trait and the attribute removed.


 > I generally support this proposal but I think a decision of whether to
 exclude `DeprecateDynamicProperties` from the backward compatibility
 promise would need to involve project leadership.

 Understood.

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