Opened 8 months ago
Last modified 2 months ago
#60875 new defect (bug)
Handler proposal for known dynamic properties that are initialized and set late only when getting its value
Reported by: | hellofromTonya | Owned by: | |
---|---|---|---|
Milestone: | 6.8 | Priority: | normal |
Severity: | normal | Version: | |
Component: | General | Keywords: | php82 |
Focuses: | php-compatibility | Cc: |
Description
As noted in #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.
There are multiple instances of known dynamic properties that only get initialized and set when getting their value (i.e. which happens within the __get()
magic method).
For example:
WP_Term
'sdata
(see the code). Tracked in #58087.WP_Block
'sattributes
(see the code).
While [54133] added #[AllowDynamicProperties]
attribute to these classes, that strategy is specifically for unknown dynamic properties and not for known like the above examples.
Strategy
These types of known dynamic properties were intentionally designed to get late / lazily initialized each time code gets the property's value. As this is by design, retaining it must be a priority to ensure not only BC but also to avoid breaking code that depends upon it.
The strategy:
- Retain the design of each property that is late / lazily initialized.
- Declare the property on the class as either private or protected visibility.
- Add
__isset()
magic method to ensureisset()
works correctly (it does not right now). This step is needed for functions likewp_list_pluck()
(see #59774). - Add
__set()
and__unset()
to have a full set of magic methods, but document both to indicate their behavior gets overwritten the next time getting the property's value. - Add
wp_trigger_error()
withE_USER_DEPRECATED
to each of the magic methods for all unknown dynamic properties.
See it in action:
- How it works now https://3v4l.org/VGDNn
- How the strategy will work https://3v4l.org/6gXXU
This ticket is to:
- layout the architectural strategy.
- identify all Core classes with this type of dynamic property.
- implement the strategy for each of instance.
References:
Change History (8)
This ticket was mentioned in Slack in #core by oglekler. View the logs.
6 months ago
#3
@
6 months ago
- Keywords early added
- Milestone changed from 6.6 to 6.7
This ticket is an epic for an overall strategy with implementation tickets, like #58087, implementing the strategy for their specific class.
I think this kind of change needs a long soak time, i.e. early
in a major. As 6.6 is late in its beta, punting this ticket to 6.7 and marking for early
.
This ticket was mentioned in Slack in #core by joemcgill. View the logs.
2 months ago
#6
follow-up:
↓ 7
@
2 months ago
@hellofromTonya was just reviewing this in a scrub of early tickets for the 6.7 milestone and wonder, given that this is a tracking ticket, if it would be best to assign early
to any sub-tasks that are being tracked here and remove the keyword from this tracking ticket if it is likely to roll across milestones?
In the meantime, I'll leave this with you to either to the next milestone when appropriate.
#7
in reply to:
↑ 6
@
2 months ago
- Keywords early removed
Replying to joemcgill:
@hellofromTonya was just reviewing this in a scrub of early tickets for the 6.7 milestone and wonder, given that this is a tracking ticket, if it would be best to assign
early
to any sub-tasks that are being tracked here and remove the keyword from this tracking ticket if it is likely to roll across milestones?
In the meantime, I'll leave this with you to either to the next milestone when appropriate.
Thanks for the ping @joemcgill. Thinking any implementation can benefit for a long soak time. That said, the early
does indeed need to be applied to the sub-task tickets. Removing it from this strategy ticket.
One thing I stumbled upon while working on a new PR that implements this proposal is the need to be careful when adding protected and private properties to existing classes that previously only contained public properties (e.g., didn't have any protected/private properties before).
PHP adds a null byte character to the names of protected and private properties when converting an object to an array, and the
simplexml_load_string()
function throws an error as it can't handle the null byte character.This code snippet illustrates this behavior: https://3v4l.org/XQL4f#v8.3.6 (change the
WP_Term::$data
's visibility to private or protected to reproduce the issue).