From b2d270496fab79054f5ccda8051ae2e93a282469 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 2 Oct 2020 13:32:04 -0700 Subject: [PATCH 01/10] Improve statement parameter type annotations --- lib/Doctrine/DBAL/Connection.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/DBAL/Connection.php b/lib/Doctrine/DBAL/Connection.php index af803e5db26..5eb4ae23924 100644 --- 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. */ From 37668e5f7169ca8b868092035075caf4bedfaea6 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 2 Oct 2020 13:33:27 -0700 Subject: [PATCH 02/10] The second parameter of Statement::fetchAll() can be a string --- lib/Doctrine/DBAL/Driver/ResultStatement.php | 32 ++++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/Doctrine/DBAL/Driver/ResultStatement.php b/lib/Doctrine/DBAL/Driver/ResultStatement.php index f31f960f5e0..5373ee39907 100644 --- 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[] */ From d2f92635a52b1d6d5076e3bf49311b183c68c77e Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 2 Oct 2020 13:33:51 -0700 Subject: [PATCH 03/10] Rework handling connection URL --- lib/Doctrine/DBAL/DriverManager.php | 23 ++++++++++++++--------- phpstan.neon.dist | 6 ++++++ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/Doctrine/DBAL/DriverManager.php b/lib/Doctrine/DBAL/DriverManager.php index 6c21d806580..5b12e4871bb 100644 --- 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/phpstan.neon.dist b/phpstan.neon.dist index 38c7bdace8c..59a0a7442f3 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -111,3 +111,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 From 408975dadb63522315ca796d6ddd0c7aa1d555af Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 2 Oct 2020 14:11:41 -0700 Subject: [PATCH 04/10] Make return type annotations in SQL Parser more accurate --- lib/Doctrine/DBAL/SQLParserUtils.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/DBAL/SQLParserUtils.php b/lib/Doctrine/DBAL/SQLParserUtils.php index b469741ca0e..16ed8a812c1 100644 --- 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 */ 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 */ private static function getNamedPlaceholderPositions(string $statement): array { From b858561ba4230db1d3f3299d14ebcebfd98fa950 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 2 Oct 2020 14:28:16 -0700 Subject: [PATCH 05/10] Fix the tests that violate the type system --- tests/Doctrine/Tests/DBAL/ConfigurationTest.php | 4 ---- tests/Doctrine/Tests/DBAL/ConnectionTest.php | 2 -- tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php | 2 +- .../Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php | 6 +----- tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php | 2 +- 5 files changed, 3 insertions(+), 13 deletions(-) diff --git a/tests/Doctrine/Tests/DBAL/ConfigurationTest.php b/tests/Doctrine/Tests/DBAL/ConfigurationTest.php index 9a969bc1d5c..b1d36c66d36 100644 --- 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 index 8bcf1b817fe..5543a42dec7 100644 --- 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/ResultCacheTest.php b/tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php index b1a88e8a26c..f0ad89be7e7 100644 --- 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); diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SQLServerPlatformTest.php index 88f37d27ca2..2357c1a796b 100644 --- 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)'], diff --git a/tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php b/tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php index f2a8ec2fd15..df2874a535a 100644 --- a/tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php +++ b/tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php @@ -485,7 +485,7 @@ public function testCompareSequences(): void $seq1 = new Sequence('foo', 1, 1); $seq2 = new Sequence('foo', 1, 2); $seq3 = new Sequence('foo', 2, 1); - $seq4 = new Sequence('foo', '1', '1'); + $seq4 = new Sequence('foo', 1, 1); $c = new Comparator(); From 230b95b293bbab4416bbe3ea3b9823399dbaf754 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 2 Oct 2020 14:38:14 -0700 Subject: [PATCH 06/10] Rework the tests that do not actually perform assertions --- .../Tests/DBAL/Functional/Driver/Mysqli/ConnectionTest.php | 4 ++-- tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Doctrine/Tests/DBAL/Functional/Driver/Mysqli/ConnectionTest.php b/tests/Doctrine/Tests/DBAL/Functional/Driver/Mysqli/ConnectionTest.php index a7cb0686ebf..08a356704ee 100644 --- 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/Platforms/OraclePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/OraclePlatformTest.php index 092c073551d..9cd147026b1 100644 --- 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(); } /** From ce651841a96332148dfa6568fc3335bf5bb01100 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 2 Oct 2020 14:56:59 -0700 Subject: [PATCH 07/10] Remove redundant method arguments --- .../Tests/DBAL/Functional/Schema/OracleSchemaManagerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/DBAL/Functional/Schema/OracleSchemaManagerTest.php b/tests/Doctrine/Tests/DBAL/Functional/Schema/OracleSchemaManagerTest.php index 41306d4ef04..570ec8ad63e 100644 --- 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 From 9e97067b46689358a73c4ebf63db3ca18bb7712b Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 2 Oct 2020 15:02:36 -0700 Subject: [PATCH 08/10] Loosen an unnecessarily strict type --- tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php b/tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php index f0ad89be7e7..05d3ff70cd2 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php @@ -243,7 +243,7 @@ public function testFetchAllColumn(): void } /** - * @param array>|list $expectedResult + * @param list $expectedResult */ private function assertCacheNonCacheSelectSameFetchModeAreEqual(array $expectedResult, int $fetchMode): void { From 41dc63a4a102dd3d973bb0bcec214fa7b64fe2ab Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 2 Oct 2020 18:17:50 -0700 Subject: [PATCH 09/10] Fix invalid parameter type annotations --- lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php | 4 ++-- lib/Doctrine/DBAL/Query/QueryBuilder.php | 6 +++--- phpstan.neon.dist | 3 +++ .../Tests/DBAL/Sharding/PoolingShardManagerTest.php | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php b/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php index 3c34a8767c3..d019d74576f 100644 --- 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/Query/QueryBuilder.php b/lib/Doctrine/DBAL/Query/QueryBuilder.php index a1306754836..bb8dce8187e 100644 --- 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 $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 $knownAliases * * @return string * diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 59a0a7442f3..93597bd6f52 100644 --- 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, array given\.\z~' + # legacy variadic-like signature # TODO: remove in 3.0.0 - '~^Method Doctrine\\DBAL(\\.*)?Connection::query\(\) invoked with \d+ parameters?, 0 required\.\z~' diff --git a/tests/Doctrine/Tests/DBAL/Sharding/PoolingShardManagerTest.php b/tests/Doctrine/Tests/DBAL/Sharding/PoolingShardManagerTest.php index d2ce7b7f94f..8ac09bd4e0c 100644 --- a/tests/Doctrine/Tests/DBAL/Sharding/PoolingShardManagerTest.php +++ b/tests/Doctrine/Tests/DBAL/Sharding/PoolingShardManagerTest.php @@ -11,7 +11,7 @@ class PoolingShardManagerTest extends TestCase { /** - * @return PoolingShardConnection|MockObject + * @return PoolingShardConnection&MockObject */ private function createConnectionMock(): PoolingShardConnection { From 90a459ff19a4089ec69dc99f90a328bdf37bfbab Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 2 Oct 2020 15:16:24 -0700 Subject: [PATCH 10/10] Bump Psalm level to 4 --- psalm.xml | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/psalm.xml b/psalm.xml index c813b8ce45d..f67b67c6566 100644 --- a/psalm.xml +++ b/psalm.xml @@ -1,7 +1,7 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -95,6 +146,34 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + +