diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index d049b95c7b2..48799043831 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -53,3 +53,9 @@ jobs: with: composer_require_dev: true args: --shepherd + + - name: Psalm type inference tests + uses: docker://vimeo/psalm-github-actions:4.6.4 + with: + composer_require_dev: true + args: --config=psalm-strict.xml diff --git a/composer.json b/composer.json index 65f5c3fe4bd..2b4119ed3a0 100644 --- a/composer.json +++ b/composer.json @@ -38,7 +38,7 @@ "doctrine/event-manager": "^1.0" }, "require-dev": { - "doctrine/coding-standard": "8.2.0", + "doctrine/coding-standard": "9.0.0", "jetbrains/phpstorm-stubs": "2020.2", "phpstan/phpstan": "0.12.81", "phpstan/phpstan-strict-rules": "^0.12.2", diff --git a/docs/en/reference/caching.rst b/docs/en/reference/caching.rst index 3a9ee383f55..2b14acbe27e 100644 --- a/docs/en/reference/caching.rst +++ b/docs/en/reference/caching.rst @@ -1,10 +1,22 @@ Caching ======= -A ``Doctrine\DBAL\Statement`` can automatically cache result sets. +A ``Doctrine\DBAL\Statement`` can automatically cache result sets. The +feature is optional though, and by default, no result set is cached. -For this to work an instance of ``Doctrine\Common\Cache\Cache`` must be provided. -This can be set on the configuration object (optionally it can also be passed at query time): +To use the result cache, there are three mandatory steps: + +1. Configure a global result cache, or provide one at query time. +2. Provide a cache profile for the result set you want to cache when + making a query. +3. Read the entire result set from the database. + +Configuring the result cache +---------------------------- + +Any instance of ``Doctrine\Common\Cache\Cache`` can be used as a result +cache and can be set on the configuration object (optionally it can also +be passed at query time): :: @@ -13,10 +25,14 @@ This can be set on the configuration object (optionally it can also be passed at $config = $conn->getConfiguration(); $config->setResultCacheImpl($cache); -To get the result set of a query cached it is necessary to pass a -``Doctrine\DBAL\Cache\QueryCacheProfile`` instance to the ``executeQuery`` or ``executeCacheQuery`` -instance. The difference between these two methods is that the former does not -require this instance, while the later has this instance as a required parameter: +Providing a cache profile +------------------------- + +To get the result set of a query cached, it is necessary to pass a +``Doctrine\DBAL\Cache\QueryCacheProfile`` instance to the +``executeQuery()`` or ``executeCacheQuery()`` methods. The difference +between these two methods is that the former has the cache profile as an +optional argument, whereas it is required when calling the latter: :: @@ -24,9 +40,10 @@ require this instance, while the later has this instance as a required parameter $stmt = $conn->executeQuery($query, $params, $types, new QueryCacheProfile(0, "some key")); $stmt = $conn->executeCacheQuery($query, $params, $types, new QueryCacheProfile(0, "some key")); -It is also possible to pass in a ``Doctrine\Common\Cache\Cache`` instance into the -constructor of ``Doctrine\DBAL\Cache\QueryCacheProfile`` in which case it overrides -the default cache instance: +As stated before, it is also possible to pass in a +``Doctrine\Common\Cache\Cache`` instance into the constructor of +``Doctrine\DBAL\Cache\QueryCacheProfile`` in which case it overrides the +default cache instance: :: @@ -34,8 +51,14 @@ the default cache instance: $cache = new \Doctrine\Common\Cache\FilesystemCache(__DIR__); new QueryCacheProfile(0, "some key", $cache); -In order for the data to actually be cached its necessary to ensure that the entire -result set is read (the easiest way to ensure this is to use one of the ``fetchAll*()`` methods): +Reading the entire result set +----------------------------- + +Caching half a result set would cause bugs if a subsequent caller needed +more rows from that same result sets. To be able to cache the entire +result set, it must be fetched entirely from the database, and not all +APIs do that. The easiest way to ensure that is to use one of the +``fetchAll*()`` methods: :: @@ -45,4 +68,6 @@ result set is read (the easiest way to ensure this is to use one of the ``fetchA .. warning:: - When using the cache layer not all fetch modes are supported. See the code of the ``Doctrine\DBAL\Cache\CachingResult`` for details. + When using the cache layer not all fetch modes are supported. See + the code of the ``Doctrine\DBAL\Cache\CachingResult`` for + details. diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 66bc7187c58..32f917313e3 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -76,10 +76,6 @@ src/Events.php - - tests/Tools/TestAsset/* - - src/Platforms/AbstractPlatform.php @@ -122,11 +118,6 @@ tests/Functional/ResultCacheTest.php - - */lib/Doctrine/DBAL/Driver/PDOConnection.php - */lib/Doctrine/DBAL/Driver/PDOStatement.php - - src/Driver/ExceptionConverterDriver.php diff --git a/psalm-strict.xml b/psalm-strict.xml new file mode 100644 index 00000000000..70cac41ebae --- /dev/null +++ b/psalm-strict.xml @@ -0,0 +1,15 @@ + + + + + + + + + + diff --git a/psalm.xml.dist b/psalm.xml.dist index 771d687ad46..d9f949a3110 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -7,6 +7,7 @@ > + diff --git a/src/Connection.php b/src/Connection.php index f16d40efd40..295cd27b3ca 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -158,11 +158,10 @@ class Connection * @param Driver $driver The driver to use. * @param Configuration|null $config The configuration, optional. * @param EventManager|null $eventManager The event manager, optional. + * @psalm-param Params $params + * @phpstan-param array $params * * @throws Exception - * - * @phpstan-param array $params - * @psalm-param Params $params */ public function __construct( array $params, @@ -204,9 +203,8 @@ public function __construct( * @internal * * @return array - * - * @phpstan-return array * @psalm-return Params + * @phpstan-return array */ public function getParams() { diff --git a/src/Connections/PrimaryReadReplicaConnection.php b/src/Connections/PrimaryReadReplicaConnection.php index 702b947cbe6..9d38f904b9d 100644 --- a/src/Connections/PrimaryReadReplicaConnection.php +++ b/src/Connections/PrimaryReadReplicaConnection.php @@ -8,6 +8,7 @@ use Doctrine\DBAL\Driver; use Doctrine\DBAL\Driver\Connection as DriverConnection; use Doctrine\DBAL\Driver\Exception as DriverException; +use Doctrine\DBAL\DriverManager; use Doctrine\DBAL\Event\ConnectionEventArgs; use Doctrine\DBAL\Events; use Doctrine\DBAL\Exception; @@ -56,7 +57,7 @@ * * Instantiation through the DriverManager looks like: * - * @psalm-import-type Params from \Doctrine\DBAL\DriverManager + * @psalm-import-type Params from DriverManager * @example * * $conn = DriverManager::getConnection(array( @@ -95,12 +96,11 @@ class PrimaryReadReplicaConnection extends Connection * @internal The connection can be only instantiated by the driver manager. * * @param array $params + * @psalm-param Params $params + * @phpstan-param array $params * * @throws Exception * @throws InvalidArgumentException - * - * @phpstan-param array $params - * @psalm-param Params $params */ public function __construct( array $params, diff --git a/src/DriverManager.php b/src/DriverManager.php index 46cb655cf74..58a0cc57916 100644 --- a/src/DriverManager.php +++ b/src/DriverManager.php @@ -145,12 +145,36 @@ private function __construct() * @param array $params * @param Configuration|null $config The configuration to use. * @param EventManager|null $eventManager The event manager to use. + * @psalm-param array{ + * charset?: string, + * dbname?: string, + * default_dbname?: string, + * driver?: key-of, + * driverClass?: class-string, + * driverOptions?: array, + * host?: string, + * keepSlave?: bool, + * keepReplica?: bool, + * master?: OverrideParams, + * memory?: bool, + * password?: string, + * path?: string, + * pdo?: \PDO, + * platform?: Platforms\AbstractPlatform, + * port?: int, + * primary?: OverrideParams, + * replica?: array, + * sharding?: array, + * slaves?: array, + * user?: string, + * wrapperClass?: class-string, + * } $params + * @phpstan-param array $params + * + * @psalm-return ($params is array{wrapperClass:mixed} ? T : Connection) * * @throws Exception * - * @phpstan-param array $params - * @psalm-param Params $params - * @psalm-return ($params is array{wrapperClass:mixed} ? T : Connection) * @template T of Connection */ public static function getConnection( @@ -211,11 +235,10 @@ public static function getAvailableDrivers(): array /** * @param array $params + * @psalm-param Params $params + * @phpstan-param array $params * * @throws Exception - * - * @phpstan-param array $params - * @psalm-param Params $params */ private static function createDriver(array $params): Driver { @@ -258,16 +281,15 @@ private static function normalizeDatabaseUrlPath(string $urlPath): string * updated list of parameters. * * @param mixed[] $params The list of parameters. + * @psalm-param Params $params + * @phpstan-param array $params * * @return mixed[] A modified list of parameters with info from a database * URL extracted into indidivual parameter parts. + * @psalm-return Params + * @phpstan-return array * * @throws Exception - * - * @phpstan-param array $params - * @phpstan-return array - * @psalm-param Params $params - * @psalm-return Params */ private static function parseDatabaseUrl(array $params): array { diff --git a/src/Platforms/AbstractPlatform.php b/src/Platforms/AbstractPlatform.php index b9147c1e98d..2060fbb74ad 100644 --- a/src/Platforms/AbstractPlatform.php +++ b/src/Platforms/AbstractPlatform.php @@ -3529,10 +3529,9 @@ protected function createReservedKeywordsList(): KeywordList * @deprecated Implement {@link createReservedKeywordsList()} instead. * * @return string + * @psalm-return class-string * * @throws Exception If not supported on this platform. - * - * @psalm-return class-string */ protected function getReservedKeywordsClass() { diff --git a/src/Platforms/SQLServer2012Platform.php b/src/Platforms/SQLServer2012Platform.php index 3d83719bfd8..c7e07655dcc 100644 --- a/src/Platforms/SQLServer2012Platform.php +++ b/src/Platforms/SQLServer2012Platform.php @@ -1584,7 +1584,7 @@ protected function getReservedKeywordsClass() */ public function quoteSingleIdentifier($str) { - return '[' . str_replace(']', '][', $str) . ']'; + return '[' . str_replace(']', ']]', $str) . ']'; } /** diff --git a/static-analysis/driver-manager-retrieves-correct-connection-type.php b/static-analysis/driver-manager-retrieves-correct-connection-type.php new file mode 100644 index 00000000000..566db2474dc --- /dev/null +++ b/static-analysis/driver-manager-retrieves-correct-connection-type.php @@ -0,0 +1,19 @@ + MyConnection::class, + ]); +} diff --git a/tests/DriverManagerTest.php b/tests/DriverManagerTest.php index f02b38e4452..cc7f4719e29 100644 --- a/tests/DriverManagerTest.php +++ b/tests/DriverManagerTest.php @@ -13,6 +13,8 @@ use PHPUnit\Framework\TestCase; use stdClass; +use function array_intersect_key; +use function array_merge; use function get_class; use function in_array; use function is_array; @@ -125,18 +127,26 @@ public function testDatabaseUrlPrimaryReplica(): void 'password' => 'bar', 'host' => 'localhost', 'port' => 11211, + 'dbname' => 'baz', + 'driver' => 'pdo_mysql', + 'url' => 'mysql://foo:bar@localhost:11211/baz', ]; - foreach ($expected as $key => $value) { - self::assertArrayHasKey($key, $params['primary']); - self::assertEquals($value, $params['primary'][$key]); - - self::assertArrayHasKey($key, $params['replica']['replica1']); - self::assertEquals($value, $params['replica']['replica1'][$key]); - } - - self::assertEquals('baz', $params['primary']['dbname']); - self::assertEquals('baz_replica', $params['replica']['replica1']['dbname']); + self::assertEquals( + [ + 'primary' => $expected, + 'replica' => [ + 'replica1' => array_merge( + $expected, + [ + 'dbname' => 'baz_replica', + 'url' => 'mysql://foo:bar@localhost:11211/baz_replica', + ] + ), + ], + ], + array_intersect_key($params, ['primary' => null, 'replica' => null]) + ); } /** diff --git a/tests/Functional/ExceptionTest.php b/tests/Functional/ExceptionTest.php index 7da2744d431..aeffa237ad6 100644 --- a/tests/Functional/ExceptionTest.php +++ b/tests/Functional/ExceptionTest.php @@ -353,10 +353,9 @@ public function testConnectionExceptionSqLite(): void /** * @param array $params + * @psalm-param Params $params * * @dataProvider getConnectionParams - * - * @psalm-param Params $params */ public function testConnectionException(array $params): void { diff --git a/tests/Functional/Platform/QuotingTest.php b/tests/Functional/Platform/QuotingTest.php index fc915d998a8..b5de6062eed 100644 --- a/tests/Functional/Platform/QuotingTest.php +++ b/tests/Functional/Platform/QuotingTest.php @@ -2,8 +2,11 @@ namespace Doctrine\DBAL\Tests\Functional\Platform; +use Doctrine\DBAL\Platforms\OraclePlatform; use Doctrine\DBAL\Tests\FunctionalTestCase; +use function key; + class QuotingTest extends FunctionalTestCase { /** @@ -29,4 +32,41 @@ public static function stringLiteralProvider(): iterable 'single-quote' => ["'"], ]; } + + /** + * @dataProvider identifierProvider + */ + public function testQuoteIdentifier(string $identifier): void + { + $platform = $this->connection->getDatabasePlatform(); + + /** + * @link https://docs.oracle.com/cd/B19306_01/server.102/b14200/sql_elements008.htm + */ + if ($platform instanceof OraclePlatform && $identifier === '"') { + self::markTestSkipped('Oracle does not support double quotes in identifiers'); + } + + $query = $platform->getDummySelectSQL( + 'NULL AS ' . $platform->quoteIdentifier($identifier) + ); + + $row = $this->connection->fetchAssociative($query); + + self::assertNotFalse($row); + self::assertSame($identifier, key($row)); + } + + /** + * @return iterable + */ + public static function identifierProvider(): iterable + { + return [ + '[' => ['['], + ']' => [']'], + '"' => ['"'], + '`' => ['`'], + ]; + } } diff --git a/tests/Functional/PrimaryReadReplicaConnectionTest.php b/tests/Functional/PrimaryReadReplicaConnectionTest.php index 7c3915b2ea9..9127c7ad97a 100644 --- a/tests/Functional/PrimaryReadReplicaConnectionTest.php +++ b/tests/Functional/PrimaryReadReplicaConnectionTest.php @@ -14,7 +14,7 @@ use const CASE_LOWER; /** - * @psalm-import-type Params from \Doctrine\DBAL\DriverManager + * @psalm-import-type Params from DriverManager */ class PrimaryReadReplicaConnectionTest extends FunctionalTestCase { @@ -54,7 +54,6 @@ private function createPrimaryReadReplicaConnection(bool $keepReplica = false): /** * @return mixed[] - * * @psalm-return Params */ private function createPrimaryReadReplicaConnectionParams(bool $keepReplica = false): array diff --git a/tests/Platforms/AbstractSQLServerPlatformTestCase.php b/tests/Platforms/AbstractSQLServerPlatformTestCase.php index ab206a3201c..b3d93b57586 100644 --- a/tests/Platforms/AbstractSQLServerPlatformTestCase.php +++ b/tests/Platforms/AbstractSQLServerPlatformTestCase.php @@ -566,14 +566,14 @@ public function testModifyLimitSubquerySimple(): void public function testQuoteIdentifier(): void { - self::assertEquals('[fo][o]', $this->platform->quoteIdentifier('fo]o')); + self::assertEquals('[fo]]o]', $this->platform->quoteIdentifier('fo]o')); self::assertEquals('[test]', $this->platform->quoteIdentifier('test')); self::assertEquals('[test].[test]', $this->platform->quoteIdentifier('test.test')); } public function testQuoteSingleIdentifier(): void { - self::assertEquals('[fo][o]', $this->platform->quoteSingleIdentifier('fo]o')); + self::assertEquals('[fo]]o]', $this->platform->quoteSingleIdentifier('fo]o')); self::assertEquals('[test]', $this->platform->quoteSingleIdentifier('test')); self::assertEquals('[test.test]', $this->platform->quoteSingleIdentifier('test.test')); } diff --git a/tests/Schema/MySQLInheritCharsetTest.php b/tests/Schema/MySQLInheritCharsetTest.php index bc0d97b7473..e7e2c9df5c1 100644 --- a/tests/Schema/MySQLInheritCharsetTest.php +++ b/tests/Schema/MySQLInheritCharsetTest.php @@ -9,6 +9,7 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Driver; use Doctrine\DBAL\Driver\Connection as DriverConnection; +use Doctrine\DBAL\DriverManager; use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Schema\Column; use Doctrine\DBAL\Schema\MySQLSchemaManager; @@ -19,7 +20,7 @@ use function array_merge; /** - * @psalm-import-type Params from \Doctrine\DBAL\DriverManager + * @psalm-import-type Params from DriverManager */ class MySQLInheritCharsetTest extends TestCase { @@ -78,10 +79,9 @@ public function testTableOptions(): void /** * @param array $params + * @psalm-param Params $params * * @return string[] - * - * @psalm-param Params $params */ private function getTableOptionsForOverride(array $params = []): array {