Make CompositeExpression immutable

Author: BenMorel

UPGRADE.md

@@ -9,6 +9,10 @@
 5. The `add()`, `getQueryPart()`, `getQueryParts()`, `resetQueryPart()` and `resetQueryParts()` methods are removed.
 6. For a `select()` query, the `getSQL()` method now throws an expression if no `SELECT` expressions have been provided.
 
+## BC BREAK: `CompositeExpression` is now immutable
+
+The `add()` and `addMultiple()` methods now return a new instance instead of mutating the current one.
+
 ## BC BREAK: `OCI8Statement::convertPositionalToNamedPlaceholders()` is removed.
 
 The `OCI8Statement::convertPositionalToNamedPlaceholders()` method has been extracted to an internal utility class.

lib/Doctrine/DBAL/Query/Expression/CompositeExpression.php

@@ -5,11 +5,15 @@
 namespace Doctrine\DBAL\Query\Expression;
 
 use Countable;
+use function array_filter;
+use function array_values;
 use function count;
 use function implode;
 
 /**
  * Composite expression is responsible to build a group of similar expression.
+ *
+ * This class is immutable.
  */
 class CompositeExpression implements Countable
 {
@@ -43,47 +47,48 @@ class CompositeExpression implements Countable
      */
     public function __construct(string $type, array $parts = [])
     {
-        $this->type = $type;
-
-        $this->addMultiple($parts);
+        $this->type  = $type;
+        $this->parts = array_values(array_filter($parts, static function ($part) {
+            return ! ($part instanceof self && count($part) === 0);
+        }));
     }
 
     /**
-     * Adds multiple parts to composite expression.
+     * Returns a CompositeExpression with the given parts added.
      *
-     * @param array<int, self|string> $parts
+     * This instance is immutable and unaffected by this method call.
      *
-     * @return $this
+     * @param array<int, self|string> $parts
      */
     public function addMultiple(array $parts = []) : self
     {
+        $result = $this;
+
         foreach ($parts as $part) {
-            $this->add($part);
+            $result = $result->add($part);
         }
 
-        return $this;
+        return $result;
     }
 
     /**
-     * Adds an expression to composite expression.
+     * Returns a CompositeExpression with the given part added.
      *
-     * @param self|string $part
+     * This instance is immutable and unaffected by this method call.
      *
-     * @return $this
+     * @param self|string $part
      */
     public function add($part) : self
     {
-        if (empty($part)) {
-            return $this;
-        }
-
         if ($part instanceof self && count($part) === 0) {
             return $this;
         }
 
-        $this->parts[] = $part;
+        $that = clone $this;
+
+        $that->parts[] = $part;
 
-        return $this;
+        return $that;
     }
 
     /**

tests/Doctrine/Tests/DBAL/Query/Expression/CompositeExpressionTest.php

@@ -18,7 +18,7 @@ public function testCount() : void
 
         self::assertCount(1, $expr);
 
-        $expr->add('u.group_id = 2');
+        $expr = $expr->add('u.group_id = 2');
 
         self::assertCount(2, $expr);
     }
@@ -29,16 +29,21 @@ public function testAdd() : void
 
         self::assertCount(1, $expr);
 
-        $expr->add(new CompositeExpression(CompositeExpression::TYPE_AND, []));
+        $expr = $expr->add(new CompositeExpression(CompositeExpression::TYPE_AND, []));
 
         self::assertCount(1, $expr);
 
-        $expr->add(new CompositeExpression(CompositeExpression::TYPE_OR, ['u.user_id = 1']));
+        $expr = $expr->add(new CompositeExpression(CompositeExpression::TYPE_OR, ['u.user_id = 1']));
 
         self::assertCount(2, $expr);
 
+        // test immutability
         $expr->add('u.user_id = 1');
 
+        self::assertCount(2, $expr);
+
+        $expr = $expr->add('u.user_id = 1');
+
         self::assertCount(3, $expr);
     }
 

tests/Doctrine/Tests/DBAL/Query/Expression/ExpressionBuilderTest.php

@@ -38,7 +38,7 @@ public function testAndX(array $parts, string $expected) : void
         $composite = $this->expr->andX();
 
         foreach ($parts as $part) {
-            $composite->add($part);
+            $composite = $composite->add($part);
         }
 
         self::assertEquals($expected, (string) $composite);
@@ -99,7 +99,7 @@ public function testOrX(array $parts, string $expected) : void
         $composite = $this->expr->orX();
 
         foreach ($parts as $part) {
-            $composite->add($part);
+            $composite = $composite->add($part);
         }
 
         self::assertEquals($expected, (string) $composite);