Doctrinebundle: Deprecation warnings: The type was implicitly marked as commented

[fmonts]

After a Symfony upgrade from 4.2.8 to 4.2.9 (and doctrine-bundle from 1.10 to 1.11), I started to get deprecation warnings like the following:

User Deprecated: The type "tinyint" 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 "App\Utils\Tinyint::requiresSQLCommentHint()."

But I don't see any documentation about this change in any changelog... Or any info about what the "commented" attribute does, for example here: https://symfony.com/doc/current/bundles/DoctrineBundle/configuration.html

This is my Tinyint class:

<?php

namespace App\Utils;

use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types\Type;

/**
 * Adds tinyint column support in Doctrine entities
 */
class Tinyint extends Type
{
    public function getName()
    {
        return 'tinyint';
    }

    public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $platform)
    {
        return 'TINYINT' . (empty($fieldDeclaration['length']) ? '' : " ({$fieldDeclaration['length']})") . (empty($fieldDeclaration['unsigned']) ? '' : ' UNSIGNED');
    }

    public function convertToPHPValue($value, AbstractPlatform $platform)
    {
        return ($value === null) ? null : intval($value);
    }

    public function convertToDatabaseValue($value, AbstractPlatform $platform)
    {
        return ($value === null) ? null : intval($value);
    }
}

In doctrine.yaml I have:

        types:
            tinyint: App\Utils\Tinyint
            [...]
        mapping_types:
            tinyint: tinyint

Where can I understand what is this "commented" attribute? (and what is the difference between true and false, because the warning says to set it to false, but the examples set it to true)

[alcaeus]

So when you add a type, the configuration actually consists of two keys: class and commented:

types:
     tinyint:
        class: App\Utils\Tinyint
        commented: true

Since commented is true by default (thus the "implicitly" in the deprecation message) and thanks to some symfony/config magic, you can shorten it to the example above.

However, types should no longer be marked as commented by the configuration. Instead, the type itself should know whether it needs to be commented, as it depends on the database platform. For example, a database with a native tinyint type might not need the type commented.

To fix the deprecation message, you have to override the requiresSQLCommentHint method that your type class inherits. If you want to replicate default behaviour, just return true:

class Tinyint
{
    // ...

    public function requiresSQLCommentHint(AbstractPlatform $platform) : bool
    {
        return true;
    }
}

After this, the deprecation message will change to let you know that you still need of type commenting in the config as the functionality will be dropped in the configuration. That's why it tells you to change commented to false. Your configuration should look like this afterwards:

        types:
            tinyint:
                class: App\Utils\Tinyint
                commented: false

After that there should not be any more deprecation messages about this functionality.

Be aware that the commented: false itself will drop a deprecation message in 2.0 and will have to be dropped then. However, in order to keep the entire deprecation backwards compatible we need to do this in two steps.

All of this said, I'll keep this issue open and re-check the documentation next week to make sure we're explaining the deprecation better than currently.

[fmonts]

Thank you for your answer.

I use MySQL, so Tinyint is a native type.
So I should use requiresSQLCommentHint true but commented: false? Why this contradiction?

[fmonts]

And what about non-native types like uuid_binary: Ramsey\Uuid\Doctrine\UuidBinaryType?
For this I don't get the deprecation warning, should I add commented: true anyway?

Edit: no, because if I do so I get

User Deprecated: The type "uuid" 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.

[alcaeus]

commented was used to mark a type as commented via configuration, as opposed to properly marking it via the type itself. commented: true and returning true in requiresSQLCommentHint have the same result.

I need to make a small correction to what I said before: using commented: false at this stage is incorrect and will produce yet another deprecation warning. All you need to do is update the requiresSQLCommentHint method of your type and drop any reference to commented in your configuration to get rid of the warnings.

As for Ramsey\Uuid\Doctrine\UuidBinaryType, this doesn't throw a deprecation warning because the type already overrides requiresSQLCommentHint: https://github.com/ramsey/uuid-doctrine/blob/b1c6b9a8cc6c93faf7ce45abc6988fb0dd8628de/src/UuidBinaryType.php#L120..L123. The commented: true is implicit, thus no deprecation notice will appear.

Again, to summarise:

• return the appropriate value in requiresSQLCommentHint
• if you explicitly don't want a type commented, use commented: false. Note that this will produce a deprecation warning if the type returns true in requiresSQLCommentHint - you'll have to update the type to fix this.

With version 2.0 of this bundle we're changing the default value of commented to false and will deprecate the option, at which point you can remove it. I hope this answers the remaining questions.

[TeLiXj]

@alcaeus your solution works to remove the deprecation warning, but fails when use schema update command if the comment option is set to false. In this case the command wants to change all tinyint fields without comment to... remove the unexistent comment.

[alcaeus]

@TeLiXj can you please elaborate? You have a custom type that returns true in requiresSQLCommentHint, but a schema migration command removes the comment? If so, I'd say that is a bug in the schema tool, which is part of DBAL.

All the bundle does is register your custom types with DBAL, where there was also a second mechanism to mark a type as commented. We're removing that second mechanism, the rest shouldn't change.

[TeLiXj]

Yes, it's a bug in the schema update command.
I have the same tinyint type described in this thread. If I set comment to true in yaml config works fine, but with false the command tries always to update the field to remove the comment.
This behaviour ocurrs with or without your fix.

[ostrolucky]

I don't think we should document features that were removed, it's too late for that. Instead I suggest to request DBAL to document requiresSQLCommentHint()

[alcaeus]

Correct. Reported to DBAL as https://github.com/doctrine/dbal/issues/3764. @TeLiXj please update your type to return true in requiresSQLCommentHint instead of setting it to true in the yaml configuration. If the problem still persists then, please reopen this issue. Thanks!