Merge pull request #4319 from morozov/psalm-4

Bump Psalm level to 4
doctrine · Oct 7, 2020 · c361b36 · c361b36
2 parents 9f0e656 + 90a459f
commit c361b36
Show file tree

Hide file tree

Showing 17 changed files with 139 additions and 54 deletions.
diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php
@@ -905,6 +905,8 @@ public function quoteIdentifier($str)
 
     /**
      * {@inheritDoc}
+     *
+     * @param int|string|Type|null $type
      */
     public function quote($value, $type = ParameterType::STRING)
     {
@@ -1882,8 +1884,8 @@ private function _bindTypedValues($stmt, array $params, array $types)
     /**
      * Gets the binding type of a given type. The given type can be a PDO or DBAL mapping type.
      *
-     * @param mixed           $value The value to bind.
-     * @param int|string|null $type  The type to bind (PDO or DBAL).
+     * @param mixed                $value The value to bind.
+     * @param int|string|Type|null $type  The type to bind (PDO or DBAL).
      *
      * @return mixed[] [0] => the (escaped) value, [1] => the binding type.
      */

diff --git a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php b/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php
@@ -181,7 +181,7 @@ public static function convertPositionalToNamedPlaceholders($statement)
      * Finds next placeholder or opening quote.
      *
      * @param string             $statement               The SQL statement to parse
-     * @param string             $tokenOffset             The offset to start searching from
+     * @param int                $tokenOffset             The offset to start searching from
      * @param int                $fragmentOffset          The offset to build the next fragment from
      * @param string[]           $fragments               Fragments of the original statement
      *                                                    not containing placeholders
@@ -228,7 +228,7 @@ private static function findPlaceholderOrOpeningQuote(
      * Finds closing quote
      *
      * @param string $statement               The SQL statement to parse
-     * @param string $tokenOffset             The offset to start searching from
+     * @param int    $tokenOffset             The offset to start searching from
      * @param string $currentLiteralDelimiter The delimiter of the current string literal
      *
      * @return bool Whether the token was found

diff --git a/lib/Doctrine/DBAL/Driver/ResultStatement.php b/lib/Doctrine/DBAL/Driver/ResultStatement.php
@@ -75,22 +75,22 @@ public function fetch($fetchMode = null, $cursorOrientation = PDO::FETCH_ORI_NEX
      *
      * @deprecated Use fetchAllNumeric(), fetchAllAssociative() or fetchFirstColumn() instead.
      *
-     * @param int|null     $fetchMode     Controls how the next row will be returned to the caller.
-     *                                    The value must be one of the {@link FetchMode} constants,
-     *                                    defaulting to {@link FetchMode::MIXED}.
-     * @param int|null     $fetchArgument This argument has a different meaning depending on the value
-     *                                    of the $fetchMode parameter:
-     *                                    * {@link FetchMode::COLUMN}:
-     *                                      Returns the indicated 0-indexed column.
-     *                                    * {@link FetchMode::CUSTOM_OBJECT}:
-     *                                      Returns instances of the specified class, mapping the columns of each row
-     *                                      to named properties in the class.
-     *                                    * {@link PDO::FETCH_FUNC}: Returns the results of calling
-     *                                      the specified function, using each row's
-     *                                      columns as parameters in the call.
-     * @param mixed[]|null $ctorArgs      Controls how the next row will be returned to the caller.
-     *                                    The value must be one of the {@link FetchMode} constants,
-     *                                    defaulting to {@link FetchMode::MIXED}.
+     * @param int|null        $fetchMode     Controls how the next row will be returned to the caller.
+     *                                       The value must be one of the {@link FetchMode} constants,
+     *                                       defaulting to {@link FetchMode::MIXED}.
+     * @param int|string|null $fetchArgument This argument has a different meaning depending on the value
+     *                                       of the $fetchMode parameter:
+     *                                       * {@link FetchMode::COLUMN}:
+     *                                         Returns the indicated 0-indexed column.
+     *                                       * {@link FetchMode::CUSTOM_OBJECT}:
+     *                                         Returns instances of the specified class, mapping the columns of each row
+     *                                         to named properties in the class.
+     *                                       * {@link PDO::FETCH_FUNC}: Returns the results of calling
+     *                                         the specified function, using each row's
+     *                                         columns as parameters in the call.
+     * @param mixed[]|null    $ctorArgs      Controls how the next row will be returned to the caller.
+     *                                       The value must be one of the {@link FetchMode} constants,
+     *                                       defaulting to {@link FetchMode::MIXED}.
      *
      * @return mixed[]
      */

diff --git a/lib/Doctrine/DBAL/DriverManager.php b/lib/Doctrine/DBAL/DriverManager.php
@@ -12,7 +12,6 @@
 use Doctrine\DBAL\Driver\SQLSrv;
 
 use function array_keys;
-use function array_map;
 use function array_merge;
 use function assert;
 use function class_implements;
@@ -22,6 +21,7 @@
 use function parse_str;
 use function parse_url;
 use function preg_replace;
+use function rawurldecode;
 use function str_replace;
 use function strpos;
 use function substr;
@@ -280,13 +280,19 @@ private static function parseDatabaseUrl(array $params): array
             throw new Exception('Malformed parameter "url".');
         }
 
-        $url = array_map('rawurldecode', $url);
+        foreach ($url as $param => $value) {
+            if (! is_string($value)) {
+                continue;
+            }
+
+            $url[$param] = rawurldecode($value);
+        }
 
         // If we have a connection URL, we have to unset the default PDO instance connection parameter (if any)
         // as we cannot merge connection details from the URL into the PDO instance (URL takes precedence).
         unset($params['pdo']);
 
-        $params = self::parseDatabaseUrlScheme($url, $params);
+        $params = self::parseDatabaseUrlScheme($url['scheme'] ?? null, $params);
 
         if (isset($url['host'])) {
             $params['host'] = $url['host'];
@@ -412,23 +418,22 @@ private static function parseSqliteDatabaseUrlPath(array $url, array $params): a
     /**
      * Parses the scheme part from given connection URL and resolves the given connection parameters.
      *
-     * @param mixed[] $url    The connection URL parts to evaluate.
-     * @param mixed[] $params The connection parameters to resolve.
+     * @param string|null $scheme The connection URL scheme, if available
+     * @param mixed[]     $params The connection parameters to resolve.
      *
      * @return mixed[] The resolved connection parameters.
      *
      * @throws Exception If parsing failed or resolution is not possible.
      */
-    private static function parseDatabaseUrlScheme(array $url, array $params): array
+    private static function parseDatabaseUrlScheme($scheme, array $params): array
     {
-        if (isset($url['scheme'])) {
+        if ($scheme !== null) {
             // The requested driver from the URL scheme takes precedence
             // over the default custom driver from the connection parameters (if any).
             unset($params['driverClass']);
 
             // URL schemes must not contain underscores, but dashes are ok
-            $driver = str_replace('-', '_', $url['scheme']);
-            assert(is_string($driver));
+            $driver = str_replace('-', '_', $scheme);
 
             // The requested driver from the URL scheme takes precedence over the
             // default driver from the connection parameters. If the driver is

diff --git a/lib/Doctrine/DBAL/Query/QueryBuilder.php b/lib/Doctrine/DBAL/Query/QueryBuilder.php
@@ -1191,7 +1191,7 @@ private function getFromClauses()
     }
 
     /**
-     * @param string[] $knownAliases
+     * @param array<string,true> $knownAliases
      *
      * @throws QueryException
      */
@@ -1335,8 +1335,8 @@ public function createPositionalParameter($value, $type = ParameterType::STRING)
     }
 
     /**
-     * @param string   $fromAlias
-     * @param string[] $knownAliases
+     * @param string             $fromAlias
+     * @param array<string,true> $knownAliases
      *
      * @return string
      *

diff --git a/lib/Doctrine/DBAL/SQLParserUtils.php b/lib/Doctrine/DBAL/SQLParserUtils.php
@@ -64,7 +64,7 @@ public static function getPlaceholderPositions($statement, $isPositional = true)
     /**
      * Returns a zero-indexed list of placeholder position.
      *
-     * @return int[]
+     * @return list<int>
      */
     private static function getPositionalPlaceholderPositions(string $statement): array
     {
@@ -81,7 +81,7 @@ static function (string $_, int $placeholderPosition, int $fragmentPosition, arr
     /**
      * Returns a map of placeholder positions to their parameter names.
      *
-     * @return string[]
+     * @return array<int,string>
      */
     private static function getNamedPlaceholderPositions(string $statement): array
     {

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -21,6 +21,9 @@ parameters:
         - '~^Property Doctrine\\DBAL\\Schema\\Schema::\$_schemaConfig \(Doctrine\\DBAL\\Schema\\SchemaConfig\) does not accept default value of type false\.\z~'
         - '~^Return type \(int\|false\) of method Doctrine\\DBAL\\Driver\\OCI8\\OCI8Connection\:\:lastInsertId\(\) should be compatible with return type \(string\) of method Doctrine\\DBAL\\Driver\\Connection::lastInsertId\(\)~'
 
+        # https://github.com/phpstan/phpstan/issues/2857
+        - '~^Parameter #2 \$registeredAliases of static method Doctrine\\DBAL\\Query\\QueryException::nonUniqueAlias\(\) expects array<string>, array<int, int\|string> given\.\z~'
+
         # legacy variadic-like signature
         # TODO: remove in 3.0.0
         - '~^Method Doctrine\\DBAL(\\.*)?Connection::query\(\) invoked with \d+ parameters?, 0 required\.\z~'
@@ -111,3 +114,9 @@ parameters:
             message: '~Return type \(Doctrine\\DBAL\\Portability\\Statement\) of method Doctrine\\DBAL\\Portability\\Connection::prepare\(\) should be compatible with return type \(Doctrine\\DBAL\\Statement\) of method Doctrine\\DBAL\\Connection::prepare\(\)~'
             paths:
                 - %currentWorkingDirectory%/lib/Doctrine/DBAL/Portability/Connection.php
+
+        # Unlike Psalm, PHPStan doesn't understand the shape of the parse_str() return value
+        -
+            message: '~^Parameter #1 \$scheme of static method Doctrine\\DBAL\\DriverManager::parseDatabaseUrlScheme\(\) expects string\|null, int\|string\|null given\.$~'
+            paths:
+                - %currentWorkingDirectory%/lib/Doctrine/DBAL/DriverManager.php
diff --git a/psalm.xml b/psalm.xml
@@ -1,7 +1,7 @@
 <?xml version="1.0"?>
 <psalm
     totallyTyped="false"
-    errorLevel="5"
+    errorLevel="4"
     resolveFromConfigFile="true"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xmlns="https://getpsalm.org/schema/config"
@@ -42,6 +42,40 @@
                 <file name="lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvConnection.php"/>
             </errorLevel>
         </FalsableReturnStatement>
+        <ForbiddenCode>
+            <errorLevel type="suppress">
+                <!-- See https://github.com/doctrine/dbal/pull/3934 -->
+                <file name="lib/Doctrine/DBAL/Logging/EchoSQLLogger.php"/>
+
+                <!-- The call to var_dump() here is by design -->
+                <file name="lib/Doctrine/DBAL/Tools/Dumper.php"/>
+            </errorLevel>
+        </ForbiddenCode>
+        <ImplicitToStringCast>
+            <errorLevel type="suppress">
+                <!-- See https://github.com/doctrine/dbal/pull/4082 -->
+                <file name="lib/Doctrine/DBAL/Schema/Visitor/Graphviz.php"/>
+            </errorLevel>
+        </ImplicitToStringCast>
+        <ImplementedReturnTypeMismatch>
+            <errorLevel type="suppress">
+                <!-- See https://github.com/doctrine/dbal/pull/3906 -->
+                <file name="lib/Doctrine/DBAL/Driver/DrizzlePDOMySql/Driver.php"/>
+
+                <!-- See https://github.com/doctrine/dbal/pull/4030 -->
+                <file name="lib/Doctrine/DBAL/Driver/Mysqli/MysqliConnection.php"/>
+                <file name="lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php"/>
+
+                <!-- See https://github.com/doctrine/dbal/pull/3080 -->
+                <file name="lib/Doctrine/DBAL/Driver/PDOConnection.php"/>
+
+                <!-- See https://github.com/doctrine/dbal/issues/4156 -->
+                <file name="lib/Doctrine/DBAL/Portability/Connection.php"/>
+
+                <!-- Fixing this issue requires an API change -->
+                <file name="lib/Doctrine/DBAL/Driver/OCI8/OCI8Connection.php"/>
+            </errorLevel>
+        </ImplementedReturnTypeMismatch>
         <MethodSignatureMismatch>
             <errorLevel type="suppress">
                 <!--
@@ -68,6 +102,23 @@
                 <file name="lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php"/>
             </errorLevel>
         </TooFewArguments>
+        <TooManyArguments>
+            <errorLevel type="suppress">
+                <!-- See https://github.com/doctrine/dbal/pull/3080 -->
+                <file name="lib/Doctrine/DBAL/Connection.php"/>
+                <file name="tests/Doctrine/Tests/DBAL/Functional/Driver/PDOPgSql/DriverTest.php"/>
+
+                <!-- See https://github.com/doctrine/dbal/pull/3562 -->
+                <file name="lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php"/>
+                <file name="lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php"/>
+            </errorLevel>
+        </TooManyArguments>
+        <TypeDoesNotContainType>
+            <errorLevel type="suppress">
+                <!-- See https://github.com/vimeo/psalm/issues/4274 -->
+                <file name="lib/Doctrine/DBAL/Schema/Index.php"/>
+            </errorLevel>
+        </TypeDoesNotContainType>
         <UndefinedConstant>
             <errorLevel type="suppress">
                 <directory name="lib/Doctrine/DBAL/Driver/SQLAnywhere"/>
@@ -95,6 +146,34 @@
                 <file name="lib/Doctrine/DBAL/Driver/Mysqli/MysqliConnection.php"/>
             </errorLevel>
         </InvalidReturnType>
+        <InvalidScalarArgument>
+            <errorLevel type="suppress">
+                <!-- See https://github.com/doctrine/dbal/pull/3498 -->
+                <file name="tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php"/>
+                <file name="tests/Doctrine/Tests/DBAL/Platforms/AbstractPlatformTestCase.php"/>
+                <file name="tests/Doctrine/Tests/DBAL/Platforms/DB2PlatformTest.php"/>
+                <file name="tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php"/>
+                <file name="tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php"/>
+                <file name="tests/Doctrine/Tests/DBAL/Platforms/SqlitePlatformTest.php"/>
+
+                <!-- See https://github.com/doctrine/dbal/pull/3574 -->
+                <file name="tests/Doctrine/Tests/DBAL/Query/Expression/ExpressionBuilderTest.php"/>
+
+                <!-- See https://github.com/doctrine/dbal/pull/4007 -->
+                <file name="lib/Doctrine/DBAL/Driver/PDOStatement.php"/>
+
+                <!-- See https://github.com/doctrine/dbal/pull/3906 -->
+                <file name="tests/Doctrine/Tests/DBAL/Sharding/PoolingShardManagerTest.php"/>
+                <file name="tests/Doctrine/Tests/DBAL/Sharding/SQLAzure/FunctionalTest.php"/>
+                <file name="tests/Doctrine/Tests/DBAL/Sharding/SQLAzure/SQLAzureShardManagerTest.php"/>
+
+                <!-- See https://github.com/doctrine/dbal/pull/3241 -->
+                <file name="lib/Doctrine/DBAL/Tools/Console/Command/ImportCommand.php"/>
+
+                <!-- See https://github.com/doctrine/dbal/issues/4318 -->
+                <file name="lib/Doctrine/DBAL/Types/ConversionException.php"/>
+            </errorLevel>
+        </InvalidScalarArgument>
         <InvalidReturnStatement>
             <errorLevel type="suppress">
                 <!-- lastInsertId has a return type that does not match the one defined in the interface-->

diff --git a/tests/Doctrine/Tests/DBAL/ConfigurationTest.php b/tests/Doctrine/Tests/DBAL/ConfigurationTest.php
@@ -38,9 +38,5 @@ public function testSetsDefaultConnectionAutoCommitMode(): void
         $this->config->setAutoCommit(false);
 
         self::assertFalse($this->config->getAutoCommit());
-
-        $this->config->setAutoCommit(0);
-
-        self::assertFalse($this->config->getAutoCommit());
     }
 }
diff --git a/tests/Doctrine/Tests/DBAL/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/ConnectionTest.php
@@ -241,8 +241,6 @@ public function testSetAutoCommit(): void
     {
         $this->connection->setAutoCommit(false);
         self::assertFalse($this->connection->isAutoCommit());
-        $this->connection->setAutoCommit(0);
-        self::assertFalse($this->connection->isAutoCommit());
     }
 
     public function testConnectStartsTransactionInNoAutoCommitMode(): void

diff --git a/tests/Doctrine/Tests/DBAL/Functional/Driver/Mysqli/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/Driver/Mysqli/ConnectionTest.php
@@ -32,8 +32,8 @@ public function testDriverOptions(): void
     {
         $driverOptions = [MYSQLI_OPT_CONNECT_TIMEOUT => 1];
 
-        $connection = $this->getConnection($driverOptions);
-        self::assertInstanceOf(Connection::class, $connection);
+        $this->getConnection($driverOptions);
+        $this->expectNotToPerformAssertions();
     }
 
     public function testUnsupportedDriverOption(): void

diff --git a/tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php b/tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php
@@ -232,7 +232,7 @@ public function testFetchAllColumn(): void
         $query = $this->connection->getDatabasePlatform()
             ->getDummySelectSQL('1');
 
-        $qcp = new QueryCacheProfile(0, 0, new ArrayCache());
+        $qcp = new QueryCacheProfile(0, null, new ArrayCache());
 
         $stmt = $this->connection->executeCacheQuery($query, [], [], $qcp);
         $stmt->fetchAll(FetchMode::COLUMN);
@@ -243,7 +243,7 @@ public function testFetchAllColumn(): void
     }
 
     /**
-     * @param array<int, array<int, int|string>>|list<int> $expectedResult
+     * @param list<mixed> $expectedResult
      */
     private function assertCacheNonCacheSelectSameFetchModeAreEqual(array $expectedResult, int $fetchMode): void
     {

diff --git a/tests/Doctrine/Tests/DBAL/Functional/Schema/OracleSchemaManagerTest.php b/tests/Doctrine/Tests/DBAL/Functional/Schema/OracleSchemaManagerTest.php
@@ -43,7 +43,7 @@ public function testRenameTable(): void
 
         $tables = $this->schemaManager->listTables();
 
-        self::assertHasTable($tables, 'list_tables_test_new_name');
+        self::assertHasTable($tables);
     }
 
     public function testListTableWithBinary(): void

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php
@@ -52,7 +52,7 @@ public function testValidIdentifiers(string $identifier): void
         $platform = $this->createPlatform();
         $platform->assertValidIdentifier($identifier);
 
-        $this->addToAssertionCount(1);
+        $this->expectNotToPerformAssertions();
     }
 
     /**

diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php
@@ -14,11 +14,9 @@ public function createPlatform(): AbstractPlatform
     }
 
     /**
-     * @param int|bool|null $lockMode
-     *
      * @dataProvider getLockHints
      */
-    public function testAppendsLockHint($lockMode, string $lockHint): void
+    public function testAppendsLockHint(?int $lockMode, string $lockHint): void
     {
         $fromClause     = 'FROM users';
         $expectedResult = $fromClause . $lockHint;
@@ -41,8 +39,6 @@ public static function getLockHints(): iterable
     {
         return [
             [null, ''],
-            [false, ''],
-            [true, ''],
             [LockMode::NONE, ' WITH (NOLOCK)'],
             [LockMode::OPTIMISTIC, ''],
             [LockMode::PESSIMISTIC_READ, ' WITH (HOLDLOCK, ROWLOCK)'],