Deprecate marking types as commented in configuration

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

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