Improve EntityColumnRule for enums

[VincentLanglet]

Closes #677

[ondrejmirtes]

Thank you!

[whataboutpereira]

    #[ORM\Column(type: Types::ENUM)]
    private CommentType $commentType;

    #[ORM\Column(type: Types::ENUM, enumType: CommentType::class)]
    private CommentType $commentType;

Now started triggering:

  Line   src\Entity\CommentNotification.php
 ------ -------------------------------------------------------------------------------------------   
  :19    Property App\Entity\CommentNotification::$commentType type mapping mismatch: backing type    
         string of enum App\Enum\CommentType does not match database type string.
         🪪  doctrine.enumType

Possibly because native enum is used in the database?

[ondrejmirtes]

Yeah, should be fixed. Please keep the previous version until then.

[whataboutpereira]

These are probably related:

        #[ORM\Column(type: Types::ENUM, options: ['values' => ['added', 'deleted', 'modified']])]
        private readonly string $action,

  :27    Property App\Entity\InventoryLog::$action type mapping mismatch: property can contain        
         string but database expects 'added'|'deleted'|'modified'.
         🪪  doctrine.columnType

Here the database field is an actual string.

[VincentLanglet]

Yes indeed.

I wont be able to make a fix before tuesday or wednesday but I think the fix is easy if you wanna try @whataboutpereira

The new code should be in a "elseif" after the if "enumTypeString !== null"

(And Im surprised there wasnt test for your situation)

[VincentLanglet]

These are probably related:

        #[ORM\Column(type: Types::ENUM, options: ['values' => ['added', 'deleted', 'modified']])]

        private readonly string $action,

  :27    Property App\Entity\InventoryLog::$action type mapping mismatch: property can contain        

         string but database expects 'added'|'deleted'|'modified'.

         🪪  doctrine.columnType

Here the database field is an actual string.

The database field is a string yes but I think for some driver Doctrine add an extra checks on those string values, for instance Mysql.

Also if you have listed values in your type, it's not a bad thing to ensure you are not accepting every possible string but only the listed values (or you're loosing the benefit of the enum type.
So for this error I dunno if @ondrejmirtes prefer:

• to not change anything since it's kinda a valid error
• to update my PR in order to just update the writableToPropertyType and let the writableToDatabaseType considered as string in order to have less impact with this PR

[whataboutpereira]

The database field is a string yes but I think for some driver Doctrine add an extra checks on those string values, for instance Mysql.

Also if you have listed values in your type, it's not a bad thing to ensure you are not accepting every possible string but only the listed values (or you're loosing the benefit of the enum type. So for this error I dunno if @ondrejmirtes prefer:

• to not change anything since it's kinda a valid error
• to update my PR in order to just update the writableToPropertyType and let the writableToDatabaseType considered as string in order to have less impact with this PR

It seems this is a non-issue. Adding /** @var 'added'|'deleted'|'modified' */ to the actual $action fixed these errors. I should really turn these into proper enums. :)

[whataboutpereira]

It seems this is a non-issue. Adding /** @var 'added'|'deleted'|'modified' */ to the actual $action fixed these errors. I should really turn these into proper enums. :)

Funnily if I go back to 2.0.4 with these added @var tags, I get the same errors. So it's one or the other. :)

[VincentLanglet]

It seems this is a non-issue. Adding /** @var 'added'|'deleted'|'modified' */ to the actual $action fixed these errors. I should really turn these into proper enums. :)

Funnily if I go back to 2.0.4 with these added @var tags, I get the same errors. So it's one or the other. :)

Of course, the type wasnt precise enough before ; that was the purpose of the Pr

But it introduced a regression we need to fix first so you can eithrr

• use 2.0.4 without the var type
• use 2.0.5 with the var type and ignore the new error

And wait for 2.0.6 and use the var tag.

[whataboutpereira]

• use 2.0.5 with the var type and ignore the new error

I was contemplating just that. Looked at the rule code as well, but unsure how to fix it. :) Thanks!

[VincentLanglet]

You can try #681 @whataboutpereira

[whataboutpereira]

You can try #681 @whataboutpereira

Thanks! Works as intended now. :)