Fix wrong types for AbstractQuery and child classes (#9774)

* Remove comment about BC I do not think we actually want to force our users to build an array collection when they want to use setParameters(). * Make phpdoc more accurate
doctrine · May 23, 2022 · 1f63389 · 1f63389
1 parent 359dd4e
commit 1f63389
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 25 deletions.
diff --git a/lib/Doctrine/ORM/AbstractQuery.php b/lib/Doctrine/ORM/AbstractQuery.php
@@ -36,6 +36,7 @@
 use function is_numeric;
 use function is_object;
 use function is_scalar;
+use function is_string;
 use function iterator_count;
 use function iterator_to_array;
 use function ksort;
@@ -282,7 +283,7 @@ public function setCacheMode($cacheMode)
      * The returned SQL syntax depends on the connection driver that is used
      * by this query object at the time of this method call.
      *
-     * @return string SQL query
+     * @return list<string>|string SQL query
      */
     abstract public function getSQL();
 
@@ -353,7 +354,6 @@ static function (Query\Parameter $parameter) use ($key): bool {
      */
     public function setParameters($parameters)
     {
-        // BC compatibility with 2.3-
         if (is_array($parameters)) {
             /** @psalm-var ArrayCollection<int, Parameter> $parameterCollection */
             $parameterCollection = new ArrayCollection();
@@ -401,8 +401,8 @@ public function setParameter($key, $value, $type = null)
      *
      * @param mixed $value
      *
-     * @return mixed[]|string|int|float|bool
-     * @psalm-return array|scalar
+     * @return mixed[]|string|int|float|bool|object|null
+     * @psalm-return array|scalar|object|null
      *
      * @throws ORMInvalidArgumentException
      */
@@ -1302,7 +1302,8 @@ protected function getHydrationCacheId()
             $parameters[$parameter->getName()] = $this->processParameterValue($parameter->getValue());
         }
 
-        $sql                    = $this->getSQL();
+        $sql = $this->getSQL();
+        assert(is_string($sql));
         $queryCacheProfile      = $this->getHydrationCacheProfile();
         $hints                  = $this->getHints();
         $hints['hydrationMode'] = $this->getHydrationMode();
@@ -1374,7 +1375,8 @@ public function __clone()
      */
     protected function getHash()
     {
-        $query  = $this->getSQL();
+        $query = $this->getSQL();
+        assert(is_string($query));
         $hints  = $this->getHints();
         $params = array_map(function (Parameter $parameter) {
             $value = $parameter->getValue();

diff --git a/lib/Doctrine/ORM/NativeQuery.php b/lib/Doctrine/ORM/NativeQuery.php
@@ -34,11 +34,9 @@ public function setSQL($sql): self
     /**
      * Gets the SQL query.
      *
-     * @return mixed The built SQL query or an array of all SQL queries.
-     *
      * @override
      */
-    public function getSQL()
+    public function getSQL(): string
     {
         return $this->sql;
     }

diff --git a/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/Collection/OneToManyPersister.php
@@ -14,7 +14,9 @@
 use function array_merge;
 use function array_reverse;
 use function array_values;
+use function assert;
 use function implode;
+use function is_string;
 
 /**
  * Persister for one-to-many collections.
@@ -225,7 +227,9 @@ private function deleteJoinedEntityCollection(PersistentCollection $collection):
             . ' FROM ' . $targetClass->name . ' t0 WHERE t0.' . $mapping['mappedBy'] . ' = :owner'
         )->setParameter('owner', $collection->getOwner());
 
-        $statement  = 'INSERT INTO ' . $tempTable . ' (' . $idColumnList . ') ' . $query->getSQL();
+        $sql = $query->getSQL();
+        assert(is_string($sql));
+        $statement  = 'INSERT INTO ' . $tempTable . ' (' . $idColumnList . ') ' . $sql;
         $parameters = array_values($sourceClass->getIdentifierValues($collection->getOwner()));
         $numDeleted = $this->conn->executeStatement($statement, $parameters);
 

diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php
@@ -174,7 +174,7 @@ final class Query extends AbstractQuery
     /**
      * The query cache lifetime.
      *
-     * @var int
+     * @var int|null
      */
     private $queryCacheTTL;
 
@@ -188,7 +188,7 @@ final class Query extends AbstractQuery
     /**
      * Gets the SQL query/queries that correspond to this DQL query.
      *
-     * @return mixed The built sql query or an array of all sql queries.
+     * @return list<string>|string The built sql query or an array of all sql queries.
      *
      * @override
      */
@@ -596,7 +596,7 @@ public function free(): void
     /**
      * Sets a DQL query string.
      *
-     * @param string $dqlQuery DQL Query.
+     * @param string|null $dqlQuery DQL Query.
      */
     public function setDQL($dqlQuery): self
     {

diff --git a/lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php b/lib/Doctrine/ORM/Query/Exec/AbstractSqlExecutor.php
@@ -18,7 +18,7 @@
  */
 abstract class AbstractSqlExecutor
 {
-    /** @var mixed[]|string */
+    /** @var list<string>|string */
     protected $_sqlStatements;
 
     /** @var QueryCacheProfile */

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -1,10 +1,5 @@
 parameters:
 	ignoreErrors:
-		-
-			message: "#^Method Doctrine\\\\ORM\\\\AbstractQuery\\:\\:processParameterValue\\(\\) should return array\\|bool\\|float\\|int\\|string but returns mixed\\.$#"
-			count: 1
-			path: lib/Doctrine/ORM/AbstractQuery.php
-
 		-
 			message: "#^Call to an undefined method Doctrine\\\\ORM\\\\Persisters\\\\Entity\\\\EntityPersister\\:\\:getCacheRegion\\(\\)\\.$#"
 			count: 1

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1044,9 +1044,6 @@
     </PossiblyFalseOperand>
   </file>
   <file src="lib/Doctrine/ORM/NativeQuery.php">
-    <LessSpecificImplementedReturnType occurrences="1">
-      <code>mixed</code>
-    </LessSpecificImplementedReturnType>
     <PropertyNotSetInConstructor occurrences="1">
       <code>$sql</code>
     </PropertyNotSetInConstructor>
@@ -1384,9 +1381,6 @@
     <InvalidScalarArgument occurrences="1">
       <code>$sqlParams</code>
     </InvalidScalarArgument>
-    <LessSpecificImplementedReturnType occurrences="1">
-      <code>mixed</code>
-    </LessSpecificImplementedReturnType>
     <LessSpecificReturnStatement occurrences="2">
       <code>parent::setHint($name, $value)</code>
       <code>parent::setHydrationMode($hydrationMode)</code>
@@ -1970,6 +1964,9 @@
     <PropertyNotSetInConstructor occurrences="1">
       <code>MultiTableUpdateExecutor</code>
     </PropertyNotSetInConstructor>
+    <PropertyTypeCoercion occurrences="1">
+      <code>$this-&gt;_sqlStatements</code>
+    </PropertyTypeCoercion>
   </file>
   <file src="lib/Doctrine/ORM/Query/Exec/SingleSelectExecutor.php">
     <PossiblyInvalidArgument occurrences="1">