Fix types on QueryBuilder (doctrine#9492)

n-e-m-a-nj-a · Mar 29, 2022 · 77113cd · 77113cd
1 parent d614b53
commit 77113cd
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 35 deletions.
diff --git a/lib/Doctrine/ORM/Query/Expr/Join.php b/lib/Doctrine/ORM/Query/Expr/Join.php
@@ -19,7 +19,10 @@ class Join
     public const ON   = 'ON';
     public const WITH = 'WITH';
 
-    /** @var string */
+    /**
+     * @var string
+     * @psalm-var self::INNER_JOIN|self::LEFT_JOIN
+     */
     protected $joinType;
 
     /** @var string */
@@ -28,7 +31,10 @@ class Join
     /** @var string|null */
     protected $alias;
 
-    /** @var string|null */
+    /**
+     * @var string|null
+     * @psalm-var self::ON|self::WITH|null
+     */
     protected $conditionType;
 
     /** @var string|Comparison|Composite|null */
@@ -44,6 +50,8 @@ class Join
      * @param string|null                      $conditionType The condition type constant. Either ON or WITH.
      * @param string|Comparison|Composite|null $condition     The condition for the join.
      * @param string|null                      $indexBy       The index for the join.
+     * @psalm-param self::INNER_JOIN|self::LEFT_JOIN $joinType
+     * @psalm-param self::ON|self::WITH|null $conditionType
      */
     public function __construct($joinType, $join, $alias = null, $conditionType = null, $condition = null, $indexBy = null)
     {
@@ -57,6 +65,7 @@ public function __construct($joinType, $join, $alias = null, $conditionType = nu
 
     /**
      * @return string
+     * @psalm-return self::INNER_JOIN|self::LEFT_JOIN
      */
     public function getJoinType()
     {
@@ -81,6 +90,7 @@ public function getAlias()
 
     /**
      * @return string|null
+     * @psalm-return self::ON|self::WITH|null
      */
     public function getConditionType()
     {

diff --git a/lib/Doctrine/ORM/Query/Parameter.php b/lib/Doctrine/ORM/Query/Parameter.php
@@ -54,9 +54,9 @@ public static function normalizeName($name)
     private $typeSpecified;
 
     /**
-     * @param string $name  Parameter name
-     * @param mixed  $value Parameter value
-     * @param mixed  $type  Parameter type
+     * @param string|int $name  Parameter name
+     * @param mixed      $value Parameter value
+     * @param mixed      $type  Parameter type
      */
     public function __construct($name, $value, $type = null)
     {

diff --git a/lib/Doctrine/ORM/QueryBuilder.php b/lib/Doctrine/ORM/QueryBuilder.php
@@ -196,7 +196,9 @@ public function setCacheable($cacheable)
     }
 
     /**
-     * @return bool TRUE if the query results are enable for second level cache, FALSE otherwise.
+     * Are the query results enabled for second level cache?
+     *
+     * @return bool
      */
     public function isCacheable()
     {
@@ -607,7 +609,7 @@ public function getParameters()
     /**
      * Gets a (previously set) query parameter of the query being constructed.
      *
-     * @param mixed $key The key (index or name) of the bound parameter.
+     * @param string|int $key The key (index or name) of the bound parameter.
      *
      * @return Parameter|null The value of the bound parameter.
      */
@@ -833,8 +835,8 @@ public function addSelect($select = null)
      *         ->setParameter('user_id', 1);
      * </code>
      *
-     * @param string $delete The class/type whose instances are subject to the deletion.
-     * @param string $alias  The class/type alias used in the constructed query.
+     * @param string|null $delete The class/type whose instances are subject to the deletion.
+     * @param string|null $alias  The class/type alias used in the constructed query.
      *
      * @return $this
      */
@@ -860,8 +862,8 @@ public function delete($delete = null, $alias = null)
      *         ->where('u.id = ?2');
      * </code>
      *
-     * @param string $update The class/type whose instances are subject to the update.
-     * @param string $alias  The class/type alias used in the constructed query.
+     * @param string|null $update The class/type whose instances are subject to the update.
+     * @param string|null $alias  The class/type alias used in the constructed query.
      *
      * @return $this
      */
@@ -886,9 +888,9 @@ public function update($update = null, $alias = null)
      *         ->from('User', 'u');
      * </code>
      *
-     * @param string $from    The class name.
-     * @param string $alias   The alias of the class.
-     * @param string $indexBy The index for the from.
+     * @param string      $from    The class name.
+     * @param string      $alias   The alias of the class.
+     * @param string|null $indexBy The index for the from.
      *
      * @return $this
      */
@@ -961,6 +963,7 @@ public function indexBy($alias, $indexBy)
      * @param string|null                                $conditionType The condition type constant. Either ON or WITH.
      * @param string|Expr\Comparison|Expr\Composite|null $condition     The condition for the join.
      * @param string|null                                $indexBy       The index for the join.
+     * @psalm-param Expr\Join::ON|Expr\Join::WITH|null $conditionType
      *
      * @return $this
      */
@@ -987,6 +990,7 @@ public function join($join, $alias, $conditionType = null, $condition = null, $i
      * @param string|null                                $conditionType The condition type constant. Either ON or WITH.
      * @param string|Expr\Comparison|Expr\Composite|null $condition     The condition for the join.
      * @param string|null                                $indexBy       The index for the join.
+     * @psalm-param Expr\Join::ON|Expr\Join::WITH|null $conditionType
      *
      * @return $this
      */
@@ -1027,6 +1031,7 @@ public function innerJoin($join, $alias, $conditionType = null, $condition = nul
      * @param string|null                                $conditionType The condition type constant. Either ON or WITH.
      * @param string|Expr\Comparison|Expr\Composite|null $condition     The condition for the join.
      * @param string|null                                $indexBy       The index for the join.
+     * @psalm-param Expr\Join::ON|Expr\Join::WITH|null $conditionType
      *
      * @return $this
      */
@@ -1277,7 +1282,7 @@ public function orHaving($having)
      * Replaces any previously specified orderings, if any.
      *
      * @param string|Expr\OrderBy $sort  The ordering expression.
-     * @param string              $order The ordering direction.
+     * @param string|null         $order The ordering direction.
      *
      * @return $this
      */
@@ -1292,7 +1297,7 @@ public function orderBy($sort, $order = null)
      * Adds an ordering to the query results.
      *
      * @param string|Expr\OrderBy $sort  The ordering expression.
-     * @param string              $order The ordering direction.
+     * @param string|null         $order The ordering direction.
      *
      * @return $this
      */

diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2090Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2090Test.php
@@ -5,6 +5,8 @@
 namespace Doctrine\Tests\ORM\Functional\Ticket;
 
 use DateTime;
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\ORM\Query\Parameter;
 use Doctrine\Tests\Models\Company\CompanyEmployee;
 use Doctrine\Tests\OrmFunctionalTestCase;
 
@@ -47,13 +49,11 @@ public function testIssue(): void
             ->set('e.startDate', ':date')
             ->set('e.salary', ':salary')
             ->where('e = :e')
-            ->setParameters(
-                [
-                    'e'      => $employee1,
-                    'date'   => $date1,
-                    'salary' => 101,
-                ]
-            )
+            ->setParameters(new ArrayCollection([
+                new Parameter('e', $employee1),
+                new Parameter('date', $date1),
+                new Parameter('salary', 101),
+            ]))
             ->getQuery()
             ->useQueryCache(true)
             ->execute();
@@ -63,13 +63,11 @@ public function testIssue(): void
             ->set('e.startDate', ':date')
             ->set('e.salary', ':salary')
             ->where('e = :e')
-            ->setParameters(
-                [
-                    'e'      => $employee2,
-                    'date'   => $date2,
-                    'salary' => 102,
-                ]
-            )
+            ->setParameters(new ArrayCollection([
+                new Parameter('e', $employee2),
+                new Parameter('date', $date2),
+                new Parameter('salary', 102),
+            ]))
             ->getQuery()
             ->useQueryCache(true)
             ->execute();
@@ -89,7 +87,11 @@ public function testIssue(): void
             ->set('e.startDate', '?1')
             ->set('e.salary', '?2')
             ->where('e = ?0')
-            ->setParameters([$employee1, $date1, 101])
+            ->setParameters(new ArrayCollection([
+                new Parameter('0', $employee1),
+                new Parameter('1', $date1),
+                new Parameter('2', 101),
+            ]))
             ->getQuery()
             ->useQueryCache(true)
             ->execute();
@@ -99,7 +101,11 @@ public function testIssue(): void
             ->set('e.startDate', '?1')
             ->set('e.salary', '?2')
             ->where('e = ?0')
-            ->setParameters([$employee2, $date2, 102])
+            ->setParameters(new ArrayCollection([
+                new Parameter('0', $employee2),
+                new Parameter('1', $date2),
+                new Parameter('2', 102),
+            ]))
             ->getQuery()
             ->useQueryCache(true)
             ->execute();

diff --git a/tests/Doctrine/Tests/ORM/QueryBuilderTest.php b/tests/Doctrine/Tests/ORM/QueryBuilderTest.php
@@ -7,12 +7,12 @@
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\Common\Collections\Criteria;
 use Doctrine\ORM\Cache;
-use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Query;
 use Doctrine\ORM\Query\Expr\Join;
 use Doctrine\ORM\Query\Parameter;
 use Doctrine\ORM\Query\ParameterTypeInferer;
 use Doctrine\ORM\QueryBuilder;
+use Doctrine\Tests\Mocks\EntityManagerMock;
 use Doctrine\Tests\Models\Cache\State;
 use Doctrine\Tests\Models\CMS\CmsArticle;
 use Doctrine\Tests\Models\CMS\CmsGroup;
@@ -29,15 +29,15 @@
  */
 class QueryBuilderTest extends OrmTestCase
 {
-    /** @var EntityManagerInterface */
+    /** @var EntityManagerMock */
     private $entityManager;
 
     protected function setUp(): void
     {
         $this->entityManager = $this->getTestEntityManager();
     }
 
-    protected function assertValidQueryBuilder(QueryBuilder $qb, $expectedDql): void
+    protected function assertValidQueryBuilder(QueryBuilder $qb, string $expectedDql): void
     {
         $dql = $qb->getDQL();
         $q   = $qb->getQuery();