Fix criteria parameter name collisions

src/Query/QueryExpressionVisitor.php

@@ -4,6 +4,7 @@
 
 namespace Doctrine\ORM\Query;
 
+use Closure;
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Expr\Comparison;
 use Doctrine\Common\Collections\Expr\CompositeExpression;
@@ -32,11 +33,19 @@ class QueryExpressionVisitor extends ExpressionVisitor
     /** @var list<mixed> */
     private array $parameters = [];
 
-    /** @param mixed[] $queryAliases */
+    /** @var Closure(Parameter): non-empty-string|null */
+    private readonly Closure|null $parameterBinder;
+
+    /**
+     * @param mixed[]                                      $queryAliases
+     * @param (callable(Parameter): non-empty-string)|null $parameterBinder
+     */
     public function __construct(
         private readonly array $queryAliases,
+        callable|null $parameterBinder = null,
     ) {
-        $this->expr = new Expr();
+        $this->expr            = new Expr();
+        $this->parameterBinder = $parameterBinder === null ? null : Closure::fromCallable($parameterBinder);
     }
 
     /**
@@ -103,76 +112,72 @@ public function walkComparison(Comparison $comparison): mixed
             }
         }
 
-        $parameter   = new Parameter($parameterName, $this->walkValue($comparison->getValue()));
-        $placeholder = ':' . $parameterName;
+        $parameter = new Parameter($parameterName, $this->walkValue($comparison->getValue()));
 
         switch ($comparison->getOperator()) {
             case Comparison::IN:
-                $this->parameters[] = $parameter;
-
-                return $this->expr->in($field, $placeholder);
+                return $this->expr->in($field, $this->bindParameter($parameter));
 
             case Comparison::NIN:
-                $this->parameters[] = $parameter;
-
-                return $this->expr->notIn($field, $placeholder);
+                return $this->expr->notIn($field, $this->bindParameter($parameter));
 
             case Comparison::EQ:
             case Comparison::IS:
                 if ($this->walkValue($comparison->getValue()) === null) {
                     return $this->expr->isNull($field);
                 }
 
-                $this->parameters[] = $parameter;
-
-                return $this->expr->eq($field, $placeholder);
+                return $this->expr->eq($field, $this->bindParameter($parameter));
 
             case Comparison::NEQ:
                 if ($this->walkValue($comparison->getValue()) === null) {
                     return $this->expr->isNotNull($field);
                 }
 
-                $this->parameters[] = $parameter;
-
-                return $this->expr->neq($field, $placeholder);
+                return $this->expr->neq($field, $this->bindParameter($parameter));
 
             case Comparison::CONTAINS:
                 $parameter->setValue('%' . $parameter->getValue() . '%', $parameter->getType());
-                $this->parameters[] = $parameter;
 
-                return $this->expr->like($field, $placeholder);
+                return $this->expr->like($field, $this->bindParameter($parameter));
 
             case Comparison::MEMBER_OF:
                 return $this->expr->isMemberOf($comparison->getField(), $comparison->getValue()->getValue());
 
             case Comparison::STARTS_WITH:
                 $parameter->setValue($parameter->getValue() . '%', $parameter->getType());
-                $this->parameters[] = $parameter;
 
-                return $this->expr->like($field, $placeholder);
+                return $this->expr->like($field, $this->bindParameter($parameter));
 
             case Comparison::ENDS_WITH:
                 $parameter->setValue('%' . $parameter->getValue(), $parameter->getType());
-                $this->parameters[] = $parameter;
 
-                return $this->expr->like($field, $placeholder);
+                return $this->expr->like($field, $this->bindParameter($parameter));
 
             default:
                 $operator = self::convertComparisonOperator($comparison->getOperator());
                 if ($operator) {
-                    $this->parameters[] = $parameter;
-
                     return new Expr\Comparison(
                         $field,
                         $operator,
-                        $placeholder,
+                        $this->bindParameter($parameter),
                     );
                 }
 
                 throw new RuntimeException('Unknown comparison operator: ' . $comparison->getOperator());
         }
     }
 
+    /** @return non-empty-string */
+    private function bindParameter(Parameter $parameter): string
+    {
+        $this->parameters[] = $parameter;
+
+        return $this->parameterBinder === null
+            ? ':' . $parameter->getName()
+            : ($this->parameterBinder)($parameter);
+    }
+
     public function walkValue(Value $value): mixed
     {
         return $value->getValue();

src/QueryBuilder.php

@@ -1255,14 +1255,22 @@ public function addCriteria(Criteria $criteria): static
             throw new Query\QueryException('No aliases are set before invoking addCriteria().');
         }
 
-        $visitor = new QueryExpressionVisitor($this->getAllAliases());
+        $visitor = new QueryExpressionVisitor(
+            $this->getAllAliases(),
+            function (Parameter $parameter): string {
+                if ($this->getParameter($parameter->getName()) === null) {
+                    $this->parameters->add($parameter);
+
+                    return ':' . $parameter->getName();
+                }
+
+                return $this->createNamedParameter($parameter->getValue(), $parameter->getType());
+            },
+        );
 
         $whereExpression = $criteria->getWhereExpression();
         if ($whereExpression) {
             $this->andWhere($visitor->dispatch($whereExpression));
-            foreach ($visitor->getParameters() as $parameter) {
-                $this->parameters->add($parameter);
-            }
         }
 
         foreach ($criteria->orderings() as $sort => $order) {

tests/Tests/ORM/Functional/Ticket/GH8702Test.php

@@ -0,0 +1,64 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\Common\Collections\Criteria;
+use Doctrine\ORM\Mapping\Column;
+use Doctrine\ORM\Mapping\Entity;
+use Doctrine\ORM\Mapping\Id;
+use Doctrine\Tests\OrmFunctionalTestCase;
+use PHPUnit\Framework\Attributes\Group;
+
+use function defined;
+
+#[Group('GH8702')]
+class GH8702Test extends OrmFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->createSchemaForModels(GH8702Item::class);
+
+        $this->_em->persist(new GH8702Item(1, 1));
+        $this->_em->persist(new GH8702Item(2, 2));
+        $this->_em->persist(new GH8702Item(3, 3));
+        $this->_em->flush();
+        $this->_em->clear();
+    }
+
+    public function testAddingMultipleCriteriaOnTheSameField(): void
+    {
+        $from = defined(Criteria::class . '::ASC') ? Criteria::create(true) : Criteria::create();
+        $from->where($from->expr()->gte('value', 2));
+
+        $to = defined(Criteria::class . '::ASC') ? Criteria::create(true) : Criteria::create();
+        $to->where($to->expr()->lte('value', 2));
+
+        $items = $this->_em->createQueryBuilder()
+            ->select('item')
+            ->from(GH8702Item::class, 'item')
+            ->addCriteria($from)
+            ->addCriteria($to)
+            ->getQuery()
+            ->getResult();
+
+        self::assertCount(1, $items);
+        self::assertSame(2, $items[0]->id);
+    }
+}
+
+#[Entity]
+class GH8702Item
+{
+    public function __construct(
+        #[Id]
+        #[Column(type: 'integer')]
+        public int $id,
+        #[Column(type: 'integer')]
+        public int $value,
+    ) {
+    }
+}

tests/Tests/ORM/QueryBuilderTest.php

@@ -31,6 +31,7 @@
 use RuntimeException;
 
 use function array_filter;
+use function defined;
 
 /**
  * Test case for the QueryBuilder class used to build DQL query string in a
@@ -540,6 +541,64 @@ public function testAddMultipleSameCriteriaWhere(): void
         self::assertNotNull($qb->getParameter('field_1'));
     }
 
+    #[Group('GH8702')]
+    public function testAddMultipleCriteriaWhereWithSameField(): void
+    {
+        $qb = $this->entityManager->createQueryBuilder();
+        $qb->select('alias1')->from(CmsUser::class, 'alias1');
+
+        $firstCriteria = defined(Criteria::class . '::ASC') ? Criteria::create(true) : Criteria::create();
+        $firstCriteria->where($firstCriteria->expr()->gte('field', 'value1'));
+
+        $secondCriteria = defined(Criteria::class . '::ASC') ? Criteria::create(true) : Criteria::create();
+        $secondCriteria->where($secondCriteria->expr()->lte('field', 'value2'));
+
+        $qb->addCriteria($firstCriteria);
+        $qb->addCriteria($secondCriteria);
+
+        self::assertEquals('alias1.field >= :field AND alias1.field <= :dcValue1', (string) $qb->getDQLPart('where'));
+        self::assertSame('value1', $qb->getParameter('field')->getValue());
+        self::assertSame('value2', $qb->getParameter('dcValue1')->getValue());
+    }
+
+    #[Group('GH8702')]
+    public function testAddCriteriaDoesNotReplaceExistingParameterWithSameName(): void
+    {
+        $qb = $this->entityManager->createQueryBuilder();
+        $qb->select('alias1')
+            ->from(CmsUser::class, 'alias1')
+            ->where('alias1.id = :field')
+            ->setParameter('field', 42);
+
+        $criteria = defined(Criteria::class . '::ASC') ? Criteria::create(true) : Criteria::create();
+        $criteria->where($criteria->expr()->eq('field', 'value'));
+
+        $qb->addCriteria($criteria);
+
+        self::assertEquals('alias1.id = :field AND alias1.field = :dcValue1', (string) $qb->getDQLPart('where'));
+        self::assertSame(42, $qb->getParameter('field')->getValue());
+        self::assertSame('value', $qb->getParameter('dcValue1')->getValue());
+    }
+
+    #[Group('GH8702')]
+    public function testAddMultipleNullCriteriaWhereWithSameFieldDoesNotAddParameters(): void
+    {
+        $qb = $this->entityManager->createQueryBuilder();
+        $qb->select('alias1')->from(CmsUser::class, 'alias1');
+
+        $firstCriteria = defined(Criteria::class . '::ASC') ? Criteria::create(true) : Criteria::create();
+        $firstCriteria->where($firstCriteria->expr()->eq('field', null));
+
+        $secondCriteria = defined(Criteria::class . '::ASC') ? Criteria::create(true) : Criteria::create();
+        $secondCriteria->where($secondCriteria->expr()->neq('field', null));
+
+        $qb->addCriteria($firstCriteria);
+        $qb->addCriteria($secondCriteria);
+
+        self::assertEquals('alias1.field IS NULL AND alias1.field IS NOT NULL', (string) $qb->getDQLPart('where'));
+        self::assertCount(0, $qb->getParameters());
+    }
+
     #[Group('DDC-2844')]
     public function testAddCriteriaWhereWithMultipleParametersWithSameField(): void
     {