The warning below is triggered on PHP 7.2 or later.

[warning] count(): Parameter must be an array or an object that implements Countable Api.php:357

CommentFileSizeAuthor
#8 interdiff-6-8.txt1013 bytesdscl
#8 3023929-count-parameter-must-be-countable-D8-8.patch550 bytesdscl
#6 3023929-fastly_compatibility_with_php7.2.patch1015 bytesLEalex
#2 fastly_compatibility_with_php7.2_3023929.patch989 bytesEugene Bsk
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Eugene Bsk created an issue. See original summary.

Eugene Bsk’s picture

Eugene Bsk CreditAttribution: Eugene Bsk commented
Assigned: Eugene Bsk » Unassigned
Status: Active » Needs review
FileSize
fastly_compatibility_with_php7.2_3023929.patch989 bytes

Here is the patch.
Could someone please review it.

gg4’s picture

gg4 CreditAttribution: gg4 commented

For reference: http://php.net/manual/en/migration72.incompatible.php

gg4’s picture

gg4 CreditAttribution: gg4 commented
Status: Needs review » Reviewed & tested by the community

Patch LGTM and applies fine.

markdorison’s picture

markdorison
he/him
CreditAttribution: markdorison at Chromatic commented
Title: Compatibility with php7.2 » count(): Parameter must be an array or an object that implements Countable
Issue summary: View changes

Patch works as expected. RTBC++.

LEalex’s picture

LEalex CreditAttribution: LEalex commented
FileSize
3023929-fastly_compatibility_with_php7.2.patch1015 bytes

i still have problems (sometimes), here's the fix (based #2) thats works for me

gg4’s picture

gg4 CreditAttribution: gg4 commented

Thanks, @LEalex. You missed you pieces of #2 and please provide and interdiff of changes between patches.

dscl’s picture

dscl CreditAttribution: dscl at DevBrains, Digital Garden commented
Status: Reviewed & tested by the community » Needs review
FileSize
3023929-count-parameter-must-be-countable-D8-8.patch550 bytes
interdiff-6-8.txt1013 bytes

There is no reason for changing the json() method to make it return associative arrays, specially because in that same file, at line 276, the result is referenced as an object.
And, as far as I can tell, the addition from #6 is the actual fix for the issue given that is was caused because json_decode() is returning null. The other 2 possible returns are countable, so we just need to make sure we don't let the null value get into count().

Still, considering the fact we are testing count() > 0, it would be simpler to just use empty() instead as it would cover all scenarios.

Pushing a new patch with the intediff to #6.

dman’s picture

dman CreditAttribution: dman at Platform.sh commented

Aye. No need to change the json_decode() call that I can see.
empty() is usually the most concise way to make warnings like this go away, I agree.

  • 547531f committed on 8.x-3.x 
    Merge pull request #19 from vvuksan/3023929-count-parameter-must-be-...
  • dscl authored 716484c on 8.x-3.x
    Issue #3023929 by dscl, Eugene Bsk, LEalex: count(): Parameter must be...
vvuksan-fastly’s picture

vvuksan-fastly CreditAttribution: vvuksan-fastly commented
Status: Needs review » Fixed

Fixed in 8.x-3.x-dev. Will be released in 8.x-3.8

vvuksan-fastly’s picture

vvuksan-fastly CreditAttribution: vvuksan-fastly commented
Status: Fixed » Closed (fixed)