There are multiple places where historical order of parameters is being used when calling implode()

PHP7.4:

Passing parameters to implode() in reverse order is deprecated, use implode($glue, $parts) instead of implode($parts, $glue).

See https://www.php.net/manual/en/migration74.deprecated.php#migration74.dep...

Comments

Taran2L created an issue. See original summary.

taran2l’s picture

Status: Active » Needs review
StatusFileSize
new2.67 KB

Patch attached

avpaderno’s picture

Title: [PHP7.4] [D7] Passing parameters to implode() in reverse order is deprecated, use implode($glue, $parts) instead of implode($parts, $glue). » Passing parameters to implode() in reverse order is deprecated, use implode($glue, $parts) instead of implode($parts, $glue).
avpaderno’s picture

Status: Needs review » Reviewed & tested by the community

This is quite simple: implode($glue, $parts) is not going to cause issues with any supported PHP version.

joseph.olstad’s picture

Issue tags: +Drupal 7.68 target, +Pending Drupal 7 commit

Triage.

First priority before this:

To increase our confidence in the test results for the above php 7.4 compatibility issue, these two others should go in first so we can re-queue php 7.3 and php 5.3 tests.
#3047844: [Regression] Tests fail on PHP 5.3
#3025335: session_id() cannot be changed after session is started

mustanggb’s picture

Issue tags: -Drupal 7.68 target +Drupal 7.69 target
mustanggb’s picture

Issue tags: -Drupal 7.69 target +Drupal 7.70 target
liam morland’s picture

Title: Passing parameters to implode() in reverse order is deprecated, use implode($glue, $parts) instead of implode($parts, $glue). » Pass parameters as implode($glue, $pieces); reverse order is deprecated
Issue tags: +PHP 7.4
andypost’s picture

izmeez’s picture

Re-queued testing with php 7.4 as per comment #5 with reference issues now closed.

izmeez’s picture

When patch in comment #2 is applied it resolves the deprecated implode function with php 7.4 but exposes warnings as described in #3084953: Trying to access array offset on value of type null in _drupal_build_css_path() where there is a working patch.

mcdruid’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -Pending Drupal 7 commit

Looking at the docs for implode() it seems it's not necessary to add an empty $glue parameter.

Looking at D8 after #3104421: PHP 7.4 deprecated reverse order of glue and pieces in implode, there are still examples of calls with just the $pieces param such as:

core/modules/filter/filter.module:580:    $text = implode($chunks);

[edit: removed some examples which were not PHP's native implode() function]

So I think we can remove the last few changes in the patch where there's only one param.

longwave’s picture

Status: Needs work » Needs review
StatusFileSize
new965 bytes
new1.16 KB
taran2l’s picture

Status: Needs review » Reviewed & tested by the community

I was also thinking about skipping the second param but decided to keep it. However, let's be consistent with D8

mcdruid’s picture

Issue tags: +Pending Drupal 7 commit

Great, thank you!

I've not been able to find any other calls with the params the wrong way around in D7 core.

This LGTM for commit; ready for final review.

fabianx’s picture

Assigned: Unassigned » mcdruid

RTBM - Ready to be merged, let's get this in.

  • mcdruid committed b20c7a9 on 7.x
    Issue #3084943 by longwave, Taran2L: Pass parameters as implode($glue, $...
mcdruid’s picture

Assigned: mcdruid » Unassigned
Status: Reviewed & tested by the community » Fixed
Issue tags: -Pending Drupal 7 commit

Thanks everybody!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.