From db2a6a0049db34917061ea13fd07fcadb4aef3da Mon Sep 17 00:00:00 2001
From: Andreas Braun <git@alcaeus.org>
Date: Fri, 5 Apr 2019 07:08:26 +0200
Subject: [PATCH] Deprecate marking types as commented in configuration

---
 ConnectionFactory.php                         | 91 ++++++++++++++++---
 DependencyInjection/Configuration.php         |  2 +-
 .../AbstractDoctrineExtensionTest.php         |  2 +-
 3 files changed, 81 insertions(+), 14 deletions(-)

diff --git a/ConnectionFactory.php b/ConnectionFactory.php
index faac74b82..7b0c46eea 100644
--- a/ConnectionFactory.php
+++ b/ConnectionFactory.php
@@ -10,6 +10,9 @@
 use Doctrine\DBAL\Exception\DriverException;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Types\Type;
+use const E_USER_DEPRECATED;
+use function get_class;
+use function trigger_error;
 
 class ConnectionFactory
 {
@@ -52,11 +55,9 @@ public function createConnection(array $params, Configuration $config = null, Ev
                 $platform->registerDoctrineTypeMapping($dbType, $doctrineType);
             }
         }
-        if (! empty($this->commentedTypes)) {
-            $platform = $this->getDatabasePlatform($connection);
-            foreach ($this->commentedTypes as $type) {
-                $platform->markDoctrineTypeCommented(Type::getType($type));
-            }
+
+        if (! empty($this->typesConfig)) {
+            $this->markTypesCommented($this->getDatabasePlatform($connection));
         }
 
         return $connection;
@@ -94,18 +95,84 @@ private function getDatabasePlatform(Connection $connection)
      */
     private function initializeTypes()
     {
-        foreach ($this->typesConfig as $type => $typeConfig) {
-            if (Type::hasType($type)) {
-                Type::overrideType($type, $typeConfig['class']);
+        foreach ($this->typesConfig as $typeName => $typeConfig) {
+            if (Type::hasType($typeName)) {
+                Type::overrideType($typeName, $typeConfig['class']);
             } else {
-                Type::addType($type, $typeConfig['class']);
+                Type::addType($typeName, $typeConfig['class']);
             }
-            if (! $typeConfig['commented']) {
+        }
+
+        $this->initialized = true;
+    }
+
+    private function markTypesCommented(AbstractPlatform $platform) : void
+    {
+        if (! $this->initialized) {
+            $this->initializeTypes();
+        }
+
+        foreach ($this->typesConfig as $typeName => $typeConfig) {
+            $type                   = Type::getType($typeName);
+            $requiresSQLCommentHint = $type->requiresSQLCommentHint($platform);
+
+            // Attribute is missing, make sure a type that doesn't require a comment is marked as commented
+            // This is deprecated behaviour that will be dropped in 2.0.
+            if ($typeConfig['commented'] === null) {
+                if (! $requiresSQLCommentHint) {
+                    @trigger_error(
+                        sprintf(
+                            'The type "%s" was implicitly marked as commented due to the configuration. This is deprecated and will be removed in DoctrineBundle 2.0. Either set the "commented" attribute in the configuration to "false" or mark the type as commented in "%s::requiresSQLCommentHint()."',
+                            $typeName,
+                            get_class($type)
+                        ),
+                        E_USER_DEPRECATED
+                    );
+
+                    $platform->markDoctrineTypeCommented($type);
+                }
+
+                continue;
+            }
+
+            // The following logic generates appropriate deprecation notices telling the user how to update their type configuration.
+            if ($typeConfig['commented']) {
+                if (! $requiresSQLCommentHint) {
+                    @trigger_error(
+                        sprintf(
+                            'The type "%s" was marked as commented in its configuration but not in the type itself. This is deprecated and will be removed in DoctrineBundle 2.0. Please update the return value of "%s::requiresSQLCommentHint()."',
+                            $typeName,
+                            get_class($type)
+                        ),
+                        E_USER_DEPRECATED
+                    );
+
+                    $platform->markDoctrineTypeCommented($type);
+
+                    continue;
+                }
+
+                @trigger_error(
+                    sprintf(
+                        'The type "%s" was explicitly marked as commented in its configuration. This is no longer necessary and will be removed in DoctrineBundle 2.0. Please remove the "commented" attribute from the type configuration.',
+                        $typeName
+                    ),
+                    E_USER_DEPRECATED
+                );
+
                 continue;
             }
 
-            $this->commentedTypes[] = $type;
+            if ($requiresSQLCommentHint) {
+                @trigger_error(
+                    sprintf(
+                        'Disabling type commenting for the commented type "%s" is deprecated and will be removed in DoctrineBundle 2.0. Please remove the "commented" attribute from the configuration and instead disable type commenting in "%s::requiresSQLCommentHint()" if you no longer want the type to be commented.',
+                        $typeName,
+                        get_class($type)
+                    ),
+                    E_USER_DEPRECATED
+                );
+            }
         }
-        $this->initialized = true;
     }
 }
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
index 0d016ee0b..b0842b774 100644
--- a/DependencyInjection/Configuration.php
+++ b/DependencyInjection/Configuration.php
@@ -94,7 +94,7 @@ private function addDbalSection(ArrayNodeDefinition $node)
                             ->end()
                             ->children()
                                 ->scalarNode('class')->isRequired()->end()
-                                ->booleanNode('commented')->defaultTrue()->end()
+                                ->booleanNode('commented')->defaultNull()->end()
                             ->end()
                         ->end()
                     ->end()
diff --git a/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php b/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php
index dae83e586..e29200036 100644
--- a/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php
+++ b/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php
@@ -522,7 +522,7 @@ public function testSetTypes()
         $container = $this->loadContainer('dbal_types');
 
         $this->assertEquals(
-            ['test' => ['class' => TestType::class, 'commented' => true]],
+            ['test' => ['class' => TestType::class, 'commented' => null]],
             $container->getParameter('doctrine.dbal.connection_factory.types')
         );
         $this->assertEquals('%doctrine.dbal.connection_factory.types%', $container->getDefinition('doctrine.dbal.connection_factory')->getArgument(0));