Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Hotfix #285 #287

Merged
merged 2 commits into from
Nov 30, 2017
Merged

Hotfix #285 #287

merged 2 commits into from
Nov 30, 2017

Conversation

michalbundyra
Copy link
Member

I've checked and changing to else is good enough solution. Even if we have there non-string type it's gonna work as before, see:
https://wiki.php.net/rfc/counting_non_countables\

Calling count() on a scalar or object that doesn't implement the Countable interface returns 1.

Removed also part of the condition to check if variable is instance of Countable. It is not possible in that place. Please note the condition was wrong and not even working because Countable was not imported.

Resolves #285

/cc @ezimuel @remicollet

It bring back the previous functionality
https://wiki.php.net/rfc/counting_non_countables

Calling `count()` on a scalar or object that doesn't implement
the Countable interface returns 1.
(This behaviour has been changed in PHP 7.2)
It is not possible to have there Countable element.
Please note it was not even imported so it was not working at all.
@shaunfreeman
Copy link

I was getting this warning message in PHP 7.2
Warning: count(): Parameter must be an array or an object that implements Countable in /vendor/zendframework/zend-db/src/Sql/AbstractSql.php on line 244
When using a reultset with Zend\DB\Resultset\Resultset this hotfix fixes this issue.

@michalbundyra
Copy link
Member Author

@shaunfreeman the fix will be released with 2.9.0, probably soon :)

@ezimuel ezimuel merged commit 366661f into zendframework:master Nov 30, 2017
@michalbundyra michalbundyra deleted the hotfix/285 branch November 30, 2017 13:53
@ezimuel
Copy link
Contributor

ezimuel commented Nov 30, 2017

@webimpress thanks! @shaunfreeman we'll release 2.9.0 next week.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants