Throw if a variadic parameter contains unexpected named arguments (#1…

…1260)
doctrine · Feb 13, 2024 · b6f4220 · b6f4220
1 parent b7860c7
commit b6f4220
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 4 deletions.
diff --git 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
@@ -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
@@ -6,6 +6,7 @@
 
 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 @@
  */
 class QueryBuilder implements Stringable
 {
+    use NoUnknownNamedArguments;
+
     /**
      * The array of DQL parts collected.
      *
@@ -611,6 +614,8 @@ public function add(string $dqlPartName, string|object|array $dqlPart, bool $app
      */
     public function select(mixed ...$select): static
     {
+        self::validateVariadicParameter($select);
+
         $this->type = QueryType::Select;
 
         if ($select === []) {
@@ -657,6 +662,8 @@ public function distinct(bool $flag = true): static
      */
     public function addSelect(mixed ...$select): static
     {
+        self::validateVariadicParameter($select);
+
         $this->type = QueryType::Select;
 
         if ($select === []) {
@@ -951,6 +958,8 @@ public function set(string $key, mixed $value): static
      */
     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 @@ public function where(mixed ...$predicates): static
      */
     public function andWhere(mixed ...$where): static
     {
+        self::validateVariadicParameter($where);
+
         $dql = $this->getDQLPart('where');
 
         if ($dql instanceof Expr\Andx) {
@@ -1006,6 +1017,8 @@ public function andWhere(mixed ...$where): static
      */
     public function orWhere(mixed ...$where): static
     {
+        self::validateVariadicParameter($where);
+
         $dql = $this->getDQLPart('where');
 
         if ($dql instanceof Expr\Orx) {
@@ -1033,6 +1046,8 @@ public function orWhere(mixed ...$where): static
      */
     public function groupBy(string ...$groupBy): static
     {
+        self::validateVariadicParameter($groupBy);
+
         return $this->add('groupBy', new Expr\GroupBy($groupBy));
     }
 
@@ -1051,6 +1066,8 @@ public function groupBy(string ...$groupBy): static
      */
     public function addGroupBy(string ...$groupBy): static
     {
+        self::validateVariadicParameter($groupBy);
+
         return $this->add('groupBy', new Expr\GroupBy($groupBy), true);
     }
 
@@ -1062,6 +1079,8 @@ public function addGroupBy(string ...$groupBy): static
      */
     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 @@ public function having(mixed ...$having): static
      */
     public function andHaving(mixed ...$having): static
     {
+        self::validateVariadicParameter($having);
+
         $dql = $this->getDQLPart('having');
 
         if ($dql instanceof Expr\Andx) {
@@ -1097,6 +1118,8 @@ public function andHaving(mixed ...$having): static
      */
     public function orHaving(mixed ...$having): static
     {
+        self::validateVariadicParameter($having);
+
         $dql = $this->getDQLPart('having');
 
         if ($dql instanceof Expr\Orx) {

diff --git a/tests/Tests/ORM/QueryBuilderTest.php b/tests/Tests/ORM/QueryBuilderTest.php
@@ -4,6 +4,7 @@
 
 namespace Doctrine\Tests\ORM;
 
+use BadMethodCallException;
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Criteria;
 use Doctrine\ORM\Cache;
@@ -275,6 +276,18 @@ public function testWhere(): void
         $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()