PHP 7.4 compatibility fix / implode argument order

[jrf]

implode() takes two parameters, $glue and $pieces.
For historical reasons, implode() accepted these parameters in either order, though it was recommended to use the documented argument order of implode( $glue, $pieces ).

PHP 7.4 is slated to deprecate the tolerance for passing the parameters for implode() in reverse order.
PHP 8.0 is expected to remove the tolerance for this completely.

Refs:

• ​https://wiki.php.net/rfc/deprecations_php_7_4#implode_parameter_order_mix
• ​https://php.net/manual/en/function.implode.php

Related:

• ​https://github.com/rmccue/Requests/pull/346
• ​https://github.com/simplepie/simplepie/pull/617

Note: join() is an alias of implode(). It is generally considered best practice to use the canonical function rather than an alias, however, that is outside the scope of this ticket.

[jrf]

FYI: I have reviewed ALL function calls to implode() and join() to get at this patch. Based on the current trunk, there are no remaining issues.

[johnbillion]

The changes to Requests and Simplepie need to be made upstream to their respective repos.

The other two changes look good.

[jrf]

Replying to johnbillion:

The changes to Requests and Simplepie need to be made upstream to their respective repos.

The other two changes look good.

Pull requests to both upstream repos have already been opened (see links above), but neither is very actively maintained.

The Requests dependency in Core is at the latest version - 1.7 -, but that version was released in October 2016 (!!!).
Ref: ​https://github.com/rmccue/Requests/releases

The Requests repo has had no commits since December 2017 and only a few, mostly DevOps related ones since the last release.

The SimplePie dependency in Core is at version 1.3.1 - which was released in October 2012 😰, while the current version is 1.5.2 released in August 2018.
Ref: ​https://github.com/simplepie/simplepie/releases

And while the SimplePie repo has actually had some commits this year (February), the version used in WP Core is pretty far behind the latest release.

So, yes, while I agree that the fixes should primarily be made upstream, I don't think WP should be throwing deprecation notices when run on PHP 7.4 while we are waiting for
1) new releases from upstream and
2) the existing WP Core versions of those libraries to be updated.

[jrf]

FYI:

• The PR to SimplePie has been merged, but no new version has been tagged yet. I've opened an issue asking for them to tag a new release: ​https://github.com/simplepie/simplepie/issues/620
• The PR to Requests has had no response whatsoever. I've bumped the PR hoping to get some traction: ​https://github.com/rmccue/Requests/pull/346#issuecomment-531415325

The patch for Core looks good and should IMO be committed ASAP.

[desrosj]

In 46155:

General: Ensure the arguments passed to implode() are in the correct order.

The implode() function accepts two. parameters, $glue and $pieces. For historical reasons, these parameters have been accepted in any order, though it was recommended that the documented order of $glue, $pieces be used.

Starting in PHP 7.4, specifying the parameters in the reverse order will trigger a deprecation notice with the plan to remove this tolerance in PHP 8.0.

This change fixes the occurrences of reversed arguments in Core with the exception of those contained in included external libraries. These will be handled separately.

Props jrf, jorbin.
See #47746.

[desrosj]

I'm leaving this open for now, but it may be best to tackle the remaining issues in separate tickets to update the two libraries.

I know @jrf has been working with @rmccue to get a new version of Requests released, but it does not seem SimplePie has seen any traction.

[jrf]

New issue introduced in: [46175]

New patch added to #36824 as that's the ticket which introduced it.

[SergeyBiryukov]

In 46182:

Tests: Replace join() with implode() in do_enclose() tests introduced in [46175], and ensure the arguments passed are in the correct order.

The implode() function accepts two parameters, $glue and $pieces. For historical reasons, these parameters have been accepted in any order, though it was recommended that the documented order of $glue, $pieces be used. It is also generally considered best practice to use the canonical function rather than an alias.

Starting in PHP 7.4, specifying the parameters in the reverse order will trigger a deprecation notice with the plan to remove this tolerance in PHP 8.0.

Props jrf.
Fixes #36824. See #47746.

[jrf]

Status summary:

• Issues in WP Core native code have been fixed.
• Issues in the external Requests and SimplePie libraries remain unpatched at this moment.

Regarding SimplePie:

• Ticket #36669 is open to address this, but has just been punted to Future Release.
• The issue has been patched upstream, but is not contained in a tagged release yet.
• If SimplePie does not get updated to a more recent version in WP 5.3, the existing code should be patched for the time being. I will attach a patch for this.

Regarding Requests:

• The issue has been patched upstream (above mentioned PR has been merged in the mean time), but is not contained in a tagged release yet.
• Still hoping for a new tagged release over the next few days to go into WP 5.3.

[jrf]

SimplePie has released a new version including the changes needed for PHP 7.4 support: ​https://github.com/simplepie/simplepie/releases/tag/1.5.3

[desrosj]

47746-simplepie-1-5-3.diff​ updates SimplePie to version 1.5.3.

Here is a test build on my fork: ​https://travis-ci.org/desrosj/wordpress-develop/builds/588479168

[desrosj]

Since there has been no tagged release for Requests, requests-1-7-3470169.diff​ updates Requests to version 1.7.3470169, which corresponds to the most recent commit.

[jrf]

@desrosj 👍🏻

FYI: Just checked with @rmccue regarding Requests and the intention is still to try and get a new release tagged over the next week.

[stevenkword]

Safety first with regards to SimplePie. It's so late in the 5.3 cycle that I don't feel comfortable making a decision at this point. Please follow the related ticket, #36669, where we will introduce the library update in the next development cycle to ensure proper time to validate compatibility among plugin authors.

[desrosj]

Let's move forward with 47746-patch-up-SimplePie.patch​ and requests-1-7-3470169.diff​ with the intention to fully update SimplePie through #36669 in 5.4, and when Requests is officially tagged the version string can be changed during beta.

[iseulde]

In 46257:

r46253: Use implode instead of join, see #47746

[desrosj]

In 46258:

External Libraries: Fix PHP 7.4 compatibility issues in the Requests library.

See: ​https://github.com/rmccue/Requests/compare/v1.7.0...3470169

Props jrf, jorbin, desrosj.
See #47746.

[desrosj]

In 46260:

External Libraries: Fix PHP 7.4 compatibility issues in the SimplePie library.

See: ​https://github.com/simplepie/simplepie/commit/38b504969ed08903cb12718e8270263a8c93080e

Props jrf, stevenkword, jorbin, desrosj.
See #47746.

[ayeshrajans]

It looks like SimplePie library has also cut a new release with this fix 👍🏽.

[jrf]

@iseulde Thanks for catching that newly introduced issue straight away ;-)

@desrosj Thanks for committing these last two patches.

I think we can close this ticket now as the current code base should be clean of the issue now.
Fingers crossed 🤞🏻 no further new issues of this type will be introduced.

A new issue can/should probably be created to update Requests to the newly tagged version once released.

[desrosj]

Thanks, everyone for the work on this!

[jorbin]

In 46416:

External Libraries: Fix PHP 7.4 compatibility issue in the Requests library.

Moves ​https://github.com/rmccue/Requests/pull/370 into WordPress.

Previous [46258].

See #47746.

[desrosj]

Dev note: ​https://make.wordpress.org/core/2019/10/11/wordpress-and-php-7-4/