[8.x] Compatibility between EnumeratesValues and BuildsQueries on when() method

[ImJustToNy]

This simple PR aims to ensure compatibility between src/Illuminate/Collections/Traits/EnumeratesValues.php and src/Illuminate/Database/Concerns/BuildsQueries.php on when() method.

Before, using when() method on collection without returning value, lead to collection being null (user must always return the same collection to chain further methods). This is understandable, but when we're doing the same thing inside query builder, we may return nothing (null) and Laravel corrects us returning builder again:

framework/src/Illuminate/Database/Concerns/BuildsQueries.php

Lines 208 to 210 in a6680d9

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

This leads to confusion in my opinion.
Thank you for reading.

[driesvints]

Ping @JosephSilber

[JosephSilber]

Might make sense, but is definitely a breaking change.

[JosephSilber]

But I can also hear an argument why the two should not be the same:

When building a query, you don't ever want to return null. It doesn't really make sense. You're querying the DB, and most likely just adding some condition to the query.

With collections, when is a conditional pipe, not a tap. Just like pipe, we always return what you've returned, since it's very possible you actually do want to return null.

If we really need this, maybe it would make sense to add a tapWhen method. But then we would have to also add all other variants (unless, whenEmpty, whenNotEmpty etc.). Doable, but not sure it's worth it.

---

I'm not convinced that they must be kept differently. Just saying that it conceptually makes sense.

[taylorotwell]

It is a breaking change so we would have to do it on master (if at all).

[ImJustToNy]

Thank you for your opinion @JosephSilber.
From my point of view, I don't really see a situation where a developer might want to return a null instead of e.g. empty collection. It is much clearer than a plain null.
You might be right, but then I would strongly suggest making some clarification in the docs and/or phpdoc, because that's very confusing behaviour IMHO.