Build/Test Tools: Remove magic methods from WP_UnitTestCase_Base (w…

…ithout a backward compatibility break).

These magic methods were introduced to prevent a backward compatibility break, but in actual fact:

1. ''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''**.[[BR]][[BR]]
 > 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 [https://3v4l.org/93HQL not supported in PHP], this effectively created a backward compatibility 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) or `false` (isset). See [54040], [54041], and [54077] for bug fixes related to this.

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

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 backward compatibility break.

2. Improve the magic methods. With all the issues with magic methods (see the discussion in the [https://www.youtube.com/watch?v=vDZWepDQQVE 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.

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

4. 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: [https://www.php.net/manual/en/language.oop5.overloading.php#object.get PHP Manual: Property overloading: __get()]

Follow-up to [35225], [35242].

Props jrf, costdev.
See #56514.

git-svn-id: https://develop.svn.wordpress.org/trunk@54087 602fd350-edb4-49c9-b593-d223f7449a82

tests/phpunit/includes/abstract-testcase.php

@@ -23,15 +23,14 @@ abstract class WP_UnitTestCase_Base extends PHPUnit_Adapter_TestCase {
 	protected static $hooks_saved = array();
 	protected static $ignore_files;
 
-	public function __isset( $name ) {
-		return 'factory' === $name;
-	}
-
-	public function __get( $name ) {
-		if ( 'factory' === $name ) {
-			return self::factory();
-		}
-	}
+	/**
+	 * Fixture factory.
+	 *
+	 * @deprecated 6.1.0 Use the WP_UnitTestCase_Base::factory() method instead.
+	 *
+	 * @var WP_UnitTest_Factory
+	 */
+	protected $factory;
 
 	/**
 	 * Fetches the factory object for generating WordPress fixtures.
@@ -103,6 +102,8 @@ public static function tear_down_after_class() {
 	public function set_up() {
 		set_time_limit( 0 );
 
+		$this->factory = self::factory();
+
 		if ( ! self::$ignore_files ) {
 			self::$ignore_files = $this->scan_user_uploads();
 		}