[PHP 8] Fix deprecation notices for optional function parameters declared before required parameter.

[ayeshrajans]

PHP 8 deprecated required declaring functions/methods that have optional parameters before required ones.

See ​https://php.watch/versions/8.0/deprecate-required-param-after-optional

function foo($param_optional = null, $param_required) {
//           ^^ optional parameter , ^^ required parameter
}

• Tests: ​https://travis-ci.com/github/Ayesh/wordpress-develop
• PHP 8 build (without the particular deprecation notice): ​https://travis-ci.com/github/Ayesh/wordpress-develop/jobs/346120156

Note that the PHP 8 tests still fail, but that is the bigger picture.
Snippet above will raise a deprecation notice in PHP 8:

Deprecated: Required parameter $param_required follows optional parameter $param_optional in ... on line ...

WordPress currently has 6 of such deprecation notices raised in PHP 8 CI tests: ​https://travis-ci.com/github/WordPress/wordpress-develop/jobs/345635046

I have gone through these 6 declarations, and updated their default values to stop this notice from being raised. The default values are determined by the caller/callee default values and return types.

[ayeshrajans]

This patch fixes the 6 deprecation notices related to function parameter order (​https://php.watch/versions/8.0/deprecate-required-param-after-optional)

[desrosj]

Thanks for getting the ball rolling on this one, @ayeshrajans! This seems reasonable, but I am going to mark as Future Release.

Historically, changes are not made to WordPress to support new versions of PHP until those versions enter the feature freeze/beta part of the release cycle. The idea is that this prevents additional work if the upstream implementation changes, or if features happen to be removed entirely. It also allows all of the compatibility changes to be bundled (as much as possible) into a single release, which makes communication through dev notes and documentation much easier.

I've updated the ​PHP Compatibility and WordPress Versions page of the handbook to reflect this.

If there are more tickets like this, please do go ahead and make them. But just know the priority for these will be for WP 5.6 later this year.

[ayeshrajans]

Thanks for the quick response @desrosj - I understand this series wouldn't be for WordPress 5.5 this August, which by that time only a PHP 8 alpha would be out.

5.6 certainly seems like the ideal target because PHP 8 release and 5.6 release would align more or less.

[jorbin]

Adding a custom keyword to make it easier once work on supporting php8 begins

[desrosj]

Since there is now a 5.6 milestone, moving this there as that's the current target for exploring this.

As it already wasn't possible to pass the required parameters without also passing the optional one anyway, I'm removing the default value for the (not so) optional parameters and documenting the expected value to "skip" the parameter in the docblock instead.

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

[jrf]

I've reviewed the patch and for the most part, I would like to suggest an alternative approach.

The original approach by @ayeshrajans was to make any parameters after the optional parameter, also optional.

Save for one specific case (rest api test), I propose doing the opposite: making the optional parameter declared before required parameters, required.

As it already wasn't possible to pass the required parameters without also passing the optional one anyway, removing the default value for the (not so) optional parameters should not be regarded as a BC break.
I've also updated the docblock @param tags with the expected value to "skip" the parameter.

Other notes regarding this PHP 8 change:

Both SimplePie as well as GetID3 also have (had) instances of this issue.
I have opened PRs in both projects to fix this.

The PR in GetID3 has been merged already and is expected to be included in the next release.

• ​https://github.com/JamesHeinrich/getID3/pull/259
• ​https://github.com/simplepie/simplepie/pull/656

[ayeshrajans]

Thanks for picking up this one, @jrf, it's nice to see we getting an early start for PHP 8!

As for the approach of making these problematic functions drop their default values, PHP 7.1 and later throw ArgumentCountError< so users are already aware of this. However, for PHP 5.6 and 7.0 only raise a warning if the user does not pass a required parameter: ​https://3v4l.org/o1TKv

I think dropping the default values can have non-zero number of users who might have ignored the warning, but get an exception in WordPress 5.6.

[jrf]

I think dropping the default values can have non-zero number of users who might have ignored the warning, but get an exception in WordPress 5.6.

If they did, IMO, they did so at their own peril and the plugin/theme would already break on PHP 7.1+.

As PHP < 7.1 support is slated to be removed from WP anyway, I don't think that plugins/themes which currently aren't compatible with PHP 7.1+ should be a concern.

Also note that _wp_delete_tax_menu_item() is marked as a private function, so has no promise of backward-compatibility anyhow.

Even so, for arguments sake, I've done a plugin + theme repository search to see if I could find any problematic plugins or themes and the net result is: 0.

Search results:

• ​Plugins using `get_comment_delimited_block_content()` without passing arguments: 0
• ​Themes using `get_comment_delimited_block_content()` without passing arguments: 0
• ​Plugins using `do_enclose()` without passing arguments: 11 - 100% false positives
• ​Themes using `do_enclose()` without passing arguments: 0
• ​Plugins using `_wp_delete_tax_menu_item()` without passing arguments: 5 - 100% false positives
• ​Themes using `_wp_delete_tax_menu_item()` without passing arguments: 0

[ayeshrajans]

That's awesome, thank you.

[SergeyBiryukov]

50343-required-parameters-before-optional.patch​ looks great, I only have some notes on the docs:

• _wp_delete_tax_menu_item() is a private function hooked to delete_term, which always passes a non-zero term ID, so it doesn't look like documenting "Pass the integer 0 if the object id is unknown" is necessary.
• It might be a good idea to also make the $object_id parameter of _wp_delete_post_menu_item() required, for consistency. It's hooked to delete_post, which also always passes a non-zero post ID. Both functions were introduced in [14295]. Calling them without a valid object ID doesn't make much sense anyway.
• get_comment_delimited_block_content() could use some clarification of the null value, something like: "Block name. Null if the block name is unknown, e.g. Classic blocks have their name set to null".

Going to handle these adjustments on commit.

[SergeyBiryukov]

In 48794:

Code Modernization: Fix PHP 8 deprecation notices for optional function parameters declared before required parameters.

As it already was not possible to pass the required parameters without also passing the optional one anyway, removing the default value for the (not so) optional parameters should not affect backward compatibility.

This change affects three functions in core:

• get_comment_delimited_block_content()
• do_enclose()
• _wp_delete_tax_menu_item()

Props jrf, ayeshrajans, desrosj.
Fixes #50343.

[jrf]

Good points about the docs. Thanks @SergeyBiryukov for doing the finishing touches ;-)

[SergeyBiryukov]

In 48795:

Menus: Make the $object_id parameter of _wp_delete_post_menu_item() required, for consistency with _wp_delete_tax_menu_item().

The function is private (only intended for core usage) and is hooked to the delete_post action, which always passes a non-zero post ID.

Follow-up to [14295], [48794].

See #50343.

Patch committed. Closing.

[jrf]

Adding needs-dev-note to make sure this change gets mentioned in one of the WP 5.6 dev posts.

[jrf]

SimplePie has released a new version containing the fix mentioned above. See #51521

[daisyo]

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