Build/Test Tools: Remove magic methods from WP_UnitTestCase_Base (without a backward compatibility break).
These magic methods were introduced to prevent a backward compatibility break, but in actual fact:
- Caused a backward compatibility break. The original
$factory property was a static property and this declared property was replaced by the magic methods. Unfortunately, it was not 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 backward compatibility break instead of preventing it.
- Were hiding errors in tests, as the magic methods would be invoked for non-existent properties and would return
null (get) or false (isset). See [54040], [54041], and [54077] for bug fixes related to this.
- Are problematic in relation to PHP 8.2, as the implementation is incomplete, does not protect against dynamic properties and hides PHP notices about undefined properties.
Now, there were several options to mitigate this:
- 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 backward compatibility break.
- Improve the magic methods. With all the issues with magic methods (see the discussion in the livestream from August 16, 2022, this would probably cause more problems than it’s worth and would make for a much more complex implementation, which is over the top for this relatively simple functionality, especially in the context of a test suite.
- Remove the magic methods without adding the property. This would again cause a backward compatibility break, though one for which the mitigation solution would be relatively straightforward, 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 plugin or theme integration tests is outside of our purview and they would still need to deal with this backward compatibility break.
- The current solution: removing the magic methods, explicitly declaring the (non-static) property and setting it in the
set_up() method. This does not constitute a backward compatibility 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: The property is straight away marked as “deprecated”, since the method should be favored over the use of the property.
Reference: PHP Manual: Property overloading: __get()
Follow-up to [35225], [35242].
Props jrf, costdev.
See #56514.