systopia / de.systopia.donrec

CiviCRM Donation Receipts Extension
GNU Affero General Public License v3.0
15 stars 25 forks source link

PHP 8.1 compatibility - do not attempt to `count()` `NULL` #178

Closed Detsieber closed 11 months ago

Detsieber commented 11 months ago

Although donrec in general is working fine with PHP 8.0/8.1, in certain conditions we get a wsod.

This is caused by the php function count, that behaves differently with php 8: https://www.php.net/manual/en/function.count.php

count() will now throw TypeError on invalid countable types passed to the value parameter.

I have prepared a pull request.

jensschuppe commented 11 months ago

If I get that correctly, you're talking about those two occurences of count() without a guard of the variable being countable: https://github.com/systopia/de.systopia.donrec/blob/d6ced5d878d6c1c80b028da827353201f5ff9703/CRM/Donrec/Form/Task/ContributeTask.php#L107 and https://github.com/systopia/de.systopia.donrec/blob/d6ced5d878d6c1c80b028da827353201f5ff9703/CRM/Donrec/Form/Task/ContributeTask.php#L114 and possibly more ...

… while I'm not sure what would cause $this->_contactIds not be an array. But the easiest solution would be to just call $this->getContactIDs() instead of direct access of $this->_contactIds, as the getter does a null-coalesce check with ?? [], which should be enough as the variable will be either an array or NULL.

Your PR, however, is filed against your fork, not the origin repo. I created #180 instead which should fix the issue for all occurences - would you be able to review that one, @Detsieber?