Deprecate marking types as commented in configuration

ConnectionFactory.php

@@ -10,15 +10,15 @@
 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
 {
     /** @var mixed[][] */
     private $typesConfig = [];
 
-    /** @var string[] */
-    private $commentedTypes = [];
-
     /** @var bool */
     private $initialized = false;
 
@@ -52,11 +52,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 +92,82 @@ 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
+    {
+        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;
             }
 
-            $this->commentedTypes[] = $type;
+            // 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;
+            }
+
+            if (! $requiresSQLCommentHint) {
+                continue;
+            }
+
+            @trigger_error(
+                sprintf(
+                    'The type "%s" was marked as uncommented in its configuration but commented in the type itself. This is deprecated and will be removed in DoctrineBundle 2.0. Please update the return value of "%s::requiresSQLCommentHint()" or remove the "commented" attribute from the type configuration.',
+                    $typeName,
+                    get_class($type)
+                ),
+                E_USER_DEPRECATED
+            );
         }
-        $this->initialized = true;
     }
 }

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()

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));