[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:
-------------------------------------+---------------------
Description changed by jrf:

Old description:

> 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
> <?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
> <?php
> $obj = new Foo();
> $obj->type = 'something';
> }}}
>
> === When is something not a dynamic property ?
>
> 1. When the property is explicitly declared on the class.
> 2. When the property is explicitly declared on the (grand)parent class.
>

> === When are dynamic properties not problematic ?
>
> 1. When the class, or one of its parents, has a magic `__set()` method
> which assigns the property to a declared property.
> {{{#!php
> <?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;
>     }
> }
> }}}
> 2. 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,
> which 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:
>
> 1. Introduce two traits into WP Core: `trait DeprecateDynamicProperties`
> and `trait ForbidDynamicProperties`.
>     * The `DeprecateDynamicProperties` trait 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 `ForbidDynamicProperties` trait 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 return `false`, `__get()` will always result in an undefined
> property warning.
> 2. Evaluate every single class in WP Core `src` directory:
>     * 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 `DeprecateDynamicProperties` trait 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).
> 3. 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 `ForbidDynamicProperties` trait. 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.

New description:

 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
 <?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
 <?php
 $obj = new Foo();
 $obj->type = 'something';
 }}}

 === When is something not a dynamic property ?

 1. When the property is explicitly declared on the class.
 2. When the property is explicitly declared on the (grand)parent class.


 === When are dynamic properties not problematic ?

 1. When the class, or one of its parents, has a magic `__set()` method
 which assigns the property to a declared property.
 {{{#!php
 <?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;
     }
 }
 }}}
 2. 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:

 1. Introduce two traits into WP Core: `trait DeprecateDynamicProperties`
 and `trait ForbidDynamicProperties`.
     * The `DeprecateDynamicProperties` trait 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 `ForbidDynamicProperties` trait 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 return `false`, `__get()` will always result in an undefined
 property warning.
 2. Evaluate every single class in WP Core `src` directory:
     * 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 `DeprecateDynamicProperties` trait 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).
 3. 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 `ForbidDynamicProperties` trait. 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.

--

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