backdrop-contrib / devel

Provides helper functions for Backdrop developers.
https://backdropcms.org/project/devel
GNU General Public License v2.0
10 stars 13 forks source link

Warning: count(): Parameter must be an array or an object that implements Countable in krumo::_object() (line 1156 ../class.krumo.php). #101

Closed alanmels closed 3 years ago

alanmels commented 4 years ago

This issue is similar to https://github.com/backdrop-contrib/devel/issues/44, however is about the different line:

Warning: count(): Parameter must be an array or an object that implements Countable in krumo::_object() (line 1156 of /home/in/public_html/modules/devel/lib/krumo/class.krumo.php).

Replaced the line with:

<div class="krumo-element<?php echo !empty($data) ? ' krumo-expand' : '';?>">

and it works for my use case. However, since the file has number of other occurrences of count(), they all should be properly re-worked for the newer PHP versions.

kktsvetkov commented 4 years ago

Can you try upgrading Krumo, I think this is fixed in https://github.com/kktsvetkov/krumo/releases/tag/0.4.1

jenlampton commented 4 years ago

I replaced the old krumo lib with version 0.4.1 and I still have the error:

Warning: count(): Parameter must be an array or an object that implements Countable in krumo::_object() (line 1156 of backdrop/modules/contrib/devel/lib/krumo/class.krumo.php).

edit: apologies, this is not the version 0.4.1. Will try again.

jenlampton commented 4 years ago

The devel module is using version 2.0.1a, but the krumo fix was added to a much more recent version. I tried applying the patch to 2.0.1a but, unsurprisingly, it failed :)

We can add our own fix directly to the older version of the krumo library as follows:

<?php
  /**
  * Render a dump for an object
  *
  * @param mixed $data
  * @param string $name
  * @access private
  * @static
  */
  Private Static Function _object(&$data, $name) {
    $count = 1;
    if (is_array($data) || $data instanceof Countable) {
      $count = count($data);
    }
?>
<li class="krumo-child">

  <div class="krumo-element<?php echo $count > 0 ? ' krumo-expand' : '';?>">

...but it would probably be better to figure out how to make a newer version work.

jenlampton commented 4 years ago

PR here could use a review.

ghost commented 3 years ago

PR works for me. @quicksketch or @backdrop-contrib/bug-squad care to merge & release new version?

jenlampton commented 3 years ago

I've merged the PR, but the bug squad isn't authorized to make a new release.

bradbulger commented 3 years ago

The old behavior when you called count() on a non-countable variable was 0 for NULL and 1 for anything else. Should this be emulating that?

kktsvetkov commented 3 years ago

@jenlampton I think that code snippet with Countable might be from an old krumo fork and not from my project. What I do instead of the counting of the properties is to use a flag called $has_properties

/**
* Render a dump for an object
*
* @param mixed $data
* @param string $name
*/
private static function _object(&$data, $name)
{
    $has_properties = !empty(get_object_vars($data));
    ?>
    <li class="krumo-child">
        <div <?php if ($has_properties) {?> onClick="krumo.toggle(this);"<?php } ?>
...

https://github.com/kktsvetkov/krumo/blob/master/class.krumo.php#L1139

jenlampton commented 3 years ago

Thanks @kktsvetkov!

I had mentioned above that we are, in fact, using a very old version of Krumo that still contains the count() calls. I also linked to the commit where Krumo switched to using $has_properties instead.

We certainly need to update! Hence, #102.