Disallow empty CompositeExpression

[BenMorel]

Q	A
Type	improvement
BC Break	no
Fixed issues	#3845

Summary

This PR disallows an empty CompositeExpression, fixing #3845.

[BenMorel]

Do we want to keep the constructor public?

Making it private would require a change to the following logic:

dbal/lib/Doctrine/DBAL/Query/QueryBuilder.php

Lines 1039 to 1052 in 39dd1e5

	private function appendToPredicate($currentPredicate, string $type, ...$predicates)
	{
	if ($currentPredicate instanceof CompositeExpression && $currentPredicate->getType() === $type) {
	return $currentPredicate->with(...$predicates);
	}
	if ($currentPredicate !== null) {
	array_unshift($predicates, $currentPredicate);
	} elseif (count($predicates) === 1) {
	return $predicates[0];
	}
	return new CompositeExpression($type, ...$predicates);
	}

[morozov]

It feels like this logic belongs to the expression class itself. It’s already aware of its type and parts and should be able to produce a new instance of self. This way, we wouldn’t even need to expose getType() anymore.

But I’m not sure how to put it exactly. Let’s leave it public for now.

[BenMorel]

What about moving these to static methods in CompositeExpression?

Here's how it would look like: BenMorel@a123732 (not included in this PR for now).

This way, we can make the constructor private, and remove the getType() method. Note that we still have to keep the TYPE_* constants public, unless we split appendToPredicate() into two distinct methods.

[morozov]

What about moving these to static methods in CompositeExpression?

Yeah, this is exactly what I had in mind. Let’s take it out of scope since it still needs additional discussion. If you’re fine with that, we can merge the PR in its current state.

[BenMorel]

All good for me. Let's merge this one, and I'll see what I can do in another PR!

[morozov]

Merged. Thanks, @BenMorel!