[wp-trac] [WordPress Trac] #47698: Remove redundant polyfills for PHP native functionality (excl JSON)

[WordPress Trac]

#47698: Remove redundant polyfills for PHP native functionality (excl JSON)
-------------------------------------------------+-------------------------
 Reporter:  jrf                                  |       Owner:  (none)
     Type:  enhancement                          |      Status:  new
 Priority:  normal                               |   Milestone:  5.3
Component:  General                              |     Version:  trunk
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch 2nd-opinion has-unit-      |     Focuses:  coding-
  tests                                          |  standards
-------------------------------------------------+-------------------------
Description changed by jrf:

Old description:

> Now the new minimum PHP requirement for WordPress is PHP 5.6.20, a number
> of the polyfills WordPress provides for PHP functionality can be removed.
>
> I will be attaching the relevant patches for this to this ticket.
>
> Let's talk through them:
>
> == **`array_merge_recursive()`**
>
> The [https://www.php.net/manual/en/function.array-replace-recursive.php
> `array_merge_recursive()`] function was introduced in PHP 5.3.0.
>
> The function has had no significant changes since then which would need
> to be accounted for.
>
> The `Array` extension is part of
> [https://www.php.net/manual/en/array.installation.php PHP Core and cannot
> be disabled].
>
> **Conclusion**: this polyfill can now be safely removed.
>

> == **SPL autoload**
> WP polyfills the following PHP native SPL extension functionality:
> * The function [https://www.php.net/manual/en/function.spl-autoload-
> register.php `spl_autoload_register()`] (PHP 5.1.0+)
> * The function [https://www.php.net/manual/en/function.spl-autoload-
> unregister.php `spl_autoload_unregister(()`] (PHP 5.1.0+)
> * The function [https://www.php.net/manual/en/function.spl-autoload-
> functions.php `spl_autoload_functions()`] (PHP 5.1.0+)
>
> These function have had no significant changes since then which would
> need to be accounted for.
>
> [https://www.php.net/manual/en/spl.installation.php Prior to PHP 5.3.0,
> the SPL extension could be disabled] via a compilation flag - and
> sometimes was -. As of PHP 5.3.0, the extension can no longer be
> disabled.
>
> **Conclusion**: these polyfills can now be safely removed.
>
> === **Implementation**
> This polyfill uses a separate file `wp-includes/spl-autoload-compat.php`.
> While I suspect this to be exceedingly rare in practice, plugins/themes
> ''could'' load this file directly.
>
> So, there is a choice to make here:
> * Remove the file completely.
> * Remove the code and deprecate the file.
>
> In line with WP's general cautiousness in these cases, I have opted for
> the second choice and have removed the code from the file, replacing it
> with a call to `_deprecated_file()` to alert any plugins/themes which
> would still try to include the file directly.
>
> === **Other considerations**
> The removed code includes a WP specific global variable
> `$_wp_spl_autoloaders` which was conditionally declared when the polyfill
> was active.
> This global variable will no longer be available.
>
> While I don't expect any plugins/themes access this variable directly,
> removing this variable could be considered a BC-break.
> All the same, as the variable would only be available on PHP < 5.3 when
> the SPL extension was disabled, one can expect that any userland code
> which ''would'' access this variable directly, would be accompanied by a
> call to `isset()`, so removing this variable should - in practice - not
> cause any problems.
>

> == **Hash**
> WP polyfills the following PHP native Hash extension functionality:
> * The function [https://www.php.net/manual/en/function.hash-hmac.php
> `hash_hmac()`] (PHP 5.1.2+)
> * The function [https://www.php.net/manual/en/function.hash-equals.php
> `hash_equals()`] (PHP 5.6.0+)
>
> These functions have had no significant changes since then which would
> need to be accounted for.
>
> While one may think those can now be safely removed, this is not the
> case.
>
> While the Hash extension has been bundled and compiled with PHP core
> since PHP 5.1.2, until PHP 7.4, the Hash extension could still be
> [https://www.php.net/manual/en/hash.installation.php disabled at compile
> time using the `--disable-hash` flag].
>
> **Conclusion**: Based on the above, these two functions should
> ''**not**'' be removed at this time.
>
> === **Implementation**
> I have added some additional documentation to these functions, making it
> explicit when will be the right time to remove them.
>

> == **Unit tests**
>
> As these functions are used throughout core, removing them can be
> considered sufficiently unit tested when all the WP Core unit tests pass,
> and they still do: https://travis-ci.com/WordPress/wordpress-
> develop/builds/119063509
>

> == **Other**
> * Ticket https://core.trac.wordpress.org/ticket/46630 is a partial
> duplicate of this ticket.
> * The JSON functionality related polyfills will be addressed in a
> separate ticket, which I will link to once I've created it.
>
> /cc @pento

New description:

 Now the new minimum PHP requirement for WordPress is PHP 5.6.20, a number
 of the polyfills WordPress provides for PHP functionality can be removed.

 I will be attaching the relevant patches for this to this ticket.

 Let's talk through them:

 == **`array_replace_recursive()`**

 The [https://www.php.net/manual/en/function.array-replace-recursive.php
 `array_replace_recursive()`] function was introduced in PHP 5.3.0.

 The function has had no significant changes since then which would need to
 be accounted for.

 The `Array` extension is part of
 [https://www.php.net/manual/en/array.installation.php PHP Core and cannot
 be disabled].

 **Conclusion**: this polyfill can now be safely removed.


 == **SPL autoload**
 WP polyfills the following PHP native SPL extension functionality:
 * The function [https://www.php.net/manual/en/function.spl-autoload-
 register.php `spl_autoload_register()`] (PHP 5.1.0+)
 * The function [https://www.php.net/manual/en/function.spl-autoload-
 unregister.php `spl_autoload_unregister(()`] (PHP 5.1.0+)
 * The function [https://www.php.net/manual/en/function.spl-autoload-
 functions.php `spl_autoload_functions()`] (PHP 5.1.0+)

 These function have had no significant changes since then which would need
 to be accounted for.

 [https://www.php.net/manual/en/spl.installation.php Prior to PHP 5.3.0,
 the SPL extension could be disabled] via a compilation flag - and
 sometimes was -. As of PHP 5.3.0, the extension can no longer be disabled.

 **Conclusion**: these polyfills can now be safely removed.

 === **Implementation**
 This polyfill uses a separate file `wp-includes/spl-autoload-compat.php`.
 While I suspect this to be exceedingly rare in practice, plugins/themes
 ''could'' load this file directly.

 So, there is a choice to make here:
 * Remove the file completely.
 * Remove the code and deprecate the file.

 In line with WP's general cautiousness in these cases, I have opted for
 the second choice and have removed the code from the file, replacing it
 with a call to `_deprecated_file()` to alert any plugins/themes which
 would still try to include the file directly.

 === **Other considerations**
 The removed code includes a WP specific global variable
 `$_wp_spl_autoloaders` which was conditionally declared when the polyfill
 was active.
 This global variable will no longer be available.

 While I don't expect any plugins/themes access this variable directly,
 removing this variable could be considered a BC-break.
 All the same, as the variable would only be available on PHP < 5.3 when
 the SPL extension was disabled, one can expect that any userland code
 which ''would'' access this variable directly, would be accompanied by a
 call to `isset()`, so removing this variable should - in practice - not
 cause any problems.


 == **Hash**
 WP polyfills the following PHP native Hash extension functionality:
 * The function [https://www.php.net/manual/en/function.hash-hmac.php
 `hash_hmac()`] (PHP 5.1.2+)
 * The function [https://www.php.net/manual/en/function.hash-equals.php
 `hash_equals()`] (PHP 5.6.0+)

 These functions have had no significant changes since then which would
 need to be accounted for.

 While one may think those can now be safely removed, this is not the case.

 While the Hash extension has been bundled and compiled with PHP core since
 PHP 5.1.2, until PHP 7.4, the Hash extension could still be
 [https://www.php.net/manual/en/hash.installation.php disabled at compile
 time using the `--disable-hash` flag].

 **Conclusion**: Based on the above, these two functions should ''**not**''
 be removed at this time.

 === **Implementation**
 I have added some additional documentation to these functions, making it
 explicit when will be the right time to remove them.


 == **Unit tests**

 As these functions are used throughout core, removing them can be
 considered sufficiently unit tested when all the WP Core unit tests pass,
 and they still do: https://travis-ci.com/WordPress/wordpress-
 develop/builds/119063509


 == **Other**
 * Ticket https://core.trac.wordpress.org/ticket/46630 is a partial
 duplicate of this ticket.
 * The JSON functionality related polyfills will be addressed in a separate
 ticket, which I will link to once I've created it.

 /cc @pento

--

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