[9.x] Conditionable::when returns Collection when callback or default return empty value (e.g. 0)

[spaceemotion]

• Laravel Version: 9.0
• PHP Version: 8.1.2
• Database Driver & Version: MySQL 8

Description:

I have the following snippet that calculates a number of serviced hours based on a condition:

$hours = $patient->serviceEntries->whenNotEmpty(
    static fn (Collection $entries) => $calculator->calcRegular($entries),
    static fn () => 0,
);

In case the calculator returns 0 hours (or the entries are empty) the expected result would be 0.
However, the function returns the collection itself, as the Conditionable trait returns $this when the return value is "empty":

framework/src/Illuminate/Conditionable/Traits/Conditionable.php

Lines 28 to 32 in ca33630

	if ($value) {
	return $callback($this, $value) ?: $this;
	} elseif ($default) {
	return $default($this, $value) ?: $this;
	}

The same logic applies to the unless function as well.

A possible solution is to use ?? instead of ?:. Not sure if this is a breaking change though (as it has worked before).

Steps To Reproduce:

$shouldBeZero = collect([1,2,3])->whenNotEmpty(fn () => 0, fn () => 0);
// returns the collection instance (with 1,2,3 as values)

[spaceemotion]

I just tried the example out in Tinkerwell Web, it correctly returns 0 (as expected). Not sure which version of PHP and Laravel is bundled there, but my local tinker session has the following output:

$ php artisan tinker
Psy Shell v0.11.1 (PHP 8.1.2 — cli) by Justin Hileman
>>> collect([1,2,3])->whenNotEmpty(static fn () => 0, static fn () => 0)
=> Illuminate\Support\Collection {#4755
     all: [
       1,
       2,
       3,
     ],
   }

[driesvints]

Seems like this is more of a feature request as this behavior is unchanged from 8.x? You mention: (as it has worked before). so was this different for you on 8.x?

[spaceemotion]

Yes, this happened after the upgrade to v9 yesterday.

[driesvints]

@inxilpro is this because of #37632 ?

[driesvints]

@spaceemotion it's just odd to me because besides the changes from above PR it's the exact same code.

[spaceemotion]

It seems like it is because of #37632.

The code in v8.x does not have the ?: $this appended:

https://github.com/laravel/framework/blob/8.x/src/Illuminate/Collections/Traits/EnumeratesValues.php#L478-L482

[driesvints]

@spaceemotion you're correct! Can you send in a PR?

[spaceemotion]

I can try this afternoon 👍

[inxilpro]

I think the reason I sent this PR to 9.x was because of this change. when() had very slightly different behavior across the framework, and that PR was meant to unify the behavior. I'll take a look a little later today to confirm.

[inxilpro]

Yeah, if you read #37632, I called this behavior out specifically. The reason for the change was to unify the behavior of when() across all implementations—some places used the ternary return and others didn't.

I agree with @spaceemotion that using the null coalesce operator would be preferable—I just used the ternary shorthand because that was what is used everywhere else in the framework.

[driesvints]

Can one of you send in a PR?

[spaceemotion]

@inxilpro Are you already working on this? If not, I could try it later.

[inxilpro]

Yeah, I can push up a PR.