[wp-trac] [WordPress Trac] #52534: PHP 8: wp_localize_script() throws a warning if third parameter is a string.

[WordPress Trac]

#52534: PHP 8: wp_localize_script() throws a warning if third parameter is a
string.
-------------------------------------------+------------------------------
 Reporter:  peterwilsoncc                  |       Owner:  (none)
     Type:  defect (bug)                   |      Status:  new
 Priority:  normal                         |   Milestone:  Awaiting Review
Component:  Script Loader                  |     Version:
 Severity:  normal                         |  Resolution:
 Keywords:  php8 has-patch has-unit-tests  |     Focuses:
-------------------------------------------+------------------------------

Comment (by jrf):

 @peterwilsoncc I had a quick look, but to be honest, that warning seems
 100% correct behaviour and warranted.

 Both the documentation of
 [https://developer.wordpress.org/reference/functions/wp_localize_script/
 `wp_localize_script()`] as well as the docs for
 [https://developer.wordpress.org/reference/classes/wp_scripts/localize/
 `WP_Scripts::localize()`] state explicitly that the third parameter is
 (should be) an array, either single or multi-dimensional.

 In other words, passing a `string` as the `$l10n` parameter is a
 programmer error and unsupported behaviour, even though WP is "tolerant"
 of this by doing an `(array) $l10n` in the `foreach()` in
 `WP_Scripts::localize()`.

 Warnings like this exist for good reason: to warn programmers about errors
 they make before those errors can make it into production. They should not
 be avoided.

 If anything, the tolerance for passing the third parameter as a string
 should be removed and replaced by a ''doing it wrong'' warning, but that
 would break BC...

 So, as the warning is kind of obscure, let's look at what the ''actual''
 problem is.
 The problem is in this code snippet - `src/wp-includes/class.wp-
 scripts.php` - line 487 - 493:
 {{{#!php
 <?php
 foreach ( (array) $l10n as $key => $value ) {
         if ( ! is_scalar( $value ) ) {
                 continue;
         }

         $l10n[ $key ] = html_entity_decode( (string) $value, ENT_QUOTES,
 'UTF-8' );
 }
 }}}

 What is happening is that `foreach()` operates on a copy of the array and
 that ''copy'' is the one which is cast to array here: `foreach ( (array)
 $l10n as $key => $value )`.
 But then on line 492, `$l10n[ $key ] = html_entity_decode(...)`, an
 assignment is made to the ''original'' parameter, which is still a string,
 which is the bit which causes the failure.

 The simple solution to this would be to just cast the `$l10n` parameter to
 array ahead of the foreach.
 {{{#!php
 <?php
 $$l10n = (array) $l10n;
 foreach ( $l10n as $key => $value ) {
         if ( ! is_scalar( $value ) ) {
                 continue;
         }

         $l10n[ $key ] = html_entity_decode( (string) $value, ENT_QUOTES,
 'UTF-8' );
 }
 }}}


 Adding any extra code to add support for passing the parameter in an
 unsupported variable type, should IMO be avoided.

 I mean, seriously, this is going down a road where WP would accept any
 parameter type instead of the documented one and will try to make do with
 it by casting and juggling and doing all sorts of magic tricks, which is a
 never-ending road to incredible torture and pain, quite apart from the
 runtime impact all this type of "overhead" code would have...

 P.S.: Very happy to see the tests, though I'd love it if the `string` test
 could be changed to a hard failure... 😇

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