Throw exception on unsupported mappings in Embeddable

[beberlei]

For 3.0 mapping drivers should be more strict on which annotations/attributes are not allowed on properties / classes. For example currently you can define assocations on Embeddable but they are just ignored by drivers. For 2.x we could start deprecating this behavior and then throw a mapping exception in 3.0.

[scaytrase]

ehm, @guilhermeblanco told us that in 3.0 embeddables will be implemented properly, why throw an exception there?https://github.com/doctrine/orm/issues/4291#issuecomment-326762320

[beberlei]

@scaytrase at this point it does not look like we can pull this off. Of course maybe at some point this will be contributed by someone. We restarted the 3.0 branch a few months ago

[kirya-dev]

Absolutely. Its may invented. But we need deside that design wee need. What about use user:adress where address is emeddable

[mpdude]

@beberlei Is this something that needs to go on the current list?

[beberlei]

@mpdude yes! Good catch

[mpdude]

How can we best tell which are those attributes/annotations that are not allowed?

[greg0ire]

I'd say let's do it for attributes only, let's not bother with annotations. Assuming people that implement custom mapping attributes do have them implement MappingAttribute, I'd say we should forbid attributes that implement MappingAttribute and are not used on that level (class, property) in the attribute driver.

EDIT: just noticed this is about embeddables only :thinking: So we are talking about PHP classes that are annotated with Embeddable. Those can have fields, and the fields should not have any associations defined. Is that all?

Maybe this should work with a list of attributes that are allowed, and then we throw when encountering an attribute that implements MappingAttribute, but is not part of the list. If we forget to add some attributes on the list, it's easy to add them.

Looking at the code, anything related to identifiers probably should not be on the list https://github.com/doctrine/orm/blob/a32578b7ea3581a7c55931ab302a1634d44cb66d/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php#L1194-L1196

Same goes for lifecycle callbacks, and some validation seems to already be in place there (although for better DX, we probably want something at the attribute driver level): https://github.com/doctrine/orm/blob/a32578b7ea3581a7c55931ab302a1634d44cb66d/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php#L3077-L3087

Here is a list of attributes, copied from the docs. I've marked the attributes I think should be allowed on fields of an embeddable. Which ones did I miss? What about #[AttributeOverride]? Does it make sense?:

    #[AssociationOverride]
?   #[AttributeOverride]
✅  #[Column]
    #[Cache]
    #[ChangeTrackingPolicy
    #[CustomIdGenerator]
    #[DiscriminatorColumn]
    #[DiscriminatorMap]
    #[Embeddable]
    #[Embedded]
    #[Entity]
    #[GeneratedValue]
    #[HasLifecycleCallbacks]
    #[Index]
    #[Id]
    #[InheritanceType]
    #[JoinColumn]
    #[JoinTable]
    #[ManyToOne]
    #[ManyToMany]
    #[MappedSuperclass]
    #[OneToOne]
    #[OneToMany]
    #[OrderBy]
    #[PostLoad]
    #[PostPersist]
    #[PostRemove]
    #[PostUpdate]
    #[PrePersist]
    #[PreRemove]
    #[PreUpdate]
    #[SequenceGenerator]
    #[Table]
    #[UniqueConstraint]
✅  #[Version]