PHP 8.0: various compatibility fixes

[jrf]

Once #50902 has been merged, a start can be made to apply fixes to WP core to fix the test failures on PHP 8.0.

Rather than opening a new ticket for every single patch, I'm proposing to use this ticket as an "epic", allowing a variety of small patches each fixing a specific failure to be added to and committed against this ticket.

For patches addressing all instances of failures related to one specific PHP 8.0 change across the codebase, separate tickets should still be opened.

For an example of issues/patches with separate tickets, see:

• #50343 PHP 8: Fix deprecation notices for optional function parameters declared before required parameter
• #50833 PHP 8: GD resources are GdImage object instances
• #50897 PHP 8: fix final private methods

When opening a separate ticket, please tag it with the php8 keyword, so these can be easily found using this report: https://core.trac.wordpress.org/query?keywords=~php8

[jrf]

Fix one fatal "argument must be passed by reference, value given" error. This single patch fixes 96 errors + 62 failures in the unit test run on PHP 8.0.

The WP native get_comment() function expects the first argument $comment to be passed by reference.

The PHP array_map() function, however, passes by value, not by reference, resulting in a fatal arguments must be passed by reference, value given error.

The PHP native array_walk() function _does_ pass by reference. Using this prevents the fatal error on PHP 8 and maintains the existing behaviour on PHP < 8.

This patch fixes 96 errors + 62 failures + 3 warnings of the test failures on PHP 8.

Refs:

• ​https://developer.wordpress.org/reference/functions/get_comment/
• ​https://www.php.net/manual/en/function.array-map.php
• ​https://www.php.net/manual/en/function.array-walk.php

Trac ticket: https://core.trac.wordpress.org/ticket/50913

As per the documentation of call_user_func_array(), the $param_arr should be a (numerically) indexed array, not a string-keyed array.

As we can use the spread operator in PHP 5.6+, there isn't really any need to use call_user_func_array() anyhow, we can call the array_merge() function directly.

The caveat to this is that the spread operator only works on numerically indexed arrays, so we need to wrap the $sidebars_widgets variable in a call to array_values() when using the spread operator.

Using array_values() in the existing call_user_func_array() call would also have solved this, but the solution now proposed, has the added benefit of getting rid of the overhead of call_user_func_array().

Refs:

• ​https://www.php.net/manual/en/function.call-user-func-array
• ​https://www.php.net/manual/en/function.array-merge
• ​https://www.php.net/manual/en/function.array-values.php

Trac ticket: https://core.trac.wordpress.org/ticket/50913

[jrf]

Fix one fatal "ArgumentCountError: array_merge() does not accept unknown named parameters" error. This patch fixes 5 errors in the unit test run on PHP 8.0.

[SergeyBiryukov]

Just noting that when testing 50913-1.patch​ with PHP 8.0 Beta 2 on my install, the "argument must be passed by reference, value given" message is a warning, rather than a fatal error. I guess it might have changed between Beta 1 and Beta 2. The patch does fix the issue though.

[SergeyBiryukov]

In 48838:

Code Modernization: Fix PHP 8 "argument must be passed by reference, value given" error in WP_Comment_Query::get_comments().

The WP native get_comment() function expects the first argument $comment to be passed by reference.

The PHP array_map() function, however, passes by value, not by reference, resulting in an "arguments must be passed by reference, value given" error.

The PHP native array_walk() function does pass by reference. Using this prevents the error on PHP 8 and maintains the existing behaviour on PHP < 8.

Props jrf.
See #50913.

[SergeyBiryukov]

In 48839:

Code Modernization: Fix PHP 8 "ArgumentCountError: array_merge() does not accept unknown named parameters" fatal error in retrieve_widgets().

As per the documentation of call_user_func_array(), the $param_arr should be a (numerically) indexed array, not a string-keyed array.

As we can use the spread operator in PHP 5.6+, there isn't really any need to use call_user_func_array() anyhow, we can call the array_merge() function directly.

The caveat to this is that the spread operator only works on numerically indexed arrays, so we need to wrap the $sidebars_widgets variable in a call to array_values() when using the spread operator.

Using array_values() in the existing call_user_func_array() call would also have solved this, but the solution now proposed, has the added benefit of getting rid of the overhead of call_user_func_array().

Props jrf.
See #50913.

[SergeyBiryukov]

In 48960:

Code Modernization: Correct the check for negative post IDs in WP_Query::parse_query() to work as expected on PHP 8.

PHP 8 changes the way string to number comparisons are performed: ​https://wiki.php.net/rfc/string_to_number_comparison

In particular, checking if an empty string is less than zero in PHP 8 evaluates to true, not false.

For WP_Query, this resulted in unintentionally returning a 404 error for most of front-end requests, instead of the relevant content.

By explicitly casting the value to int, we make sure to compare both values as numbers, rather than a string and a number.

Follow-up to [38288].

Props trepmal.
See #50913.

[SergeyBiryukov]

[48960] brings the number of unit test failures down from 331 in ​build 11258.12 to 23 in ​build 11268.12.

Still a lot of work ahead, though :)

[SergeyBiryukov]

In 48961:

Code Modernization: Remove unnecessary reference sign from get_comment() definition.

This fixes a PHP 8 "argument must be passed by reference, value given" error when using array_map( 'get_comment', ... ).

Object variables in PHP 5+ contain a reference to the object, and it's the reference that's passed around.

Note: This reverts [48838], which is now redundant.

Follow-up to a similar change for get_post() in [21572].

See #50913.

[jrf]

@SergeyBiryukov [48961] should be considered a BC-break and will need a dev-note. Chances are there will be a plugin/theme out there which is relying on the reference for non-object passed parameters. While that will realistically only come into play if the variable they passed to get_comment() would be regarded as "empty", the fact that the variable will now no longer automatically be filled is a BC-break.

[SergeyBiryukov]

Replying to jrf:

[48961] should be considered a BC-break and will need a dev-note.

Thanks, adding the tag. Just noted a few other instances of array_map( 'get_comment', ... ) in core and the tests, and thought this could be a more common pattern than relying on the reference for filling an empty variable, so bringing some consistency with get_post() would make sense.

A ​brief search shows that some popular plugins do indeed use array_map( 'get_comment', ... ).

[andraganescu]

While testing on PHP 8 I found that any page (home or archive) will return 404 as if there were no posts available. The problem appears to be in wp-includes/class-wp-query.php at line 762, where the new ​Saner string to number comparisons makes it so that "" < 0 === true, so $qv['p'] < 0 is true since the default for $qv['p'] is string(0).

Changing the condition to intval($qv['p']) < 0 fixes the wrong 404s.

Question: should I open a PR against this ticket for such findings as well or is this ticket for more holistic fixes?

[azaozz]

Replying to andraganescu:

...the new ​Saner string to number comparisons makes it so that "" < 0 === true

Ugh, thinking that's going to be another big "back-compat-fail" thing in PHP 8.0 :(

Question: should I open a PR against this ticket for such findings as well or is this ticket for more holistic fixes?

Thinking a PR would be good, at least for "documentation purposes". Then gather all the "findings" and publish them on make/core. I'm sure the PHP team will publish a "Migration guide" at some point but the sooner plugins authors know what to look for when checking their plugins, the better :)

[jrf]

Replying to andraganescu:

While testing on PHP 8 I found that any page (home or archive) will return 404 as if there were no posts available. The problem appears to be in wp-includes/class-wp-query.php at line 762, where the new ​Saner string to number comparisons makes it so that "" < 0 === true, so $qv['p'] < 0 is true since the default for $qv['p'] is string(0).

This was already fixed in [48960] (https://core.trac.wordpress.org/ticket/50913#comment:10)

[andraganescu]

Wow, to think I did go through the comments :D

[jeherve]

Replying to SergeyBiryukov:

In 48961:

Code Modernization: Remove unnecessary reference sign from get_comment() definition.

This fixes a PHP 8 "argument must be passed by reference, value given" error when using array_map( 'get_comment', ... ).

Object variables in PHP 5+ contain a reference to the object, and it's the reference that's passed around.

Note: This reverts [48838], which is now redundant.

Follow-up to a similar change for get_post() in [21572].

See #50913.

After this change, and considering @jrf's comment, should we consider updating map_meta_cap to take this into account, when no arguments are passed and get_comment( $args[0] ) now produces a notice?
-- https://core.trac.wordpress.org/browser/trunk/src/wp-includes/capabilities.php#L389

[SergeyBiryukov]

Replying to jeherve:

After this change, and considering @jrf's comment, should we consider updating map_meta_cap to take this into account, when no arguments are passed and get_comment( $args[0] ) now produces a notice?
-- https://core.trac.wordpress.org/browser/trunk/src/wp-includes/capabilities.php#L389

Thanks for bringing this up.

If the notice comes from current_user_can( 'edit_comment' ), that doesn't seem any different from edit_post and similar checks:

• current_user_can( 'delete_post' )
• current_user_can( 'edit_post' )
• current_user_can( 'read_post' )
• current_user_can( 'publish_post' )

These all require a post ID to check, and will cause a PHP notice if the ID is not provided. Performing these checks without passing in a post ID (or a comment ID, in case of edit_comment) is a developer error, so I think the notice is legitimate, hiding it would only make debugging harder.

See also: #48415, specifically comment:2:ticket:48415.

[SergeyBiryukov]

Replying to SergeyBiryukov:

These all require a post ID to check, and will cause a PHP notice if the ID is not provided. Performing these checks without passing in a post ID (or a comment ID, in case of edit_comment) is a developer error, so I think the notice is legitimate, hiding it would only make debugging harder.

Found an existing open ticket for this: #44591, let's add a _doing_it_wrong() notice there.

[SergeyBiryukov]

In 48972:

Tests: Replace the native PHPUnit getMockForAbstractClass() and getMockBuilder() methods.

This avoids parse errors in PHPUnit internals due to match being a reserved keyword in PHP 8.

To run on PHP 8, the tests relying on these methods require PHPUnit 9.3 or later.

When the test suite is updated for compatibility with PHPUnit 9.x, these overrides can be removed.

See #50913, #50902.

[SergeyBiryukov]

In 48973:

Tests: Require PHP less than 8.0 for some wpdb tests.

These tests ensure that wpdb::prepare() throws a _doing_it_wrong() notice when called with an incorrect number of arguments, or with arguments of a wrong type.

PHP 8 introduces similar error messages natively, making these tests redundant on PHP 8.0 or later.

Follow-up to [41470], [41662].

See #50913.

[SergeyBiryukov]

Just noting that [48973] might need to be reverted to address backward compatibility for wpdb::prepare() instead, see comment:7:ticket:50639 for more details.

[SergeyBiryukov]

Looking at the remaining failures, there is one particular issue in the import tests that's outside of WordPress core:

Function libxml_disable_entity_loader() is deprecated

It needs something like [48789] applied to the WordPress Importer plugin.

Looks like that will need a PR to ​WordPress Importer on GitHub and then a sync to the ​SVN repo.

If someone is able to help with that, please do :)

[jrf]

Looks like that will need a PR to WordPress Importer on GitHub and then a sync to the SVN repo.

If someone is able to help with that, please do :)

@SergeyBiryukov I already opened a PR for the WordPress Importer issue over a month ago: ​https://github.com/WordPress/wordpress-importer/pull/78

[SergeyBiryukov]

In 48979:

Tests: Revert [48973].

These tests ensure that a _doing_it_wrong() notice is thrown when wpdb::prepare() is called incorrectly, but also that the function will still handle the provided input as correctly as possible.

Disabling these tests on PHP 8 hides a problem, i.e. the function will no longer throw a notice and handle things correctly, it will now cause a white screen of death due to a fatal error.

That is a backward compatibility break, and wpdb::prepare() should be updated instead to maintain the original behaviour on PHP 8.

Props jrf, ayeshrajans.
See #50913, #50639.

[SergeyBiryukov]

In 48980:

Code Modernization: Return an empty string from wpdb::_real_escape() if a non-scalar value is passed.

This avoids a fatal error on PHP 8 caused by passing a non-string value to mysqli_real_escape_string(), and maintains the current behaviour.

See #50913, #50639.

[SergeyBiryukov]

In 48981:

Code Modernization: Return an empty string from wpdb::prepare() if there are not enough arguments to match the placeholders.

This avoids a fatal error on PHP 8 caused by passing mismatched arguments to vsprintf(), and maintains the current behaviour.

Follow-up to [48979], [48980].

See #50913, #50639.

[SergeyBiryukov]

In 48993:

Tests: Fix the failure in test_get_weekday_undefined_index() on PHP 8.

The test ensures that WP_Locale::get_weekday() throws an "undefined offset" notice when called with an incorrect $weekday_number parameter.

In PHP 8, that notice is now a warning, so the test needs to be adjusted accordingly.

See #50913.

[SergeyBiryukov]

Replying to SergeyBiryukov:

In 48960:

Code Modernization: Correct the check for negative post IDs in WP_Query::parse_query() to work as expected on PHP 8.

PHP 8 changes the way string to number comparisons are performed: ​https://wiki.php.net/rfc/string_to_number_comparison

In particular, checking if an empty string is less than zero in PHP 8 evaluates to true, not false.

Just noting that I checked other instances of ... < 0 in core to see if any of them involve an empty string, but it doesn't look like they are affected. Most of them already explicitly cast the left part to (int), and the rest are unlikely to contain a string either. If someone would like to double-check that, please do :)

[SergeyBiryukov]

In 49007:

Tests: Fix the failures in REST API format keyword validation tests on PHP 8.

The tests ensure that rest_sanitize_value_from_schema() and rest_validate_value_from_schema() throw an "undefined offset" notice when the required type schema keyword is not passed.

In PHP 8, that notice is now a warning, so the tests need to be adjusted accordingly.

Follow-up to [48300], [48993].

See #50913.

[SergeyBiryukov]

In 49008:

Coding Standards: Fix WPCS issue in [49007].

See #50913.

[SergeyBiryukov]

In 49015:

Tests: Use more specific assertions in Tests_Image_Functions::test_load_directory().

This avoids an error on PHP 8 caused by calling get_resource_type() on a string.

See #50913.

[SergeyBiryukov]

In 49019:

Media: Return a WP_Error from WP_Image_Editor_GD::load() if file contents could not be retrieved.

This avoids an error on PHP 8 caused by calling imagecreatefromstring() on an empty result.

See #50913.

[SergeyBiryukov]

In 49026:

Privacy: Check if the accumulated data in wp_privacy_process_personal_data_export_page() is not empty.

This avoids an error on PHP 8 caused by passing an empty string to array_merge(), instead of an array.

See #50913.

[SergeyBiryukov]

In 49032:

Tests: Check if image metadata for a particular size was successfully retrieved in some media tests.

This outputs a proper message in case of failure, instead of an obscure PHP error further in the test.

See #50913.

[SergeyBiryukov]

In 49037:

Tests: Backport two changes from PHPUnit 9.3:

• Replace the Match interface with ParametersMatch, to avoid parse errors due to match being a reserved keyword in PHP 8.
• Replace ReflectionParameter::getClass() usage, which is deprecated in PHP 8.

This allows tests relying on the getMockForAbstractClass() and getMockBuilder() methods to run again on PHP 8.

When the test suite is updated for compatibility with PHPUnit 9.x, these overrides can be removed.

Follow-up to [48972].

See #50913, #50902.

[SergeyBiryukov]

In 49039:

Build/Test Tools: Use trunk revision 2387243 of the WordPress Importer plugin.

This revision includes a change to only call libxml_disable_entity_loader() in PHP < 8, in order for unit tests in the import group to pass on PHP 8.

This function has been deprecated in PHP 8.0 because in libxml 2.9.0, external entity loading is disabled by default, so this function is no longer needed to protect against XXE attacks.

Follow-up to [46542], [48789].

Props jrf.
See #50913.

[xknown]

I'm also going through some issues reported by PHPStan (see ​https://github.com/WordPress/wordpress-develop/pull/553), mostly in places where we are passing the incorrect type to WP core or PHP's built-in functions.

@SergeyBiryukov what would be the best way to handle this given the amount of changes that are required? I'm doing one commit per affected file which will hopefully make things easier to include it in core.

[SergeyBiryukov]

Replying to xknown:

I'm also going through some issues reported by PHPStan (see ​https://github.com/WordPress/wordpress-develop/pull/553), mostly in places where we are passing the incorrect type to WP core or PHP's built-in functions.

That looks great, thanks!

what would be the best way to handle this given the amount of changes that are required? I'm doing one commit per affected file which will hopefully make things easier to include it in core.

That works for me :)

[SergeyBiryukov]

In 49043:

Code Modernization: Correct the check for parent argument in wp_insert_term() and wp_update_term().

PHP 8 changes the way string to number comparisons are performed: ​https://wiki.php.net/rfc/string_to_number_comparison

In particular, checking if a non-empty, non-numeric string is greater than zero in PHP 8 evaluates to true, not false.

For wp_insert_term(), this resulted in a "Parent term does not exist" error for a non-numeric string, instead of discarding the value.

By explicitly casting the value to int, we make sure to compare both values as numbers, rather than a string and a number.

Follow-up to [29196], [29830], [29867].

See #50913.

[SergeyBiryukov]

In 49044:

Tests: Check if image sizes were successfully retrieved in some REST API attachments controller tests.

This outputs a proper message in case of failure, instead of an obscure PHP error further in the test.

Props TimothyBlynJacobs.
See #50913, #51393.

[SergeyBiryukov]

In 49046:

Tests: Correct the check for image sizes in some REST API attachments controller tests.

If the sizes data could not be retrieved, the controller returns an empty object instead of an array.

This makes sure that the value is in fact an array before proceeding, and outputs a proper message in case of failure, instead of an obscure PHP error further in the test.

Follow-up to [49044].

See #50913, #51393.

[SergeyBiryukov]

Looking at this failure:

1) Tests_Functions::test_wp_check_filetype_and_ext with data set #13 ('/var/www/tests/phpunit/includ...st.csv', 'test.csv', array('csv', 'text/csv', false))
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 (
-    'ext' => 'csv'
-    'type' => 'text/csv'
+    'ext' => false
+    'type' => false
     'proper_filename' => false
 )
/var/www/tests/phpunit/tests/functions.php:1210

It's caused by a change in PHP 8: previously the Fileinfo extension returned the text/plain MIME type for CSV files, and now it returns application/csv. wp_check_filetype_and_ext() does not take that new value into account.

Relevant commits and issues upstream:

• ​fileinfo: Upgrade to libmagic 5.39
• ​PR/174: petre: The new CSV file identification is returning "application/csv"
• ​0000174: CSV files identified as "application/csv" instead of registered "text/csv"

Per the comments in the issue 174 above, the incorrect type is now fixed and text/csv should be returned instead. However, it's not quite clear when that fix makes it into the PHP project.

Since it's a safe change that should not affect anything else, I think wp_check_filetype_and_ext() should be updated to also check for application/csv, same as it currently does for application/rtf.

This can be reverted later if the correct text/csv type makes it into the PHP 8 final release.

[SergeyBiryukov]

In 49049:

Upload: Add a check in wp_check_filetype_and_ext() to account for CSV files having the application/csv MIME type.

Previously, the PHP Fileinfo extension used to detect CSV files as text/plain.

In PHP 8, this has changed, and CSV files are detected as application/csv.

Follow-up to [44438].

See #50913.

[SergeyBiryukov]

In 49051:

Tests: Correct assertion in WP_Test_REST_Comments_Controller::check_comment_data().

author_avatar_urls should be present in the comment data array keys, not values.

The test only passed accidentally due to assertContains() not performing a strict type check.

See #38266, #50913.

[SergeyBiryukov]

In 49068:

Code Modernization: Remove a single trailing percent sign before calling sprintf() on the $default parameter in get_theme_mod().

This avoids a "Missing format specifier at end of string" fatal error on PHP 8, and maintains the current behaviour.

See #50913.

[SergeyBiryukov]

In 49072:

Code Modernization: Return false from wpdb::query() if the query was filtered to an empty string using the query filter.

This avoids a fatal error on PHP 8 caused by passing an empty string to mysqli_query(), and maintains the current behaviour.

Follow-up to [48980], [48981].

See #50913, #50639.

[SergeyBiryukov]

In 49073:

Code Modernization: Check if the file to retrieve metadata from in get_file_data() was successfully opened.

This avoids a fatal error on PHP 8 caused by passing a false value to fread(), instead of a file resource.

See #50913.

[SergeyBiryukov]

In 49074:

Build/Test Tools: Comment out the xdebug group test run for PHP 8 for now.

Xdebug supports PHP 8 only from version 3.0, which is not released yet.

Once Xdebug 3.0 is released and included in the Docker image, this should be uncommented again.

Follow-up to [48957], [49037].

See #50913, #50902.

[SergeyBiryukov]

In 49076:

Code Modernization: Ignore the _multiwidget property when collecting widget numbers in WP_Customize_Manager::import_theme_starter_content().

PHP 8 changes the way string to number comparisons are performed: ​https://wiki.php.net/rfc/string_to_number_comparison

In particular, when calling max() on an array with numeric values and a non-numeric string, in PHP 8 the string is returned instead of a number.

For ::import_theme_starter_content(), this resulted in retrieving the _multiwidget property instead of the maximum widget number for a particular type.

By explicitly ignoring the _multiwidget property, we make sure to retrieve the correct widget number value.

Follow-up to [48960], [49043].

See #50913.

[SergeyBiryukov]

In 49077:

Build/Test Tools: Remove PHP 8 from allowed failures.

With all known unit test issues now addressed, WordPress 5.6 aims to support PHP 8 as much as possible.

See #50913, #50902.

[SergeyBiryukov]

Replying to SergeyBiryukov:

Replying to xknown:

I'm also going through some issues reported by PHPStan (see ​https://github.com/WordPress/wordpress-develop/pull/553), mostly in places where we are passing the incorrect type to WP core or PHP's built-in functions.

That looks great, thanks!

Created a separate ticket for that: #51423, as this one is getting quite large.

[xknown]

Replying to SergeyBiryukov:

Created a separate ticket for that: #51423, as this one is getting quite large.

Sounds good, thanks!

[jrf]

With all known unit test issues now addressed, WordPress 5.6 aims to support PHP 8 as much as possible.

Just for the record - and I know @SergeyBiryukov is fully aware of this:

The current test suite covers less than 40% of the WordPress Core code, so that still means that over 60% of the WordPress Core code is currently untested again PHP 8.0 and the PHP 8 compatibility status of the code is unknown.

[jrf]

I've just gone through some of the more recent changes to PHP 8.0 and noticed this commit which we may need to account for: ​https://github.com/php/php-src/commit/5cb8b04646ed99c794c45f8c24fc5c3c7c59e320

TL;DR: Looks like support for MySQL < 5.5 is being dropped in PHP 8.0 when not using mysqlnd.

This doesn't necessarily mean that WP needs to drop support for those older MySQl versions, but it may warrant a safeguard in the installation routine.
I.e. a check that:

=> If on PHP 8
  => and the available MySQL version < 5.5.
    => make sure the `mysqlnd` driver is available.
      => Otherwise refuse to install

There may be more places where a check would be warranted. Input welcome.

Closing as committed.

Closing as fixed.

[SergeyBiryukov]

Replying to jrf:

The current test suite covers less than 40% of the WordPress Core code, so that still means that over 60% of the WordPress Core code is currently untested again PHP 8.0 and the PHP 8 compatibility status of the code is unknown.

Right. As shared during the ​recent weekly dev chat, I think the next steps for PHP 8 support would be:

• Updating the Docker image to use the same PHP extensions for PHP 8 that we have on PHP 7.x: gd, mbstring, etc. There is a PR by @desrosj waiting for review: ​https://github.com/WordPress/wpdev-docker-images/pull/36. If anyone is able to help with that, please do :)
• Fixing some function argument type issues reported by PHPStan (a static analyzer): #51423.
• More testing on PHP 8, expanding test coverage, and creating tickets for any issues found. @andraganescu and @desrosj are working on a call for testing to be published later.

[desrosj]

Wanted to provide a few updates here.

The ​PHP 8 wpdev Docker image has been updated to be more inline with the ones for other versions of PHP. There are a few exceptions:

• xDebug is not installed. Support for PHP 8.0 is not yet added, though it is planned for version 3.0.
• The Imagick extension is not installed. The ​main branch of Imagick does support PHP 8. However, an official version has not been released containing those changes. ​I inquired about when to expect that tagged version.

I also switched the container being used from devilbox (which was installing PHP 8.0.0-dev) to the latest one from the ​official PHP account (8.0 RC1). Since PHP 8 is now in the beta/RC stages, they will be creating containers with each release.

I'll make sure to update the PHP 8 image as each new RC is released. Before starting the Docker container to test on PHP 8, make sure to run npm run env:pull to ensure you have the most up to date version of the container installed.

The call for testing has also been published: ​https://make.wordpress.org/core/2020/10/06/call-for-testing-php-8-0/.

[SergeyBiryukov]

Replying to desrosj:

xDebug is not installed. Support for PHP 8.0 is not yet added, though it is planned for version 3.0.

Just to follow up on this, Xdebug 3.0.0beta1 with PHP 8 support has been released:
​https://xdebug.org/announcements/2020-10-14

[konamiman]

Hi, a few days ago I posted a comment in the ticket for unit tests regarding a custom version of PHPUnit 7 I've prepared for WooCommerce, linking here for visibility: https://core.trac.wordpress.org/ticket/50902#comment:20

[jorbin]

In 49161:

Bootstrap/Load: Don't assume WP_CONTENT_DIR is defined

When the mysql extention isn't loaded and a custom db dropin is not in place, we give folks a nice error. However, we can't assume that the WP_CONTENT_DIR constant is set yet since this runs before we define default constants.

This fixes a PHP8 error.

See: #50913.

[SergeyBiryukov]

In 49163:

Bootstrap/Load: Don't assume WP_CONTENT_DIR is defined.

When the mysql extention isn't loaded and a custom db dropin is not in place, we give folks a nice error. However, we can't assume that the WP_CONTENT_DIR constant is set yet since this runs before we define default constants.

This fixes a PHP 8 error.

Props jorbin.
Merges [49161] to trunk.
See #50913.

[SergeyBiryukov]

In 49164:

Bootstrap/Load: Revert [49161] from the 5.5 branch.

See #50913.

WordPress should not throw any notices.

array_map() passes by value, while array_walk() passes by reference.

The use of array_map() on line 52 within the export_entries() method will cause a "Warning: PO::export_entry(): Argument #1 ($entry) must be passed by reference, value given" warning on PHP 8.0 because the export_entry() method is declared to expect the $entry object to be passed by reference.

Since PHP 5.0, objects are passed by reference by default and this does not have to be declared in the function signature of a method expecting this anymore.

Also, the code within the method doesn't even try to change $entry and return a value.

So, all in all, the reference in the function declaration is redundant, old-school and should be removed.

While this could be considered a BC-break, I have done a search for use of this method in plugins and reviewed a significant number of the returned results. None of these would run into trouble with the change now made. Rather this change fixes the same issue for a number of plugins also using array_map().

All the same, this change should be mentioned in a dev-note as it is a very small and insignificant BC-break.

Search results: ​https://wpdirectory.net/search/01EMWBEKAM2B6ECSTCTEDAZGQW

Trac ticket: https://core.trac.wordpress.org/ticket/50913

[jrf]

Fixes another "argument # must be passed by reference" issue - see the PR for more information and a passing build + see ​https://3v4l.org/KbpLJ for proof of issue

[SergeyBiryukov]

In 49186:

Code Modernization: Remove unnecessary reference sign from PO::export_entry() definition.

This fixes a PHP 8 "argument must be passed by reference, value given" error when using array_map() in PO::export_entries().

Object variables in PHP 5+ contain a reference to the object, and it's the reference that's passed around.

Props jrf.
See #50913.

Closing as committed.

[SergeyBiryukov]

Let's call this one fixed for 5.6. Please create new tickets for any follow-up issues.

Thanks everyone!

[jrf]

:+1: Thanks @SergeyBiryukov for all your work on this!

[SergeyBiryukov]

#51949 was marked as a duplicate.

[daisyo]

Dev note published: ​https://make.wordpress.org/core/2020/11/23/wordpress-and-php-8-0/

[SergeyBiryukov]

#51957 was marked as a duplicate.