Repository: doctrine/migrations (php)

Issue:
SQL generator relies on deprecated `AbstractPlatform::getName()`
### Feature Request

<!-- Fill in the relevant information below to help triage your issue. -->

|    Q        |   A
|------------ | ------
| New Feature | yes
| RFC         | no
| BC Break    | no

#### Summary

With https://github.com/doctrine/dbal/pull/4755 being released, the `SqlGenerator` class should be updated to use an alternative API with the `AbstractPlatform::getName()` method being deprecated.


Pull request:
Address or ignore deprecations

#### Summary

It should be noted that this changes the generated SQL inside migrations
so that it does not generate code that triggers deprecations.

Closes #1218 

Patch:
diff --git a/lib/Doctrine/Migrations/AbstractMigration.php b/lib/Doctrine/Migrations/AbstractMigration.php
index c06d8819..04981bbf 100644
--- a/lib/Doctrine/Migrations/AbstractMigration.php
+++ b/lib/Doctrine/Migrations/AbstractMigration.php
@@ -28,7 +28,7 @@ abstract class AbstractMigration
     /** @var Connection */
     protected $connection;
 
-    /** @var AbstractSchemaManager */
+    /** @var AbstractSchemaManager<AbstractPlatform> */
     protected $sm;
 
     /** @var AbstractPlatform */
diff --git a/lib/Doctrine/Migrations/DependencyFactory.php b/lib/Doctrine/Migrations/DependencyFactory.php
index 4d315396..fbba91f5 100644
--- a/lib/Doctrine/Migrations/DependencyFactory.php
+++ b/lib/Doctrine/Migrations/DependencyFactory.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Doctrine\Migrations;
 
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\Migrations\Configuration\Configuration;
 use Doctrine\Migrations\Configuration\Connection\ConnectionLoader;
@@ -241,6 +242,9 @@ class DependencyFactory
         });
     }
 
+    /**
+     * @return AbstractSchemaManager<AbstractPlatform>
+     */
     private function getSchemaManager(Connection $connection): AbstractSchemaManager
     {
         return method_exists($connection, 'createSchemaManager')
diff --git a/lib/Doctrine/Migrations/Generator/DiffGenerator.php b/lib/Doctrine/Migrations/Generator/DiffGenerator.php
index 409fa83e..bd5c7469 100644
--- a/lib/Doctrine/Migrations/Generator/DiffGenerator.php
+++ b/lib/Doctrine/Migrations/Generator/DiffGenerator.php
@@ -27,7 +27,7 @@ class DiffGenerator
     /** @var DBALConfiguration */
     private $dbalConfiguration;
 
-    /** @var AbstractSchemaManager */
+    /** @var AbstractSchemaManager<AbstractPlatform> */
     private $schemaManager;
 
     /** @var SchemaProvider */
@@ -45,6 +45,9 @@ class DiffGenerator
     /** @var SchemaProvider */
     private $emptySchemaProvider;
 
+    /**
+     * @param AbstractSchemaManager<AbstractPlatform> $schemaManager
+     */
     public function __construct(
         DBALConfiguration $dbalConfiguration,
         AbstractSchemaManager $schemaManager,
diff --git a/lib/Doctrine/Migrations/Generator/SqlGenerator.php b/lib/Doctrine/Migrations/Generator/SqlGenerator.php
index 3ecbda31..feb4e08e 100644
--- a/lib/Doctrine/Migrations/Generator/SqlGenerator.php
+++ b/lib/Doctrine/Migrations/Generator/SqlGenerator.php
@@ -12,6 +12,7 @@ use Doctrine\SqlFormatter\SqlFormatter;
 
 use function array_unshift;
 use function count;
+use function get_class;
 use function implode;
 use function sprintf;
 use function stripos;
@@ -68,14 +69,20 @@ class SqlGenerator
         }
 
         if (count($code) !== 0 && $checkDbPlatform && $this->configuration->isDatabasePlatformChecked()) {
-            $currentPlatform = $this->platform->getName();
+            $currentPlatform = '\\' . get_class($this->platform);
 
             array_unshift(
                 $code,
                 sprintf(
-                    '$this->abortIf($this->connection->getDatabasePlatform()->getName() !== %s, %s);',
-                    var_export($currentPlatform, true),
-                    var_export(sprintf("Migration can only be executed safely on '%s'.", $currentPlatform), true)
+                    <<<'PHP'
+$this->abortIf(
+    !$this->connection->getDatabasePlatform() instanceof %s,
+    "Migration can only be executed safely on '%s'."
+);
+PHP
+                    ,
+                    $currentPlatform,
+                    $currentPlatform
                 ),
                 ''
             );
diff --git a/lib/Doctrine/Migrations/Metadata/Storage/TableMetadataStorage.php b/lib/Doctrine/Migrations/Metadata/Storage/TableMetadataStorage.php
index 72952706..c3ba6196 100644
--- a/lib/Doctrine/Migrations/Metadata/Storage/TableMetadataStorage.php
+++ b/lib/Doctrine/Migrations/Metadata/Storage/TableMetadataStorage.php
@@ -46,7 +46,7 @@ final class TableMetadataStorage implements MetadataStorage
     /** @var Connection */
     private $connection;
 
-    /** @var AbstractSchemaManager */
+    /** @var AbstractSchemaManager<AbstractPlatform> */
     private $schemaManager;
 
     /** @var AbstractPlatform */
diff --git a/lib/Doctrine/Migrations/Provider/DBALSchemaDiffProvider.php b/lib/Doctrine/Migrations/Provider/DBALSchemaDiffProvider.php
index d6e4cc20..ccccb17f 100644
--- a/lib/Doctrine/Migrations/Provider/DBALSchemaDiffProvider.php
+++ b/lib/Doctrine/Migrations/Provider/DBALSchemaDiffProvider.php
@@ -23,9 +23,12 @@ class DBALSchemaDiffProvider implements SchemaDiffProvider
     /** @var AbstractPlatform */
     private $platform;
 
-    /** @var AbstractSchemaManager */
+    /** @var AbstractSchemaManager<AbstractPlatform> */
     private $schemaManager;
 
+    /**
+     * @param AbstractSchemaManager<AbstractPlatform> $schemaManager-
+     */
     public function __construct(AbstractSchemaManager $schemaManager, AbstractPlatform $platform)
     {
         $this->schemaManager = $schemaManager;
diff --git a/lib/Doctrine/Migrations/Provider/EmptySchemaProvider.php b/lib/Doctrine/Migrations/Provider/EmptySchemaProvider.php
index bcb3f887..7258e7be 100644
--- a/lib/Doctrine/Migrations/Provider/EmptySchemaProvider.php
+++ b/lib/Doctrine/Migrations/Provider/EmptySchemaProvider.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Doctrine\Migrations\Provider;
 
+use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\Schema\Schema;
 
@@ -15,9 +16,12 @@ use Doctrine\DBAL\Schema\Schema;
  */
 final class EmptySchemaProvider implements SchemaProvider
 {
-    /** @var AbstractSchemaManager */
+    /** @var AbstractSchemaManager<AbstractPlatform> */
     private $schemaManager;
 
+    /**
+     * @param AbstractSchemaManager<AbstractPlatform> $schemaManager
+     */
     public function __construct(AbstractSchemaManager $schemaManager)
     {
         $this->schemaManager = $schemaManager;
diff --git a/lib/Doctrine/Migrations/SchemaDumper.php b/lib/Doctrine/Migrations/SchemaDumper.php
index 6c2b659a..1ffc9b0c 100644
--- a/lib/Doctrine/Migrations/SchemaDumper.php
+++ b/lib/Doctrine/Migrations/SchemaDumper.php
@@ -40,7 +40,7 @@ class SchemaDumper
     /** @var AbstractPlatform */
     private $platform;
 
-    /** @var AbstractSchemaManager */
+    /** @var AbstractSchemaManager<AbstractPlatform> */
     private $schemaManager;
 
     /** @var Generator */
@@ -53,7 +53,8 @@ class SchemaDumper
     private $excludedTablesRegexes;
 
     /**
-     * @param string[] $excludedTablesRegexes
+     * @param AbstractSchemaManager<AbstractPlatform> $schemaManager
+     * @param string[]                                $excludedTablesRegexes
      */
     public function __construct(
         AbstractPlatform $platform,
diff --git a/phpstan-dbal-2.neon.dist b/phpstan-dbal-2.neon.dist
index ede0697c..52732c79 100644
--- a/phpstan-dbal-2.neon.dist
+++ b/phpstan-dbal-2.neon.dist
@@ -17,3 +17,19 @@ parameters:
             paths:
                 - tests/Doctrine/Migrations/Tests/Tools/Console/legacy-config-dbal/cli-config.php
                 - tests/Doctrine/Migrations/Tests/Tools/Console/legacy-config-wrong/cli-config.php
+
+        -
+            message: '~.*AbstractSchemaManager.*is not generic\.$~'
+            paths:
+                - lib/Doctrine/Migrations/SchemaDumper.php
+                - lib/Doctrine/Migrations/AbstractMigration.php
+                - lib/Doctrine/Migrations/DependencyFactory.php
+                - lib/Doctrine/Migrations/Generator/DiffGenerator.php
+                - lib/Doctrine/Migrations/Metadata/Storage/TableMetadataStorage.php
+                - lib/Doctrine/Migrations/Provider/DBALSchemaDiffProvider.php
+                - lib/Doctrine/Migrations/Provider/EmptySchemaProvider.php
+                - tests/Doctrine/Migrations/Tests/Generator/DiffGeneratorTest.php
+                - tests/Doctrine/Migrations/Tests/Metadata/Storage/ExistingTableMetadataStorageTest.php
+                - tests/Doctrine/Migrations/Tests/Metadata/Storage/TableMetadataStorageTest.php
+                - tests/Doctrine/Migrations/Tests/Provider/EmptySchemaProviderTest.php
+                - tests/Doctrine/Migrations/Tests/SchemaDumperTest.php
diff --git a/phpstan-dbal-3.neon.dist b/phpstan-dbal-3.neon.dist
index f84ec084..f94eca0c 100644
--- a/phpstan-dbal-3.neon.dist
+++ b/phpstan-dbal-3.neon.dist
@@ -23,3 +23,14 @@ parameters:
                 - tests/Doctrine/Migrations/Tests/Tools/Console/legacy-config-dbal/cli-config.php
                 - tests/Doctrine/Migrations/Tests/Tools/Console/legacy-config-wrong/cli-config.php
 
+        # We need to implement platform-aware comparison
+        -
+            message: '~Call to deprecated method getMigrate(From|ToSql)~'
+            paths:
+                - lib/Doctrine/Migrations/Generator/DiffGenerator.php
+                - lib/Doctrine/Migrations/Provider/DBALSchemaDiffProvider.php
+
+        # Switch to Logging\Connection after dropping compatibility with DBAL 2
+        -
+            message: '~deprecated.*DebugStack~'
+            path: tests/Doctrine/Migrations/Tests/Metadata/Storage/TableMetadataStorageTest.php


Test patch:
diff --git a/tests/Doctrine/Migrations/Tests/Generator/DiffGeneratorTest.php b/tests/Doctrine/Migrations/Tests/Generator/DiffGeneratorTest.php
index f485e608..8fe37ca0 100644
--- a/tests/Doctrine/Migrations/Tests/Generator/DiffGeneratorTest.php
+++ b/tests/Doctrine/Migrations/Tests/Generator/DiffGeneratorTest.php
@@ -21,7 +21,7 @@ class DiffGeneratorTest extends TestCase
     /** @var DBALConfiguration|MockObject */
     private $dbalConfiguration;
 
-    /** @var AbstractSchemaManager|MockObject */
+    /** @var AbstractSchemaManager<AbstractPlatform>|MockObject */
     private $schemaManager;
 
     /** @var SchemaProvider|MockObject */
diff --git a/tests/Doctrine/Migrations/Tests/Generator/SqlGeneratorTest.php b/tests/Doctrine/Migrations/Tests/Generator/SqlGeneratorTest.php
index 79e27c1d..618d5f3f 100644
--- a/tests/Doctrine/Migrations/Tests/Generator/SqlGeneratorTest.php
+++ b/tests/Doctrine/Migrations/Tests/Generator/SqlGeneratorTest.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Doctrine\Migrations\Tests\Generator;
 
 use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Platforms\SqlitePlatform;
 use Doctrine\Migrations\Configuration\Configuration;
 use Doctrine\Migrations\Generator\SqlGenerator;
 use Doctrine\Migrations\Metadata\Storage\TableMetadataStorageConfiguration;
@@ -34,11 +35,20 @@ final class SqlGeneratorTest extends TestCase
 
     public function testGenerate(): void
     {
-        $this->configuration->setCheckDatabasePlatform(true);
+        $configuration = new Configuration();
+        $platform      = new SqlitePlatform();
+
+        $metadataConfig = new TableMetadataStorageConfiguration();
+        $configuration->setMetadataStorageConfiguration($this->metadataConfig);
+        $migrationSqlGenerator = new SqlGenerator($configuration, $platform);
+        $configuration->setCheckDatabasePlatform(true);
 
         $expectedCode = $this->prepareGeneratedCode(
             <<<'CODE'
-$this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'mysql', 'Migration can only be executed safely on \'mysql\'.');
+$this->abortIf(
+    !$this->connection->getDatabasePlatform() instanceof \Doctrine\DBAL\Platforms\SqlitePlatform,
+    "Migration can only be executed safely on '\Doctrine\DBAL\Platforms\SqlitePlatform'."
+);
 
 $this->addSql('SELECT 1');
 $this->addSql('SELECT 2');
@@ -46,11 +56,7 @@ $this->addSql('%s');
 CODE
         );
 
-        $this->platform->expects(self::once())
-            ->method('getName')
-            ->willReturn('mysql');
-
-        $code = $this->migrationSqlGenerator->generate($this->sql, true, 80);
+        $code = $migrationSqlGenerator->generate($this->sql, true, 80);
 
         self::assertSame($expectedCode, $code);
     }
diff --git a/tests/Doctrine/Migrations/Tests/Metadata/Storage/ExistingTableMetadataStorageTest.php b/tests/Doctrine/Migrations/Tests/Metadata/Storage/ExistingTableMetadataStorageTest.php
index 442937cb..5528d9bf 100644
--- a/tests/Doctrine/Migrations/Tests/Metadata/Storage/ExistingTableMetadataStorageTest.php
+++ b/tests/Doctrine/Migrations/Tests/Metadata/Storage/ExistingTableMetadataStorageTest.php
@@ -7,6 +7,7 @@ namespace Doctrine\Migrations\Tests\Metadata\Storage;
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Connections\PrimaryReadReplicaConnection;
 use Doctrine\DBAL\DriverManager;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\Migrations\AbstractMigration;
@@ -34,7 +35,7 @@ class ExistingTableMetadataStorageTest extends TestCase
     /** @var TableMetadataStorageConfiguration */
     private $config;
 
-    /** @var AbstractSchemaManager */
+    /** @var AbstractSchemaManager<AbstractPlatform> */
     private $schemaManager;
 
     /** @var MigrationsRepository */
diff --git a/tests/Doctrine/Migrations/Tests/Metadata/Storage/TableMetadataStorageTest.php b/tests/Doctrine/Migrations/Tests/Metadata/Storage/TableMetadataStorageTest.php
index 929f8a61..d34e876c 100644
--- a/tests/Doctrine/Migrations/Tests/Metadata/Storage/TableMetadataStorageTest.php
+++ b/tests/Doctrine/Migrations/Tests/Metadata/Storage/TableMetadataStorageTest.php
@@ -11,6 +11,7 @@ use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Driver\PDO\SQLite\Driver as SQLiteDriver;
 use Doctrine\DBAL\DriverManager;
 use Doctrine\DBAL\Logging\DebugStack;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Types\DateTimeType;
@@ -42,7 +43,7 @@ class TableMetadataStorageTest extends TestCase
     /** @var TableMetadataStorageConfiguration */
     private $config;
 
-    /** @var AbstractSchemaManager */
+    /** @var AbstractSchemaManager<AbstractPlatform> */
     private $schemaManager;
 
     private function getSqliteConnection(?Configuration $configuration = null): Connection
diff --git a/tests/Doctrine/Migrations/Tests/Provider/EmptySchemaProviderTest.php b/tests/Doctrine/Migrations/Tests/Provider/EmptySchemaProviderTest.php
index 2beed275..f713e55f 100644
--- a/tests/Doctrine/Migrations/Tests/Provider/EmptySchemaProviderTest.php
+++ b/tests/Doctrine/Migrations/Tests/Provider/EmptySchemaProviderTest.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Doctrine\Migrations\Tests\Provider;
 
+use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
 use Doctrine\DBAL\Schema\SchemaConfig;
 use Doctrine\Migrations\Provider\EmptySchemaProvider;
@@ -15,7 +16,7 @@ use PHPUnit\Framework\MockObject\MockObject;
  */
 class EmptySchemaProviderTest extends MigrationTestCase
 {
-    /** @var AbstractSchemaManager|MockObject */
+    /** @var AbstractSchemaManager<AbstractPlatform>|MockObject */
     private $schemaManager;
 
     /** @var EmptySchemaProvider */
@@ -24,7 +25,6 @@ class EmptySchemaProviderTest extends MigrationTestCase
     public function testCreateSchema(): void
     {
         $schemaConfig = new SchemaConfig();
-        $schemaConfig->setExplicitForeignKeyIndexes(true);
 
         $this->schemaManager->expects(self::once())
             ->method('createSchemaConfig')
@@ -34,7 +34,6 @@ class EmptySchemaProviderTest extends MigrationTestCase
 
         self::assertSame([], $schema->getTables());
         self::assertSame([], $schema->getSequences());
-        self::assertTrue($schema->hasExplicitForeignKeyIndexes());
         self::assertSame([], $schema->getNamespaces());
     }
 
diff --git a/tests/Doctrine/Migrations/Tests/SchemaDumperTest.php b/tests/Doctrine/Migrations/Tests/SchemaDumperTest.php
index c87e2d6a..35918f16 100644
--- a/tests/Doctrine/Migrations/Tests/SchemaDumperTest.php
+++ b/tests/Doctrine/Migrations/Tests/SchemaDumperTest.php
@@ -21,7 +21,7 @@ class SchemaDumperTest extends TestCase
     /** @var AbstractPlatform|MockObject */
     private $platform;
 
-    /** @var AbstractSchemaManager|MockObject */
+    /** @var AbstractSchemaManager<AbstractPlatform>|MockObject */
     private $schemaManager;
 
     /** @var Generator|MockObject */
diff --git a/tests/Doctrine/Migrations/Tests/Tools/Console/ConsoleRunnerTest.php b/tests/Doctrine/Migrations/Tests/Tools/Console/ConsoleRunnerTest.php
index 3498dab0..1c873f99 100644
--- a/tests/Doctrine/Migrations/Tests/Tools/Console/ConsoleRunnerTest.php
+++ b/tests/Doctrine/Migrations/Tests/Tools/Console/ConsoleRunnerTest.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Doctrine\Migrations\Tests\Tools\Console;
 
+use Doctrine\DBAL\Platforms\SqlitePlatform;
 use Doctrine\DBAL\Tools\Console\Helper\ConnectionHelper;
 use Doctrine\Migrations\DependencyFactory;
 use Doctrine\Migrations\Tools\Console\ConsoleRunner;
@@ -44,7 +45,7 @@ class ConsoleRunnerTest extends TestCase
         try {
             $dependencyFactory = ConsoleRunnerStub::findDependencyFactory();
             self::assertInstanceOf(DependencyFactory::class, $dependencyFactory);
-            self::assertSame('sqlite', $dependencyFactory->getConnection()->getDatabasePlatform()->getName());
+            self::assertInstanceOf(SqlitePlatform::class, $dependencyFactory->getConnection()->getDatabasePlatform());
         } finally {
             chdir($dir);
         }
@@ -62,7 +63,7 @@ class ConsoleRunnerTest extends TestCase
         try {
             $dependencyFactory = ConsoleRunnerStub::findDependencyFactory();
             self::assertInstanceOf(DependencyFactory::class, $dependencyFactory);
-            self::assertSame('sqlite', $dependencyFactory->getConnection()->getDatabasePlatform()->getName());
+            self::assertInstanceOf(SqlitePlatform::class, $dependencyFactory->getConnection()->getDatabasePlatform());
             self::assertInstanceOf(EntityManager::class, $dependencyFactory->getEntityManager());
         } finally {
             chdir($dir);