Opened 4 years ago
Last modified 11 months ago
#56034 accepted task (blessed)
PHP 8.2: proposal for handling unknown dynamic properties deprecations — at Version 4
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Future Release | Priority: | normal |
| Severity: | normal | Version: | |
| Component: | General | Keywords: | php82 |
| Focuses: | php-compatibility | Cc: |
Description (last modified by )
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
This ticket is only intended for situations 3 and 4 (see below) where fixing a typo or explicitly declaring the property is not possible.
The problem
What is a dynamic property ?
<?php class Foo { public $id; public function __construct( $id, $field_name ) { // This is fine, the $id property exists on the class. $this->id = $id; // This is a dynamically created property as the property // is not declared on the class. $this->field_name = $field_name; } }
Dynamic properties can both be created from inside a class, like in the above example, as well as from outside the class (see the below example), which makes these issues very tricky to find via static analysis.
<?php $obj = new Foo(); $obj->type = 'something';
When is something not a dynamic property ?
- When the property is explicitly declared on the class.
- When the property is explicitly declared on the (grand)parent class.
When are dynamic properties not problematic ?
- When the class, or one of its parents, has a magic
__set()method which assigns the property to a declared property.<?php class Foo { private $fields = []; public function __construct($field_name ) { $this->field_name = $field_name; // Not problematic. } public function __set( $name, $value ) { $this->fields[ $name ] = $value; } }
- When the class, or its parent, extends the PHP native
stdClass, which has highly optimized versions of the__set()magic method available.
Mitigation
The solution needed depends on the specific situation and each instance where a deprecation is thrown needs to be individually evaluated.
We can basically categorize dynamic properties into four base situations:
| Situation | Solution | |
|---|---|---|
| 1. | Typo in property name | Fix the typo, either in the property as declared or where the property assignment happens |
| 2. | Known, named, dynamic property | Declare the property on the (parent) class |
| 3. | Known use of unknown dynamic properties | Declare the full set of magic methods on a class (preferred) or let the class extend stdClass (discouraged)
|
| 4. | Unknown use of unknown dynamic properties | Use the #[AllowDynamicProperties] attribute on the (parent) class and/or declare the full set of magic methods on a class and/or let the class extend stdClass
|
Note: the #[AllowDynamicProperties] attribute is expected to be a temporary solution and it is expected that support for the attribute will be removed at some point in the future.
The larger problem
Intended future roadmap for PHP
The deprecation of dynamic properties in PHP is only step 1 in a larger plan to remove support for dynamic properties completely.
No impact analysis was done for the RFC, as even PHP Core realized that identifying all uses of dynamic properties via static analysis is neigh impossible.
With that in mind, the #[AllowDynamicProperties] attribute was introduced to allow for doing an actual impact analysis for the next step of the plan, i.e. removing support for dynamic properties altogether.
Note: when support for dynamic properties would be removed altogether, correctly set up magic __set() methods will still continue to work, including those in the stdClass.
At this time it is unclear when the removal of support for dynamic properties will land, but it is expected to be either in PHP 9.0 or 10.0.
Once it lands, the attribute is expected to no longer be respected.
WordPress infrastructure
Due to the extendable nature of WordPress and its large plugin and theme infrastructure, the problem facing WordPress is exponential as every single class in WordPress may be used and/or extended from within plugins and themes and these plugins/themes may be setting dynamic properties on the WordPress Core classes.
Now, while WP Core at least has tests for a small part of its codebase and runs those diligently, which allows for finding (a subset of the) dynamic properties via the deprecation notices, the majority of plugins/themes do not have any tests.
As noted previously, finding dynamic properties via static analysis is hard, so these plugin/themes would have to largely rely on error logging/user reporting of the PHP 8.2 deprecation notices to fix things and that is still providing the plugin/theme is actively maintained, while a large number of plugins/themes are not.
Also note that the PHP 8.x uptake under WordPress users is low (due to WP Core not being fully compatible) and the number of users logging errors is also low, so user reporting is not a reliable method to allow plugins/themes to get ready for this change.
Altogether, this means that end-users run a significant risk of their sites going down once PHP removes support for dynamic properties and the user upgrades to that PHP version, with little advance notice.
Proposal
Taking all of the above into account, I would like to propose the following:
- Introduce two traits into WP Core:
trait DeprecateDynamicPropertiesandtrait ForbidDynamicProperties.- The
DeprecateDynamicPropertiestrait would contain a full set of the magic methods (__isset(),__get(),__set(),__unset()and will throw a deprecation notice whenever any of those methods are called. Properties will still be set via the magic methods. This trait is intended to be a temporary stop-gap solution and will not fall under the WordPress BC promise. When PHP removes support for dynamic properties, this trait should be removed from WordPress as well. - The
ForbidDynamicPropertiestrait would contain a full set of the magic methods (__isset(),__get(),__set(),__unset()and will throw either a fatal error whenever the__set()method is called. Properties will not be set via the magic methods,__isset()will always returnfalse,__get()will always result in an undefined property warning.
- The
- Evaluate every single class in WP Core
srcdirectory:- If the class - or one of its parents - already has a full set of properly implemented magic methods in place, no action is needed.
- If the class - or one of its parents - already extends
stdClass, no action is needed. - If the class really should support dynamic properties, the magic methods should be implemented on the class itself (or its parent).
- For every single class which does not fall into the above categories, the
DeprecateDynamicPropertiestrait should be added, as well as the#[AllowDynamicProperties]attribute (to allow the class to be recognized for the PHP Core scan for the next RFC).
- Every new class introduced to WP Core after the initial change from step 2, would be required to either have a full set of the magic methods or to add the
ForbidDynamicPropertiestrait. New classes should not be allowed to have the attribute.
If the above proposal is accepted, it would effectively backport the PHP 8.2 deprecation all the way back to PHP 5.6, making it far more likely that users discover any dynamic property related issues in their site.
This will give end-users plenty of time to either contact the plugin/theme owner to mitigate the issue and/or to switch to other plugins/themes if the plugin/theme is no longer actively maintained.
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.
Altogether, this should greatly diminish the dramatic impact of PHP removing support for dynamic properties altogether in a future release.
Practical
I'm perfectly happy to prepare the patch for this together with some people. However, as the proposed patch comes down to a huge amount of work, I want to see some second opinions supporting this proposal first before starting the work on this.
Related: #56009
Trac ticket #56033 is open for addressing dynamic properties in category 1 and 2.
#55357 was marked as a duplicate.