[wp-trac] [WordPress Trac] #53635: PHP 8.1: various compatibility fixes

[WordPress Trac]

#53635: PHP 8.1: various compatibility fixes
--------------------------------------------+------------------------------
 Reporter:  SergeyBiryukov                  |       Owner:  hellofromTonya
     Type:  task (blessed)                  |      Status:  accepted
 Priority:  normal                          |   Milestone:  5.9
Component:  General                         |     Version:
 Severity:  normal                          |  Resolution:
 Keywords:  php81 has-patch has-unit-tests  |     Focuses:  coding-
                                            |  standards
--------------------------------------------+------------------------------
Changes (by hellofromTonya):

 * keywords:  php81 has-patch has-unit-tests commit => php81 has-patch has-
     unit-tests


Comment:

 In [changeset:"51831" 51831]:
 {{{
 #!CommitTicketReference repository="" revision="51831"
 Build/Test Tools: Fix null handling and string type casting in
 `WP_UnitTestCase_Base::assertSameIgnoreEOL()`.

 Basically, the whole `assertSameIgnoreEOL()` assertion was fundamentally
 flawed. The assertion contends that it checks that the expected and actual
 values are of the same type and value, but the reality was very different.

 * The function uses `map_deep()` to potentially handle all sorts of
 inputs.
 * `map_deep()` handles arrays and objects with special casing, but will
 call the callback on everything else without further distinction.
 * The callback used passes the expected/actual value on to the
 `str_replace()` function to remove potential new line differences.
 * And the `str_replace()` function will - with a non-array input for the
 `$subject` - always return a string.
 * The output of these calls to `map_deep()` will therefore have
 "normalized" _all properties_ in objects, _all values_ in arrays and _all
 non-object, non-array values_ to strings.
 * And a call to `assertSame()` will therefore NEVER do a proper type check
 as the type of all input has already, unintentionally, been "normalized"
 to string.

 Aside from this clear flaw in the design of the assertion, PHP 8.1 now
 exposes a further issue as a `null` value for an object property, an array
 value or a plain value, will now yield a ` str_replace(): Passing null to
 parameter #3 ($subject) of type array|string is deprecated` notice.

 To fix both these issues, the fix in this PR ensures that the call to
 `str_replace()` will now only be made if the input is a text string.
 All other values passed to the callback are left in their original type.

 This ensures that a proper value AND type comparison can be done as well
 as prevents the PHP 8.1 deprecation notices.

 Ref:
 * https://developer.wordpress.org/reference/functions/map_deep/
 * https://www.php.net/manual/en/function.str-replace.php

 This commit:
 - Fixes type-casting of non-string values to `string` (the flawed part of
 this assertion) by invoking `str_replace()` when the value is of string
 type.
 - Fixes the PHP 8.1 `str_replace(): Passing null to parameter #3
 ($subject) of type array|string is deprecated` deprecation notice.
 - Micro-optimization: skips `map_deep()` when actual and/or expected are
 `null` (no need to process).
 - Adjusts the method documentation for both this method and the
 `assertEqualsIgnoreEOL()` alias method to document that the `$expected`
 and `$actual` parameters can be of any type.

 Follow-up to [48937], [51135], [51478].

 Props jrf, hellofromTonya.
 See #53363, #53635.
 }}}

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