Merge pull request #6061 from morozov/psalm-5

Update Psalm to 5.12.0
doctrine · Jun 13, 2023 · 25f8477 · 25f8477
2 parents 95d093a + ebdea00
commit 25f8477
Show file tree

Hide file tree

Showing 38 changed files with 297 additions and 174 deletions.
diff --git a/composer.json b/composer.json
@@ -40,7 +40,7 @@
     "require-dev": {
         "doctrine/coding-standard": "12.0.0",
         "fig/log-test": "^1",
-        "jetbrains/phpstorm-stubs": "2022.3",
+        "jetbrains/phpstorm-stubs": "dev-master#b51c647ece0a88c59fbc94028daebd047377bcdb",
         "phpstan/phpstan": "1.10.14",
         "phpstan/phpstan-phpunit": "1.3.11",
         "phpstan/phpstan-strict-rules": "^1.5",
@@ -49,7 +49,7 @@
         "squizlabs/php_codesniffer": "3.7.2",
         "symfony/cache": "^5.4|^6.0",
         "symfony/console": "^4.4.30|^5.4|^6.0",
-        "vimeo/psalm": "4.30.0"
+        "vimeo/psalm": "5.12.0"
     },
     "suggest": {
         "symfony/console": "For helpful console commands such as SQL execution and import of files."

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -19,6 +19,7 @@ parameters:
                 - src/Driver/AbstractOracleDriver/EasyConnectString.php
                 - src/Platforms/*Platform.php
                 - src/Schema/*SchemaManager.php
+                - tests/TestUtil.php
 
         # In some namespaces, we use array<string,mixed>, some elements of which are actually boolean
         -
@@ -89,6 +90,13 @@ parameters:
             count: 1
             path: src/Driver/Mysqli/Connection.php
 
+        # https://github.com/phpstan/phpstan/issues/9429
+        -
+            message: '~^Strict comparison using === between int<0, max> and false will always evaluate to false.$~'
+            paths:
+                - src/Driver/IBMDB2/Connection.php
+                - src/Driver/IBMDB2/Result.php
+
 includes:
     - vendor/phpstan/phpstan-phpunit/extension.neon
     - vendor/phpstan/phpstan-phpunit/rules.neon

diff --git a/psalm-strict.xml b/psalm-strict.xml
@@ -13,4 +13,14 @@
             <directory name="vendor" />
         </ignoreFiles>
     </projectFiles>
+    <issueHandlers>
+        <NoValue>
+            <errorLevel type="suppress">
+                <!--
+                    This error looks bogus.
+                -->
+                <file name="static-analysis/driver-manager-retrieves-correct-connection-type.php"/>
+            </errorLevel>
+        </NoValue>
+    </issueHandlers>
 </psalm>
diff --git a/psalm.xml.dist b/psalm.xml.dist
@@ -2,6 +2,8 @@
 <psalm
     errorLevel="2"
     phpVersion="8.2"
+    findUnusedBaselineEntry="false"
+    findUnusedCode="false"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xmlns="https://getpsalm.org/schema/config"
     xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
@@ -22,6 +24,15 @@
         <file name="vendor/jetbrains/phpstorm-stubs/sqlsrv/sqlsrv.php" />
     </stubs>
     <issueHandlers>
+        <ArgumentTypeCoercion>
+            <!--
+                The actual return type of the driver functions or methods depends on the
+                values of the passed arguments, so it cannot be determined statically.
+            -->
+            <errorLevel type="suppress">
+                <file name="src/Driver/PgSQL/Result.php"/>
+            </errorLevel>
+        </ArgumentTypeCoercion>
         <ConflictingReferenceConstraint>
             <errorLevel type="suppress">
                 <!--
@@ -85,6 +96,18 @@
                 <file name="src/Driver/OCI8/Connection.php"/>
             </errorLevel>
         </ImplementedReturnTypeMismatch>
+        <InaccessibleProperty>
+            <errorLevel type="suppress">
+                <!--
+                     Even though DateInterval properties are recommended to be considered as readonly,
+                     there is no way to initialize an inverted interval using the constructor.
+
+                     See: https://github.com/vimeo/psalm/pull/9895
+                -->
+                <file name="src/Types/DateIntervalType.php"/>
+                <file name="tests/Types/DateIntervalTest.php"/>
+            </errorLevel>
+        </InaccessibleProperty>
         <InvalidArgument>
             <errorLevel type="suppress">
                 <!-- We're testing with invalid input here. -->
@@ -104,6 +127,40 @@
                 <file name="src/Driver/PDO/Exception.php"/>
             </errorLevel>
         </InvalidPropertyAssignmentValue>
+        <LessSpecificReturnStatement>
+            <!--
+                The actual return type of the driver functions or methods depends on the
+                values of the passed arguments, so it cannot be determined statically.
+            -->
+            <errorLevel type="suppress">
+                <file name="src/Driver/IBMDB2/Result.php"/>
+                <file name="src/Driver/OCI8/Result.php"/>
+                <file name="src/Driver/PDO/Result.php"/>
+                <file name="src/Driver/PgSQL/Result.php"/>
+                <file name="src/Driver/SQLite3/Result.php"/>
+            </errorLevel>
+        </LessSpecificReturnStatement>
+        <MoreSpecificReturnType>
+            <errorLevel type="suppress">
+                <!--
+                    The actual return type of the driver functions or methods depends on the
+                    values of the passed arguments, so it cannot be determined statically.
+                -->
+                <file name="src/Driver/IBMDB2/Result.php"/>
+                <file name="src/Driver/OCI8/Result.php"/>
+                <file name="src/Driver/PDO/Result.php"/>
+                <file name="src/Driver/PgSQL/Result.php"/>
+                <file name="src/Driver/SQLite3/Result.php"/>
+            </errorLevel>
+        </MoreSpecificReturnType>
+        <NoValue>
+            <errorLevel type="suppress">
+                <!--
+                    This error looks bogus.
+                -->
+                <file name="static-analysis/driver-manager-retrieves-correct-connection-type.php"/>
+            </errorLevel>
+        </NoValue>
         <NullableReturnStatement>
             <errorLevel type="suppress">
                 <!--
@@ -131,6 +188,9 @@
                 <!-- See https://github.com/psalm/psalm-plugin-phpunit/pull/82 -->
                 <file name="tests/Functional/PrimaryReadReplicaConnectionTest.php"/>
                 <file name="tests/Functional/Schema/PostgreSQLSchemaManagerTest.php"/>
+                <!-- There is no way to mark a nullable element in an array shape as always set,
+                     and the PHPUnit plugin for Psalm does not seem to understand static assertions -->
+                <file name="tests/TestUtil.php"/>
             </errorLevel>
         </PossiblyUndefinedArrayOffset>
         <PossiblyUndefinedVariable>
@@ -212,6 +272,19 @@
                 <referencedClass name="OCILob"/>
             </errorLevel>
         </UndefinedDocblockClass>
+        <UnsupportedPropertyReferenceUsage>
+            <errorLevel type="suppress">
+                <!-- This code is valid -->
+                <file name="src/Driver/Mysqli/Result.php"/>
+                <file name="src/Driver/Mysqli/Statement.php"/>
+            </errorLevel>
+        </UnsupportedPropertyReferenceUsage>
+        <UnsupportedReferenceUsage>
+            <errorLevel type="suppress">
+                <!-- This code is valid -->
+                <file name="src/Driver/SQLSrv/Statement.php"/>
+            </errorLevel>
+        </UnsupportedReferenceUsage>
         <InvalidReturnStatement>
             <errorLevel type="suppress">
                 <!-- lastInsertId has a return type that does not match the one defined in the interface-->

diff --git a/src/Connection.php b/src/Connection.php
@@ -33,6 +33,7 @@
 use Traversable;
 
 use function array_key_exists;
+use function array_merge;
 use function assert;
 use function count;
 use function implode;
@@ -47,7 +48,7 @@
  *
  * @psalm-import-type Params from DriverManager
  * @psalm-type WrapperParameterType = string|Type|ParameterType|ArrayParameterType
- * @psalm-type WrapperParameterTypeArray = array<int, WrapperParameterType>|array<string, WrapperParameterType>
+ * @psalm-type WrapperParameterTypeArray = array<int<0, max>, WrapperParameterType>|array<string, WrapperParameterType>
  * @psalm-consistent-constructor
  */
 class Connection implements ServerVersionProvider
@@ -347,19 +348,14 @@ public function isTransactionActive(): bool
     /**
      * Adds condition based on the criteria to the query components
      *
-     * @param array<string, mixed> $criteria   Map of key columns to their values
-     * @param array<int, string>   $columns    Column names
-     * @param array<int, mixed>    $values     Column values
-     * @param array<int, string>   $conditions Key conditions
+     * @param array<string, mixed> $criteria Map of key columns to their values
      *
-     * @throws Exception
+     * @return array{list<string>, list<mixed>, list<string>}
      */
-    private function addCriteriaCondition(
-        array $criteria,
-        array &$columns,
-        array &$values,
-        array &$conditions,
-    ): void {
+    private function getCriteriaCondition(array $criteria): array
+    {
+        $columns = $values = $conditions = [];
+
         foreach ($criteria as $columnName => $value) {
             if ($value === null) {
                 $conditions[] = $columnName . ' IS NULL';
@@ -370,25 +366,25 @@ private function addCriteriaCondition(
             $values[]     = $value;
             $conditions[] = $columnName . ' = ?';
         }
+
+        return [$columns, $values, $conditions];
     }
 
     /**
      * Executes an SQL DELETE statement on a table.
      *
      * Table expression and columns are not escaped and are not safe for user-input.
      *
-     * @param array<string, mixed>                                                           $criteria
-     * @param array<int, string|ParameterType|Type>|array<string, string|ParameterType|Type> $types
+     * @param array<string, mixed>                                                                  $criteria
+     * @param array<int<0,max>, string|ParameterType|Type>|array<string, string|ParameterType|Type> $types
      *
      * @return int|numeric-string The number of affected rows.
      *
      * @throws Exception
      */
     public function delete(string $table, array $criteria = [], array $types = []): int|string
     {
-        $columns = $values = $conditions = [];
-
-        $this->addCriteriaCondition($criteria, $columns, $values, $conditions);
+        [$columns, $values, $conditions] = $this->getCriteriaCondition($criteria);
 
         $sql = 'DELETE FROM ' . $table;
 
@@ -443,9 +439,9 @@ public function getTransactionIsolation(): TransactionIsolationLevel
      *
      * Table expression and columns are not escaped and are not safe for user-input.
      *
-     * @param array<string, mixed>                                                           $data
-     * @param array<string, mixed>                                                           $criteria
-     * @param array<int, string|ParameterType|Type>|array<string, string|ParameterType|Type> $types
+     * @param array<string, mixed>                                                                  $data
+     * @param array<string, mixed>                                                                  $criteria
+     * @param array<int<0,max>, string|ParameterType|Type>|array<string, string|ParameterType|Type> $types
      *
      * @return int|numeric-string The number of affected rows.
      *
@@ -461,7 +457,11 @@ public function update(string $table, array $data, array $criteria = [], array $
             $set[]     = $columnName . ' = ?';
         }
 
-        $this->addCriteriaCondition($criteria, $columns, $values, $conditions);
+        [$criteriaColumns, $criteriaValues, $criteriaConditions] = $this->getCriteriaCondition($criteria);
+
+        $columns    = array_merge($columns, $criteriaColumns);
+        $values     = array_merge($values, $criteriaValues);
+        $conditions = array_merge($conditions, $criteriaConditions);
 
         if (is_string(key($types))) {
             $types = $this->extractTypeValues($columns, $types);
@@ -481,8 +481,8 @@ public function update(string $table, array $data, array $criteria = [], array $
      *
      * Table expression and columns are not escaped and are not safe for user-input.
      *
-     * @param array<string, mixed>                                                           $data
-     * @param array<int, string|ParameterType|Type>|array<string, string|ParameterType|Type> $types
+     * @param array<string, mixed>                                                                  $data
+     * @param array<int<0,max>, string|ParameterType|Type>|array<string, string|ParameterType|Type> $types
      *
      * @return int|numeric-string The number of affected rows.
      *
@@ -518,7 +518,7 @@ public function insert(string $table, array $data, array $types = []): int|strin
      * @param array<int, string>                                                             $columns
      * @param array<int, string|ParameterType|Type>|array<string, string|ParameterType|Type> $types
      *
-     * @return array<int, string|ParameterType|Type>|array<string, string|ParameterType|Type>
+     * @return array<int<0, max>, string|ParameterType|Type>
      */
     private function extractTypeValues(array $columns, array $types): array
     {
@@ -1308,13 +1308,13 @@ final public function convertException(Driver\Exception $e): DriverException
     }
 
     /**
-     * @param array<int, mixed>|array<string, mixed> $params
+     * @param list<mixed>|array<string, mixed> $params
      * @psalm-param WrapperParameterTypeArray $types
      *
      * @return array{
      *     string,
-     *     array<int, mixed>|array<string, mixed>,
-     *     array<int, string|ParameterType|Type>|array<string, string|ParameterType|Type>
+     *     list<mixed>|array<string, mixed>,
+     *     array<int<0, max>, string|ParameterType|Type>|array<string, string|ParameterType|Type>
      * }
      */
     private function expandArrayParameters(string $sql, array $params, array $types): array

diff --git a/src/Connections/PrimaryReadReplicaConnection.php b/src/Connections/PrimaryReadReplicaConnection.php
@@ -218,9 +218,14 @@ public function ensureConnectedToReplica(): void
     protected function connectTo(string $connectionName): DriverConnection
     {
         $params = $this->getParams();
-        assert(isset($params['primary'], $params['replica']));
+        assert(isset($params['primary']));
 
-        $connectionParams = $this->chooseConnectionConfiguration($connectionName, $params);
+        if ($connectionName === 'primary') {
+            $connectionParams = $params['primary'];
+        } else {
+            assert(isset($params['replica']));
+            $connectionParams = $this->chooseReplicaConnectionParameters($params['primary'], $params['replica']);
+        }
 
         try {
             return $this->driver->connect($connectionParams);
@@ -230,28 +235,25 @@ protected function connectTo(string $connectionName): DriverConnection
     }
 
     /**
-     * @param array<string, mixed> $params
-     * @psalm-param array{primary: OverrideParams, replica: array<OverrideParams>} $params
+     * @param OverrideParams        $primary
+     * @param array<OverrideParams> $replicas
      *
      * @return array<string, mixed>
      * @psalm-return OverrideParams
      */
-    protected function chooseConnectionConfiguration(
-        string $connectionName,
+    protected function chooseReplicaConnectionParameters(
         #[SensitiveParameter]
-        array $params,
+        array $primary,
+        #[SensitiveParameter]
+        array $replicas,
     ): array {
-        if ($connectionName === 'primary') {
-            return $params['primary'];
-        }
-
-        $config = $params['replica'][array_rand($params['replica'])];
+        $params = $replicas[array_rand($replicas)];
 
-        if (! isset($config['charset']) && isset($params['primary']['charset'])) {
-            $config['charset'] = $params['primary']['charset'];
+        if (! isset($params['charset']) && isset($primary['charset'])) {
+            $params['charset'] = $primary['charset'];
         }
 
-        return $config;
+        return $params;
     }
 
     /**

diff --git a/src/Driver/IBMDB2/Connection.php b/src/Driver/IBMDB2/Connection.php
@@ -74,7 +74,13 @@ public function exec(string $sql): int|string
             throw StatementError::new();
         }
 
-        return db2_num_rows($stmt);
+        $numRows = db2_num_rows($stmt);
+
+        if ($numRows === false) {
+            throw StatementError::new();
+        }
+
+        return $numRows;
     }
 
     public function lastInsertId(): string

diff --git a/src/Driver/IBMDB2/Exception/CannotCopyStreamToStream.php b/src/Driver/IBMDB2/Exception/CannotCopyStreamToStream.php
@@ -13,7 +13,7 @@
  */
 final class CannotCopyStreamToStream extends AbstractException
 {
-    /** @psalm-param array{message: string}|null $error */
+    /** @psalm-param array{message: string, ...}|null $error */
     public static function new(?array $error): self
     {
         $message = 'Could not copy source stream to temporary file';

diff --git a/src/Driver/IBMDB2/Exception/CannotCreateTemporaryFile.php b/src/Driver/IBMDB2/Exception/CannotCreateTemporaryFile.php
@@ -13,7 +13,7 @@
  */
 final class CannotCreateTemporaryFile extends AbstractException
 {
-    /** @psalm-param array{message: string}|null $error */
+    /** @psalm-param array{message: string, ...}|null $error */
     public static function new(?array $error): self
     {
         $message = 'Could not create temporary file';