Bug: when() & whenNot() evaluate empty array to true

[ECode16]

PHP Version

8.3

CodeIgniter4 Version

4.6.1

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

No response

What happened?

If condition for function when() or whenNot() in ConditionalTrait is empty array the function evaluate it to true (as opposed to PHP which evaluates it to false).

Steps to Reproduce

when([], $callback)

Expected Output

Callback not called.

Anything else?

Same thing is happening with "0", 0 and 0.0, if this behaviour is intentional I believe it should be mentioned in description and user guide.

[michalsn]

Yes, it seems like a bug since the original code in the trait contained a condition with if ($condition) for the when() method and if (! $condition) for the notWhen() method.

[paulbalandan]

Yes, I think that's a bug. But come to think of it, I think it should not account for all possible truthy/falsey values. After all, it is a condition so at the very least I'm expecting the type to be a boolean or maybe a callable returning a boolean.

[paulbalandan]

The workaround to this, or perhaps you may put it as a proper fix is to convert the condition to a proper boolean: like $cond !== []

[michalsn]

Values like [], 0, "0", 0.0, and null are treated as falsy in native PHP conditionals. Changing this behavior introduces inconsistency and may break user expectations.

IMO, the refactor (done to satisfy a PHPStan rule) introduced a behavioral breaking change.

The user guide states that (https://codeigniter.com/user_guide/database/query_builder.html#builder-when):

The first parameter is the condition, and it should evaluate to a boolean.

For me, the user guide implies PHP truthiness, so the current logic violates our documentation.

I would go with:

if (! in_array($condition, [false, 0, 0.0, '', '0', [], null], true)) {

[ECode16]

At risk of making fool of myself but why not just explicitly convert it to bool?

[michalsn]

That’s not a bad suggestion, and I would be in favor - if only it wouldn’t bring us back to square one, which is PHPStan complaining.

[paulbalandan]

I think @ECode16 is suggesting to cast to bool instead of the in_array check.

[michalsn]

@paulbalandan would that be okay with PHPStan? Because, like I said (probably poorly) - I like the idea. I just had an impression that PHPStan would not like that. I'll be able to check it out in a few hours.

[paulbalandan]

I think yes, what I remember is PHPStan complains when the expression in the if condition is not a proper boolean.

[michalsn]

I've confirmed that casting to boolean doesn’t cause any issues with PHPStan. Thank you @ECode16 for the suggestion.

[ECode16]

Happy to help.