Repository: doctrine/orm (php)

Issue:
Passing a single argument to the QueryBuilder::where() method using named arguments
### Bug Report

<!-- Fill in the relevant information below to help triage your issue. -->

|    Q        |   A
|------------ | ------
| BC Break    | yes
| Version     | 3.0.0

#### Summary
Hello there! When we try to call the `QueryBuilder::where()` method using a named argument, we encounter an issue with accessing an undefined array key. As a result, we get a "Warning: Undefined array key 0". The problem occurs because the value of the argument is an associative array.

https://github.com/doctrine/orm/blob/9acc70d5b8be55a6ac33e5c7313a124fa90dbe21/src/QueryBuilder.php#L954

#### Current behavior
Warning: Undefined array key 0

#### How to reproduce
Сall the `QueryBuilder::where()` method using a named argument:
```php
$qb->where(predicates: 't.column = :value')
```
<!--
Provide steps to reproduce the bug.
If possible, also add a code snippet with relevant configuration, entity mappings, DQL etc.
Adding a failing Unit or Functional Test would help us a lot - you can submit one in a Pull Request separately, referencing this bug report.
-->

#### Expected behavior
The method worked without errors.

Pull request description:
Throw if a variadic parameter contains unexpected named arguments
Fixes #11253.

If a method signature has a variadic parameter, you can call that method with any arbitrary named argument that you want. Hooray. However, our codebase rarely expects that and doing so sometimes raises error messages about invalid array access and similar issues.

This practice could also hide typos in named argument calls.

In order to improve the DX on those methods, I'd like to guard the calls to those methods by introducing a trait that validates if a variadic parameter contains any named arguments. If it finds any, an exception is raised that reports those arguments.

I've started with the `QueryBuilder` class, but I could extend this change to other classes if we agree that this is what we want.

Patch:
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
index 17a090f3a..688463b1b 100644
--- a/psalm-baseline.xml
+++ b/psalm-baseline.xml
@@ -1042,10 +1042,6 @@
   </file>
   <file src="src/QueryBuilder.php">
     <ArgumentTypeCoercion>
-      <code>$having</code>
-      <code>$having</code>
-      <code>$where</code>
-      <code>$where</code>
       <code><![CDATA[[$rootAlias => $join]]]></code>
       <code><![CDATA[[$rootAlias => $join]]]></code>
     </ArgumentTypeCoercion>
diff --git a/src/Internal/NoUnknownNamedArguments.php b/src/Internal/NoUnknownNamedArguments.php
new file mode 100644
index 000000000..7584744c1
--- /dev/null
+++ b/src/Internal/NoUnknownNamedArguments.php
@@ -0,0 +1,55 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\ORM\Internal;
+
+use BadMethodCallException;
+
+use function array_filter;
+use function array_is_list;
+use function array_keys;
+use function array_values;
+use function assert;
+use function debug_backtrace;
+use function implode;
+use function is_string;
+use function sprintf;
+
+use const DEBUG_BACKTRACE_IGNORE_ARGS;
+
+/**
+ * Checks if a variadic parameter contains unexpected named arguments.
+ *
+ * @internal
+ */
+trait NoUnknownNamedArguments
+{
+    /**
+     * @param TItem[] $parameter
+     *
+     * @template TItem
+     * @psalm-assert list<TItem> $parameter
+     */
+    private static function validateVariadicParameter(array $parameter): void
+    {
+        if (array_is_list($parameter)) {
+            return;
+        }
+
+        [, $trace] = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS);
+        assert(isset($trace['class']));
+
+        $additionalArguments = array_values(array_filter(
+            array_keys($parameter),
+            is_string(...),
+        ));
+
+        throw new BadMethodCallException(sprintf(
+            'Invalid call to %s::%s(), unknown named arguments: %s',
+            $trace['class'],
+            $trace['function'],
+            implode(', ', $additionalArguments),
+        ));
+    }
+}
diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php
index 115dea018..f79e2fe83 100644
--- a/src/QueryBuilder.php
+++ b/src/QueryBuilder.php
@@ -6,6 +6,7 @@ namespace Doctrine\ORM;
 
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Criteria;
+use Doctrine\ORM\Internal\NoUnknownNamedArguments;
 use Doctrine\ORM\Internal\QueryType;
 use Doctrine\ORM\Query\Expr;
 use Doctrine\ORM\Query\Parameter;
@@ -38,6 +39,8 @@ use function substr;
  */
 class QueryBuilder implements Stringable
 {
+    use NoUnknownNamedArguments;
+
     /**
      * The array of DQL parts collected.
      *
@@ -611,6 +614,8 @@ class QueryBuilder implements Stringable
      */
     public function select(mixed ...$select): static
     {
+        self::validateVariadicParameter($select);
+
         $this->type = QueryType::Select;
 
         if ($select === []) {
@@ -657,6 +662,8 @@ class QueryBuilder implements Stringable
      */
     public function addSelect(mixed ...$select): static
     {
+        self::validateVariadicParameter($select);
+
         $this->type = QueryType::Select;
 
         if ($select === []) {
@@ -951,6 +958,8 @@ class QueryBuilder implements Stringable
      */
     public function where(mixed ...$predicates): static
     {
+        self::validateVariadicParameter($predicates);
+
         if (! (count($predicates) === 1 && $predicates[0] instanceof Expr\Composite)) {
             $predicates = new Expr\Andx($predicates);
         }
@@ -976,6 +985,8 @@ class QueryBuilder implements Stringable
      */
     public function andWhere(mixed ...$where): static
     {
+        self::validateVariadicParameter($where);
+
         $dql = $this->getDQLPart('where');
 
         if ($dql instanceof Expr\Andx) {
@@ -1006,6 +1017,8 @@ class QueryBuilder implements Stringable
      */
     public function orWhere(mixed ...$where): static
     {
+        self::validateVariadicParameter($where);
+
         $dql = $this->getDQLPart('where');
 
         if ($dql instanceof Expr\Orx) {
@@ -1033,6 +1046,8 @@ class QueryBuilder implements Stringable
      */
     public function groupBy(string ...$groupBy): static
     {
+        self::validateVariadicParameter($groupBy);
+
         return $this->add('groupBy', new Expr\GroupBy($groupBy));
     }
 
@@ -1051,6 +1066,8 @@ class QueryBuilder implements Stringable
      */
     public function addGroupBy(string ...$groupBy): static
     {
+        self::validateVariadicParameter($groupBy);
+
         return $this->add('groupBy', new Expr\GroupBy($groupBy), true);
     }
 
@@ -1062,6 +1079,8 @@ class QueryBuilder implements Stringable
      */
     public function having(mixed ...$having): static
     {
+        self::validateVariadicParameter($having);
+
         if (! (count($having) === 1 && ($having[0] instanceof Expr\Andx || $having[0] instanceof Expr\Orx))) {
             $having = new Expr\Andx($having);
         }
@@ -1077,6 +1096,8 @@ class QueryBuilder implements Stringable
      */
     public function andHaving(mixed ...$having): static
     {
+        self::validateVariadicParameter($having);
+
         $dql = $this->getDQLPart('having');
 
         if ($dql instanceof Expr\Andx) {
@@ -1097,6 +1118,8 @@ class QueryBuilder implements Stringable
      */
     public function orHaving(mixed ...$having): static
     {
+        self::validateVariadicParameter($having);
+
         $dql = $this->getDQLPart('having');
 
         if ($dql instanceof Expr\Orx) {


Test patch:
diff --git a/tests/Tests/ORM/QueryBuilderTest.php b/tests/Tests/ORM/QueryBuilderTest.php
index 3c3f05ba4..96927bbbf 100644
--- a/tests/Tests/ORM/QueryBuilderTest.php
+++ b/tests/Tests/ORM/QueryBuilderTest.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Doctrine\Tests\ORM;
 
+use BadMethodCallException;
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Criteria;
 use Doctrine\ORM\Cache;
@@ -275,6 +276,18 @@ class QueryBuilderTest extends OrmTestCase
         $this->assertValidQueryBuilder($qb, 'SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.id = :uid');
     }
 
+    public function testWhereWithUnexpectedNamedArguments(): void
+    {
+        $qb = $this->entityManager->createQueryBuilder()
+            ->select('u')
+            ->from(CmsUser::class, 'u');
+
+        $this->expectException(BadMethodCallException::class);
+        $this->expectExceptionMessage('Invalid call to Doctrine\ORM\QueryBuilder::where(), unknown named arguments: foo, bar');
+
+        $qb->where(foo: 'u.id = :uid', bar: 'u.name = :name');
+    }
+
     public function testComplexAndWhere(): void
     {
         $qb = $this->entityManager->createQueryBuilder()