[wp-trac] [WordPress Trac] #56514: PHP 8.2: fix magic method use within the test suite

[WordPress Trac]

#56514: PHP 8.2: fix magic method use within the test suite
--------------------------------------------+---------------------
 Reporter:  jrf                             |       Owner:  (none)
     Type:  defect (bug)                    |      Status:  new
 Priority:  normal                          |   Milestone:  6.1
Component:  General                         |     Version:  trunk
 Severity:  normal                          |  Resolution:
 Keywords:  has-patch php82 has-unit-tests  |     Focuses:
--------------------------------------------+---------------------

Comment (by SergeyBiryukov):

 In [changeset:"54087" 54087]:
 {{{
 #!CommitTicketReference repository="" revision="54087"
 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:

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

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