PHP 8.2: proposal for handling unknown dynamic properties deprecations

[jrf]

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 ?

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
   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 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.

[jrf]

#55357 was marked as a duplicate.

[hellofromTonya]

I support this proposal and the reasons for it. @jrf and I discussed the details, impacts, and reasoning a while back.

I also support adding the traits. Though new for Core itself, a trait is the correct approach for what is needed to accomplish this work.

Once there's consensus, @antonvlasenko and I will join in the effort to build it.

[peterwilsoncc]

I agree with introducing traits to WP, they were previously avoided due to PHP version support but that is no longer an issue.

• is it possible for WPCS to be set up to require all classes use one of the traits?
• 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.

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 ​search results with a mix of true and false positives.)

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.

[jrf]

Replying to 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 ​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.

[jrf]

Oh.. and one more caveat:

• Classes which extend another Core class, should be forbidden to implement the trait.

The trait availability will be inherited from parent classes and a child class and a parent class both use-ing the same trait causes conflicts.

[markjaquith]

I support this proposal wholeheartedly.

This seems like the best approach to give plugin authors and site operators maximum warning about the impending full deprecation of dynamic properties.

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.

@jrf Could a sniff leverage PHPDoc @since information to identify classes that are newly-added to WordPress core but have erroneously been given #[AllowDynamicProperties] or the DeprecateDynamicProperties trait?

[peterwilsoncc]

Replying to jrf:

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.

Of course, this makes sense.

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.

All clear, thank you. Limiting the implementation to the two traits you describe makes sense to me.

[chaion07]

Hi, @jrf thank you for reporting this. We reviewed this ticket during a recent bug-scrub session. Here's the summary of the feedback that we received from the team:

1. Added keyword 'needs-patch'
2. So far the discussion shows support for the proposal. It could also be raised in this week's devchat to try to get more eyes on it and see if anyone can get started on the patch(es).

Thanks!

Props: @afragen & @costdev

[hellofromTonya]

There is enough committer support for this proposal to move forward.

@jrf @antonvlasenko and I are planning two 3-day mob programming sessions this month: Aug 22/23/24 and Aug 29/30/31. Session details will be coming including how to join the sessions and the times.

[jrf]

FYI: I've done some deep dive research into getting this set-up and have run into a number of unexpected issues which will block the proposed solution.

So far, I have not been able to find a way round these issues.

@hellofromTonya @markjaquith and me will be discussing what I've found and brainstorming on whether collectively we can still come up with a working solution during a livestream tomorrow.

You can find the livestream (and afterwards the recording) here: ​https://youtu.be/vDZWepDQQVE
We'll start streaming tomorrow Tuesday August 16 round 14:00 UTC.

If you want to read up on some of the research findings, here's a thread with more information: ​https://twitter.com/jrf_nl/status/1558589727766417411

[jrf]

Summary of today's livestream:

Regarding the traits:

• Together we have created a semi-working proof of concept of the ForbidDynamicProperties trait with a huge amount of tests (not all passing yet) and more tests are still needed.
• There are concerns about the performance impact of the current solution. This will need further investigation.
• If we can smooth out the issues/failing tests, we can apply the same principle to the DeprecateDynamicProperties trait.
• As this was nowhere near as straight forward as we originally thought, we've discussed and propose to publish these traits in a separate package - DynamicPropertyUtils - for WP to use as an external dependency. Consider this as a give-back to the wider PHP community as a lot more projects will need to solve this issue in the near future.

Actions items:

• Get a repo opened for the package within the WP GH organisation.
• @markjaquith and me will then publish the code we created and work can then collaboratively continue on smoothing things out.

Regarding the pre-existing magic methods in WP Core:

• We've looked at and did an initial review of all of them. Luckily it is a limited list.
• All of them need work, a number them can/should be removed, but we need to safeguard these change with tests to prevent BC-breaks (and possibly some changes/reverting of declared property visibilities is needed). The work on this should include investigating whether and if so, how the properties "handled" in the magic methods are used by plugins/themes.
• All of the magic method implementations need (additional) tests.
• We also would like to investigate/work on some patterns as examples for how to implement magic methods correctly. We'd like to see if we can transform those patterns into re-usable traits, in which case, they'd be added to the separate package mentioned above.

Regarding timing:

Considering our current findings and the potential impact of any changes on Core and the wider extender field, we very much recommend for this ticket to move to the 6.2 milestone and be marked as early.

Work should continue on creating the patches over the next month or two, to ensure the patches are ready and reviewed ahead of the 6.2 branch opening for commit. They should then be committed as soon as the branch opens to allow for maximum exposure of these changes to real-world testing.

In the interim.…

Open question:
Should we - TEMPORARILY - add the #[AllowDynamicProperties] attribute to all (parent) classes in WP Core for the time being to reduce the volume of notices being logged in error logs for sites using those logs ?

Once we have the proper patches ready, we can then decide if and if so, when to remove the attributes.

Please leave opinions on this in this thread.

[peterwilsoncc]

Replying to jrf:

Open question:
Should we - TEMPORARILY - add the #[AllowDynamicProperties] attribute to all (parent) classes in WP Core for the time being to reduce the volume of notices being logged in error logs for sites using those logs ?

Once we have the proper patches ready, we can then decide if and if so, when to remove the attributes.

Please leave opinions on this in this thread.

Do you have a preference? I'm happy to defer to you, @markjaquith and @hellofromTonya on this.

Core already uses #[ReturnTypeWillChange] to reduce notices so it's not a new pattern.

[jrf]

Replying to peterwilsoncc:

Replying to jrf:
Do you have a preference? I'm happy to defer to you, @markjaquith and @hellofromTonya on this.

Personally I think it will reduce the noise, both in the test runs as well as from people opening issues about the deprecation notices, so I'm in favour of adding the attributes (for the time being).

Core already uses #[ReturnTypeWillChange] to reduce notices so it's not a new pattern.

Well, there is one significant difference with the #[ReturnTypeWillChange] attribute:

For the methods to which we added #[ReturnTypeWillChange], we really can't fix this until the minimum PHP version has gone up. Depending on the method, the minimum PHP version needed is different, for some PHP 7.0 will suffice, for others PHP 8.0 is needed.

For the dynamic properties deprecation, we can fix the code for the most part, but we will need more time to do so and using the attribute "buys" us that time.

[peterwilsoncc]

Replying to jrf:

Personally I think it will reduce the noise, both in the test runs as well as from people opening issues about the deprecation notices, so I'm in favour of adding the attributes (for the time being).

Agreed, adding them to reduce noise makes sense. 👍

Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0.

There are a number of ways to mitigate this:

• If it's an accidental typo for a declared property: fix the typo.
• For known properties: declare them on the class.
• For unknown properties: add the magic __get(), __set() et al methods to the class or let the class extend stdClass which has highly optimized versions of these magic methods build in.
• For unknown _use of_ dynamic properties, the #[AllowDynamicProperties] attribute can be added to the class. The attribute will automatically be inherited by child classes.

Trac ticket 56034 is open to investigate and handle the third and fourth type of situations, however it has become clear this will need more time and will not be ready in time for WP 6.1.

To reduce "noise" in the mean time, both in the error logs of WP users moving onto PHP 8.2, in the test run logs of WP itself, in test runs of plugins/themes, as well as to prevent duplicate issues from being opened for the same, this patch adds the #[AllowDynamicProperties] attribute to all "parent" classes in WP.

The logic used to create the patch is as follows:

• If a class already has the attribute: no action needed.
• If a class does not extend: add the attribute.
• If a class does extend:
  • If it extends stdClass: no action needed (as stdClass supports dynamic properties).
  • If it extends a PHP native class: add the attribute.
  • If it extends a class from one of WP's external dependencies: add the attribute.
• In all other cases: no action - the attribute should not be needed as child classes inherit from the parent.

Whether or not a class contains magic methods has not been taken into account, as a review of the currently existing magic methods has shown that those are generally not sturdy enough and often even set dynamic properties (which they shouldn't).
See the live stream from August 16 for more details.

The patch has been automatically generated (and can be automatically updated (by me) if needs be, with or without updated logic).

The patch has only been created for classes in the WP src directory.

• Tests should not get this attribute, but should be fixed to not use dynamic properties instead. Patches for this are already open under ticket 56033.
• While a number of WP native themes (2014, 2019, 2020, 2021) contain classes, I have not added the attribute to those as those themes are generally maintained elsewhere.

Refs:

• ​https://wiki.php.net/rfc/deprecate_dynamic_properties

Trac ticket: https://core.trac.wordpress.org/ticket/56034

[jrf]

I have opened ​GitHub PR #3123 with a patch to add the #[AllowDynamicProperties] attributes in all the right places.

With this patch (and the ​other open PHP 8.2 patches) in place, there are only four tests still visibly failing on PHP 8.2 issues.
Those tests are all related to the test fixture Basic_Object in tests/phpunit/includes/class-basic-object.php and I intend to pull a separate patch to fix up the magic methods in that test fixture.

Note, there may be some more failing tests being hidden by PHP 8.1 test failures, but it won't be much anymore.

[peterwilsoncc]

I've tested against the Developer Docs Parser and confirmed the PHP Properties don't confuse the parser. It is working as expected.

If you wish to test this it can take a while to do a full scan, so targeting particular files will speed things up. I used WP_Date_Query as a good test as it's well documented both as a class and for the methods.

Meta environment for dev docs is ​https://github.com/WordPress/wporg-developer and the CLI command wp parser create /path/to/wordpress/wp-includes/class-wp-date-query.php --user=1 limits the import.

[jrf]

Thanks for testing that @peterwilsoncc !

I know there was some discussion about this last year when we had to introduce the function-level #[ReturnTypeWillChange] attribute in select places and that @coffee2code was looking into updating the doc generation tooling. I haven't kept track of the status of that, but I'm happy to hear that it all seems to be working without a problem now.

\<rant\>
Magic methods are not supposed to _use_ dynamic properties... they are supposed to _handle_ access _to_ inaccessible (protected or private) or non-existing properties *sigh*.

What's the point of having the magic methods in place otherwise ?
\</rant\>

This is a test fixture (dummy class only used in a test context), which incorrectly implements the magic methods.

What with the deprecation of dynamic properties in PHP 8.2, this needs to be fixed.

The new implementation represents a "proper" implementation of the magic methods for a class without non-public or typed properties.

Notes:

• Instead of relying on dynamic properties, the magic methods now store properties in a private $arbitrary_props array and retrieve them from there as well.
• The original $foo property, even though declared as private, was never private in practice due to the way the magic methods were originally implemented. In effect, it was fully publicly retrievable and modifiable without any (type) restrictions.

With that in mind, the foo property has been moved into the $arbitrary_props array to keep the implementation of the magic methods as clean and straight-forward as possible.
With the adjusted magic methods, access to and modification of $foo will (on the surface) continue to work in the same way as before, while under the hood, it is no longer affected by the dynamic properties deprecation.

• Take note of the use of array_key_exists() instead of isset() in the __get() method. This is intentional and allows for null values to be stored and retrieved.
• Also take note of __set() method no longer returning. __set() is supposed to be a void method.

In practice, the return value would always be ignored due to how PHP handles magic methods, so in effect, this change will not make any difference and does not constitute a BC-break.

The return value of set() is ignored because of the way PHP processes the assignment operator.

Alternatives considered:

• Instead of fixing the magic methods, they could have been removed instead and the class be made to extend stdClass.

I've chosen not to do so for two reasons:

1. It's kind of nice to have at least _one_ correct implementation of magic methods in WP, which can be used as an example to point to as well.
2. Extending stdClass would change the class hierarchy, which _may_ or _may not_ affect the tests using this fixture (depending on what's being done with the class).

Extending stdClass would also obfuscate what's going on in the class and would require extensive documentation to prevent the extension being inadvertently removed at a future point in time.

• Instead of fixing the magic methods, the test fixture could have been deprecate and/or removed, with the few tests which use the fixture being updated to use stdClass for their test fixture instead.

I've chosen not to do so as there may well be external (plugin/theme) tests relying on this test fixtures and evaluating if this is so, would be hard as WP Directory cannot be used, as test code is normally not included in the code published on wp.org.
Also note, there is still a (deprecated) Basic_Subclass fixture in the test suite, which extends this class.

Refs:

• ​https://www.php.net/manual/en/language.oop5.overloading.php#object.set
• ​https://wiki.php.net/rfc/deprecate_dynamic_properties
• php/php-src#7786

These magic methods and the Basic_Object test fixture were originally introduced in [28480] and [28523].
The fixture was deprecated in [42381] and undeprecated again in [45807].

At this time, the test fixture is used in the WP_Test_REST_Post_Meta_Fields and the Tests_REST_API test classes.

Trac ticket: https://core.trac.wordpress.org/ticket/56034

### PHP 8.2 | WP_UnitTestCase_Base: remove magic methods (without BC break)

⚠️ This PR depends on PR 3127, 3128 and 3129, which should be merged first (as otherwise the tests will start failing). ⚠️

Introduced in [35242] in response to Trac#30017 and Trac#33968.

These magic methods were introduced to prevent a BC-break, but in actual fact:

1. _Caused_ a BC break.

The original $factory property was a static property and this declared property was replaced by the magic methods.
Unfortunately, it wasn't realized at the time, that these magic methods _are not called for static property access_.

Property overloading only works in object context. These magic methods will not be triggered in static context.

And as approaching a static property in a non-static manner is ​not supported in PHP, this effectively created a BC-break instead of preventing it.

2. Were hiding errors in tests...

... as the magic methods would be invoked for non-existent properties and would return null (get)/false (isset).
See PRs 3127, 3128 and 3129 for bug fixes related to this.

3. Are problematic in relation to PHP 8.2...

... as the implementation is incomplete, doesn't protect against dynamic properties and hides PHP notices about undefined properties.

Now, there were several options to mitigate this:

1. Revert the original commit.

This would be problematic, as the _non-static_ version of these properties has now been supported for 7 years, so this would create a new BC-break.

2. Improve the magic methods.

With all the issues we've seen with magic methods (see the discussion in the ​livestream from August 16), this would probably cause more problems than its worth and would make for a much more complex implementation, which is over the top for this relative simple functionality, especially in the context of a test suite.

3. Remove the magic methods without adding the property.

This would again cause a BC-break, though one for which the mitigation solution would be relatively straight-forward. I.e. to replace property access using $this->factory with a function call $this->factory() (or self::factory() as the method is declared as static).
While we can (and have in a subsequent commit) mitigate this for the WP Core test suite, mitigating this for integration tests for plugins/themes is outside of our purview and they would still need to deal with this BC-break.

4. The current solution: removing the magic methods and explicitly declaring the (non-static) property and setting it in the set_up() method.

This does not constitute a BC-break with the functionality as it was over the past 7 years.
Setting the property in set_up() may be "late", but that is the earliest place in which the property can be set as non-static.
If the factory would be needed prior to set_up(), the (static) WP_UnitTestCase_Base::factory() method should be called directly. This is no different from how this functionality behaved over the past 7 years.

Note: I've straight-away marked the property as "deprecated" as the method should be favoured over the use of the property.

Refs:

• ​https://www.php.net/manual/en/language.oop5.overloading.php#object.get

---

While working on the above commit, I have investigated how the factory method and property were used in the WP Core test suite.

These were my findings:

• Direct use of the property in tests $this->factory->...: 614 instances in 53 test files
• Direct use of the method in tests (non-static) $this->factory()->...: 131 instances in 38 test files
• Direct use of the method in tests (static) [self|static|parent]::factory()->...: 3441 instances in 268 test files

The use of static function calls to the factory() method clearly win out and this is in line with the original intention of the changes which was made in [35242].

With that in mind, I've created the two below commits which fix the non-static usages of the method and usages of the property within the WP Core test suite.

---

### Tests: correctly use the factory method

This replaces all non-static calls to the WP_UnitTestCase_Base::factory() method with static function calls as the method is declared as static.

This is a consistency improvement for the test suite.

Follow up to [35242].

### Tests: use the factory method instead of the property

This replaces all references to the WP_UnitTestCase_Base::$factory property with static function calls to the WP_UnitTestCase_Base::factory() method.

This is a consistency improvement for the test suite.

Follow up to [35242].

Trac ticket: https://core.trac.wordpress.org/ticket/56034 (first commit)
Trac ticket: https://core.trac.wordpress.org/ticket/55652 (second + third commit)

[jrf]

I've opened ​GitHub PR 3126 and ​GitHub PR 3130 to fix/remove the pre-existing magic methods in the test suite.

Please see the commit messages for full details.

[jrf]

Oh and just as a point of order: the three currently open PRs are intended for WP 6.1, while the bigger changes (with the traits) will be postponed to WP 6.2.

To prevent these three patches being overlooked for WP 6.1, I'll put this ticket back in the 6.1 milestone.

[peterwilsoncc]

@jrf Are you able to create a follow up ticket for the bigger changes so the milestone history for the PHP property changes aren't lost when reviewing code history at a later date?

[jrf]

@peterwilsoncc You mean create a duplicate of this ticket ? Can do.

Personally, I find that even worse for tracing history as you then have to go through discussions in multiple tickets and have to try and piece things together, but whatever. 🤷🏻‍♀️

I've created a new Trac ticket for WP 6.1 specifically for this patch: https://core.trac.wordpress.org/ticket/56513

I've created a new Trac ticket for WP 6.1 specifically for this patch (and the one in #3130): https://core.trac.wordpress.org/ticket/56514

I've created a new Trac ticket for WP 6.1 specifically for this patch (and the one in #3126): https://core.trac.wordpress.org/ticket/56514

[jrf]

Replying to peterwilsoncc:

Are you able to create a follow up ticket for the bigger changes so the milestone history for the PHP property changes aren't lost when reviewing code history at a later date?

I've opened two additional tickets:

1. Trac #56513 to add the attributes in WP 6.1.
2. Trac #56514 to fix the magic methods used in the test suite in WP 6.1.

I've also linked the currently open PRs to those new tickets.

With that set up, this ticket will now be moved out of the WP 6.1 milestone again and back to WP 6.2

(In other words: this ticket stays open for the bigger changes, the smaller, currently actionable changes have been moved to other Trac tickets).

Rebased without changes now all three of the PRs which needed to be merged first, have been. This should now show a passing build. (unless new issues were introduced in the mean time....)

Thanks for the PR! Merged in r54087, r54088, and r54090.

Thanks @SergeyBiryukov !

Thanks for the PR! Merged in r54095.

This looks good to me. 👍 I only have one question so far:

While a number of WP native themes (2014, 2019, 2020, 2021) contain classes, I have not added the attribute to those as those themes are generally maintained elsewhere.

I'm not quite sure what that means, they may have been developed on GitHub initially, but the repos are archived after RC stage and the themes are subsequently maintained on Trac with the rest of core. So it seems like they should be included too?

Thanks for the PR! Merged in r54117.

Thanks for the PR! Merged in r54117.

[SergeyBiryukov]

In 54133:

Code Modernization: Add AllowDynamicProperties attribute to all (parent) classes.

Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0.

There are a number of ways to mitigate this:

• If it is an accidental typo for a declared property: fix the typo.
• For known properties: declare them on the class.
• For unknown properties: add the magic __get(), __set(), et al. methods to the class or let the class extend stdClass which has highly optimized versions of these magic methods built in.
• For unknown use of dynamic properties, the #[AllowDynamicProperties] attribute can be added to the class. The attribute will automatically be inherited by child classes.

Trac ticket #56034 is open to investigate and handle the third and fourth type of situations, however it has become clear this will need more time and will not be ready in time for WP 6.1.

To reduce “noise” in the meantime, both in the error logs of WP users moving onto PHP 8.2, in the test run logs of WP itself, in test runs of plugins and themes, as well as to prevent duplicate tickets from being opened for the same issue, this commit adds the #[AllowDynamicProperties] attribute to all “parent” classes in WP.

The logic used for this commit is as follows:

• If a class already has the attribute: no action needed.
• If a class does not extend: add the attribute.
• If a class does extend:
  • If it extends stdClass: no action needed (as stdClass supports dynamic properties).
  • If it extends a PHP native class: add the attribute.
  • If it extends a class from one of WP's external dependencies: add the attribute.
• In all other cases: no action — the attribute should not be needed as child classes inherit from the parent.

Whether or not a class contains magic methods has not been taken into account, as a review of the currently existing magic methods has shown that those are generally not sturdy enough and often even set dynamic properties (which they should not). See the ​live stream from August 16, 2022 for more details.

This commit only affects classes in the src directory of WordPress core.

• Tests should not get this attribute, but should be fixed to not use dynamic properties instead. Patches for this are already being committed under ticket #56033.
• While a number bundled themes (2014, 2019, 2020, 2021) contain classes, they are not a part of this commit and may be updated separately.

Reference: ​PHP RFC: Deprecate dynamic properties.

Follow-up to [53922].

Props jrf, hellofromTonya, markjaquith, peterwilsoncc, costdev, knutsp, aristath.
See #56513, #56034.

Thanks for the PR! Merged the current patch in r54133, bundled themes may be updated separately if needed.

Closing this PR as the changes have been merged, let's open a new one for bundled themes if needed.

[SergeyBiryukov]

In 54481:

Code Modernization: Add AllowDynamicProperties attribute to recently introduced classes.

This commit is a follow-up to [54133] for new classes introduced in WordPress 6.1 since the previous commit.

Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0.

There are a number of ways to mitigate this:

• If it is an accidental typo for a declared property: fix the typo.
• For known properties: declare them on the class.
• For unknown properties: add the magic __get(), __set(), et al. methods to the class or let the class extend stdClass which has highly optimized versions of these magic methods built in.
• For unknown use of dynamic properties, the #[AllowDynamicProperties] attribute can be added to the class. The attribute will automatically be inherited by child classes.

Trac ticket #56034 is open to investigate and handle the third and fourth type of situations, however it has become clear this will need more time and will not be ready in time for WP 6.1.

To reduce “noise” in the meantime, both in the error logs of WP users moving onto PHP 8.2, in the test run logs of WP itself, in test runs of plugins and themes, as well as to prevent duplicate tickets from being opened for the same issue, the #[AllowDynamicProperties] attribute has been added to all “parent” classes in WP.

Reference: ​PHP RFC: Deprecate dynamic properties.

Follow-up to [53922], [54133].

Props jrf.
See #56513, #56034.

[hellofromTonya]

All work for 6.2 has been committed. The next phase of this ticket will be handled in 6.3, preferably as early as possible in the 6.3 alpha cycle.

That said, if a patch is ready before 6.2 RC1, then please consider moving the ticket back into the 6.2 milestone.

[hellofromTonya]

In 55580:

Code Modernization: Fix dynamic properties in WP_Admin_Bar.

To fix the dynamic properties, the following changes are included:

• Removes WP_Admin_Bar::__get().
• Declares menu as a property on the class, deprecates it, and initializes it to an empty array.
• Removes the unused 'proto' dynamic property.

Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0.

Why remove the WP_Admin_Bar::__get() magic method?

tl;dr
The magic method is no longer needed.

The magic method only handled the menu and proto dynamic properties. Introducing a full set of magic methods is overkill for this class. Instead of having to maintain magic methods, this changeset instead directly addresses the 2 properties (see below).

Why declare the menu property on the class?

tl;dr
To simplify the code while maintaining backwards compatibility for extenders who are using this deprecated property.

The menu property was introduced during the 3.1.0 development cycle as a declared property [15671]. Its purpose was to internally hold the menu structure.

During the WP 3.3.0 development cycle, it was replaced by a new private property called nodes (see [19120]).

But breakage reports from extenders caused it to be restored. [19501] added the __get() magic method, i.e. for handling it as a dynamic property, and deprecated it.

We're not going to maintain compat for $menu. Suggest we make it array() and plugins will have to deal. We can throw a _deprecated_argument() and push them to use the new methods.

~ Source: see #19371 comment 17

​A search of the wp.org plugins and themes repository shows that a few plugins are still using this deprecated property. To maintain backwards compatibility, menu is moved back to the class as a declared property, set to an empty array (as it's been since 3.3.0), and deprecated in the property's description.

Why remove the proto dynamic property?

tl;dr

• It was not intended to be released in 3.1.
• There are no usages of it in Core or in the WP.org's plugin or theme directories.
• It should be safe to remove.

This property was first introduced in the WP 3.1.0 development cycle to replace the PROTO constant (see [16038]) for protocol handling for the admin bar's hyperlinks. [16077] replaced the property's usages with URL functions such as get_admin_url() and admin_url(). But it missed removing the property, which was no longer needed or used.

It was relocated to the __get() magic method as a dynamic property when the menu property was restored (see [19501]).

A search of WP.org's plugins and themes repositories shows no usages of the property. Core hasn't used it since the removed in [16038] before 3.1 final release. It should be safe to remove it, but committing very early in the 6.3 alpha cycle to give time for reports of usages, if there are any.

References:

• A ​live open public working session where these changes were discussed and agreed to.
• ​PHP RFC: Deprecate dynamic properties.

Follow-up to [19501], [19120], [16308], [16038], [15671].

Props antonvlasenko, hellofromTonya, jrf, markjaquith, desrosj, ironprogrammer, peterwilsoncc, SergeyBiryukov.
See #56876, #56034.

[spacedmonkey]

In 56161:

Build/Test: Fix dynamic property deprecation warning in object cache drop-in.

Fix deprecation warning for dynamic property in object cache drop-in used in core unit tests.

Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0.

Props spacedmonkey, johnbillion.
Fixes #58736.
See #56034.

[chaion07]

Thanks @jrf for continuing to work on this. We reviewed this ticket during a recent bug-scrub session and here's the summary of the discussion:

1. The Team wants to make sure that all necessary steps have been completed. It seems that just the parts that have been tackled so far is done.
2. The Team is inquiring if the planned work has been completed since it is hard to understand from the aforementioned comments.
3. @hellofromTonya will surely take care of this one.

Props to @mukesh27 & @hareesh-pillai

[hellofromTonya]

This ticket is not yet complete. With 6.3 now in RC, punting to 6.4.

[hellofromTonya]

Adding the new php-compatibility focus, which identifies this ticket as a compatibility issue / task. php82 keyword identifies that PHP 8.2 is the first PHP version that introduced this incompatibility.

[hellofromTonya]

In 56349:

Code Modernization: Deprecate dynamic properties in WP_List_Table magic methods.

The unknown use of unknown dynamic property within the WP_List_Table property magic methods is now deprecated. A descriptive deprecation notice is provided to alert developers to declare the property on the child class extending WP_List_Table.

Changes in this commit:

• Adds a deprecation notice to the __get(), __set(), __isset(), __unset() magic methods, i.e. to alert and inform developers when attempting to get/set/isset/unset a dynamic property.
• Fixes __get() to explicitly returns null when attempting to get a dynamic property.
• Removes returning the value when setting a declared property, as (a) unnecessary and (b) __set() should return void ​per the PHP handbook.
• Adds unit tests for happy and unhappy paths.

For backward compatibility, no changes are made to the internal declared properties listed in $compat_fields and accessed through the magic methods.

For example:
A child class uses a property named $data that is not declared / defined as a property on the child class. When getting its value, e.g. $list_table->data, the WP_List_Table::__get() magic method is invoked, the following deprecation notice thrown, and null returned:

The property data is not defined. Setting a dynamic (undefined) property is deprecated since version 6.4.0! Instead, define the property on the class.

Why not remove the magic methods, remove the $compat_fields property, and restore the properties public?

tl;dr Backward compatibility.

Several plugins, one of which has over 5M installs, add a property to the $compat_fields array. Removing the property would cause an Undefined property Warning (PHP 8) | Notice (PHP 7) to be thrown. Removing the associated code would change the functionality.

Why not limit the deprecation for PHP versions >= 8.2?

tl;dr original design intent and inform

The magic methods and $compat_fields property were added for one purpose: to continue providing external access to internal properties declared on WP_List_Table. They were not intended to be used for dynamic properties.

Deprecating that unintended usage both alerts developers a change is needed in their child class and informs them what to change.

References:

• Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0.
• A ​live open public working session where these changes were discussed and agreed to.
• ​PHP RFC: Deprecate dynamic properties.

Related to #14579, #22234, #30891.

Follow-up to [15491], [28493], [28521], [28524], [31146].

Props antonvlasenko, jrf, markjaquith, hellofromTonya, SergeyBiryukov, desrosj, peterwilsoncc, audrasjb, costdev, oglekler, jeffpaul.
Fixes #58896.
See #56034.

[hellofromTonya]

In 56353:

Code Modernization: Deprecate dynamic properties in WP_User_Query magic methods.

The unknown use of unknown dynamic property within the WP_User_Query property magic methods is now deprecated. A descriptive deprecation notice is provided to alert developers to declare the property on the child class extending WP_User_Query.

Changes in this commit:

• Adds a deprecation notice to the __get(), __set(), __isset(), __unset() magic methods, i.e. to alert and inform developers when attempting to get/set/isset/unset a dynamic property.
• Fixes __get() to explicitly returns null when attempting to get a dynamic property.
• Fixes __set() by removing the value return after setting a declared property, as (a) unnecessary and (b) __set() should return void ​per the PHP handbook.
• Fixes __isset() to return false if not in the $compat_fields, as isset() and __isset() should always return bool:
  • ​`__isset()` in the PHP manual
  • ​`isset()` in the PHP manual
• Adds unit tests for happy and unhappy paths.

For backward compatibility, no changes are made to the internal declared properties listed in $compat_fields and accessed through the magic methods.

For example:
A child class uses a property named $data that is not declared as a property on the child class. When getting its value, e.g. $user_query->data, the WP_User_Query::__get() magic method is invoked, the following deprecation notice thrown, and null returned:

The property data is not declared. Setting a dynamic property is deprecated since version 6.4.0! Instead, declare the property on the class.

Why not remove the magic methods, remove the $compat_fields property, and restore the properties public?

tl;dr Backward compatibility.

If a plugin adds a property to $compat_fields array, then sites using that plugin would experience (a) an Undefined property Warning (PHP 8) | Notice (PHP 7) and (b) a possible change in behavior.

Why not limit the deprecation for PHP versions >= 8.2?

tl;dr original design intent and inform

The magic methods and $compat_fields property were added for one purpose: to continue providing external access to internal properties declared on WP_User_Query. They were not intended to be used for dynamic properties.

Deprecating that unintended usage both alerts developers a change is needed in their child class and informs them what to change.

References:

• Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0.
• A ​live open public working session where these changes were discussed and agreed to.
• ​PHP RFC: Deprecate dynamic properties.

Related to #14579, #27881, #30891.

Follow-up to [15491], [28528], [31144].

Props antonvlasenko, rajinsharwar, jrf, markjaquith, hellofromTonya, SergeyBiryukov, desrosj, peterwilsoncc, audrasjb, costdev, oglekler, jeffpaul.
Fixes #58897.
See #56034.

[hellofromTonya]

In 56354:

Code Modernization: Deprecate dynamic properties in WP_Text_Diff_Renderer_Table magic methods.

The unknown use of unknown dynamic property within the WP_Text_Diff_Renderer_Table property magic methods is now deprecated. A descriptive deprecation notice is provided to alert developers to declare the property on the child class extending WP_Text_Diff_Renderer_Table.

Changes in this commit:

• Adds a deprecation notice to the __get(), __set(), __isset(), __unset() magic methods, i.e. to alert and inform developers when attempting to get/set/isset/unset a dynamic property.
• Fixes __get() to explicitly returns null when attempting to get a dynamic property.
• Fixes __set() by removing the value return after setting a declared property, as (a) unnecessary and (b) __set() should return void ​per the PHP handbook.
• Fixes __isset() to return false if not in the $compat_fields, as isset() and __isset() should always return bool:
  • ​`__isset()` in the PHP manual
  • ​`isset()` in the PHP manual
• Adds a test class with happy and unhappy paths for these changes.

For backward compatibility, no changes are made to the internal declared properties listed in $compat_fields and accessed through the magic methods.

For example:
A child class uses a property named $data that is not declared as a property on the child class. When getting its value, e.g. $user_query->data, the WP_Text_Diff_Renderer_Table::__get() magic method is invoked, the following deprecation notice thrown, and null returned:

The property data is not declared. Setting a dynamic property is deprecated since version 6.4.0! Instead, declare the property on the class.

Why not remove the magic methods, remove the $compat_fields property, and restore the properties public?

tl;dr Backward compatibility.

If a plugin adds a property to $compat_fields array, then sites using that plugin would experience (a) an Undefined property Warning (PHP 8) | Notice (PHP 7) and (b) a possible change in behavior.

Why not limit the deprecation for PHP versions >= 8.2?

tl;dr original design intent and inform

The magic methods and $compat_fields property were added for one purpose: to continue providing external access to internal properties declared on WP_Text_Diff_Renderer_Table. They were not intended to be used for dynamic properties.

Deprecating that unintended usage both alerts developers a change is needed in their child class and informs them what to change.

References:

• Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0.
• A ​live open public working session where these changes were discussed and agreed to.
• ​PHP RFC: Deprecate dynamic properties.

Follow-up to [28525], [31135].

Props antonvlasenko, rajinsharwar, jrf, markjaquith, hellofromTonya, SergeyBiryukov, desrosj, peterwilsoncc, audrasjb, costdev, oglekler, jeffpaul.
Fixes #58898.
See #56034.

[hellofromTonya]

6.4 RC1 is next week. I think the remaining work to resolve this ticket needs more time than is left in the 6.4 beta cycle.

Will punting it impact removing the "beta support" label from PHP 8.2 for compatibility?
No.

Why? PHP 8.2 usage percentage ("enough sites") is ​currently at ~4.5%, which is ​below the 10% usage percentage requirement for the next tier of compatibility. But that said, it is gaining usage indicating this work in this ticket will need to get resolved in 6.5 or 6.6.

[hellofromTonya]

In 56938:

Code Modernization: Declare dynamic properties on WP_Text_Diff_Renderer_Table.

Core uses 3 known, named, dynamic properties on the WP_Text_Diff_Renderer_Table class:

• _title
• _title_left
• _title_right

When reviewing revisions (within the admin UI), wp_text_diff() passes the arguments (without the leading _) to a new instance, which raised deprecation notices (see [56354]).

Note: the parent class adds the leading _ to each of these properties (see [7747]).

The deprecation notices are resolved by declaring each of these known, named, dynamic properties on the class, per the #56034 mitigation strategy. These new properties are not initialized to retain their previous null behavior.

Follow-up to [56354], [23506], [7747].

Props wildworks, antonvlasenko, hellofromTonya, ironprogrammer, kafleg, mukesh27, nicolefurlan, presskopp, sabernhardt.
Fixes #59431.
See #58898, #56034.

[swissspidy]

@hellofromTonya What's left to be done here, and can that be done in 6.5? If not, I'd suggest punting or closing depending on the status.

[antonvlasenko]

I'd like to highlight a point not previously mentioned in the discussion.
When implementing the missing magic methods in the class, it may be beneficial to allow setting of public properties that were previously unset but now need to be assigned new values. Here’s an example to clarify:

class Foo {
    public $property;
}

$foo = new Foo();
unset( $foo->property );
$foo->property = 'some_value'; // Note that PHP doesn't trigger a deprecation error, so the magic method shouldn't throw one either.

If the __set method was defined for the Foo class, it should have allowed Foo::$property to be reassigned, even if it had been previously unset.
This might help improve backward compatibility while ensuring that dynamic properties are properly handled.

For a more detailed illustration of this idea, please see the Baz::__set() method in following code snippet: ​https://3v4l.org/QU0tT#v8.3.6.

This is also true for other magic methods, such as __get.

[jrf]

Replying to antonvlasenko:

I'd like to highlight a point not previously mentioned in the discussion.
When implementing the missing magic methods in the class, it may be beneficial to allow setting of public properties that were previously unset but now need to be assigned new values.

@antonvlasenko More than anything, that exactly the kind of situation why I have been advocating that we should try to _get rid of_ magic methods instead of fixing/introducing them....

[hellofromTonya]

No work on this happened during 6.6. Moving to 6.7.

[hellofromTonya]

In 59058:

Code Modernization: Explicitly declare all properties in AtomParser.

Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0.

There are a number of ways to mitigate this:

• If it's an accidental typo for a declared property: fix the typo.
• For known properties: declare them on the class.
• For unknown properties: add the magic __get(), __set() et al methods to the class or let the class extend stdClass which has highly optimized versions of these magic methods build in.
• For unknown _use of_ dynamic properties, the #[AllowDynamicProperties] attribute can be added to the class. The attribute will automatically be inherited by child classes.

In this case, the property added are explicitly referenced in this class, so fall in the "known property" category.

Refs:

• ​https://wiki.php.net/rfc/deprecate_dynamic_properties

Props jrf.
See #56034.

[hellofromTonya]

A few sub-task tickets were punted to 6.8 as their solutions need to be ready for early alpha.

As this ticket is an epic ticket that lays out the subtasks and provides guidance, it spans multiple majors. To help the triage / tech lead folks in a major release squad, moving it to Future Release. The subtasks will be individually milestoned.

Once all of the subtasks are done, this ticket can be closed.