Edge-case regression with 2.9.x #6750

New Issue

admin · 2026-01-22T15:38:00+01:00

admin commented

2026-01-22 15:38:00 +01:00

Originally created by @albe on GitHub (Jun 8, 2021).

We (neos/flow framework) suffer from a slight change in expectations for some annotation classes since #8266 - namely the ManyToOne, ManyToMany and Embedded now have a required first constructor argument. While those arguments were always considered "required" in a pure doctrine environment, we partially enhanced the DX by extracting some information for the annotations from other reflection data (like the targetEntity/class the annotation targets). Hence we have usage of doctrine annotations like:

@var Collection<SomeEntity>
@ORM\ManyToMany

which now breaks, because the ManyToMany is attempted to be constructed with null as targetEntity in the DocParser. Until now we solved the "missing" targetEntity (and other things) via a custom AnnotationDriver and that worked out quite nicely. With this change we are at a loss though, because we'd need to hook into the DocParser now, which happens way too early.

See https://github.com/neos/flow-development-collection/issues/2487#issuecomment-850405128

While we can "solve" this by restricting doctrine/orm to <2.9, we would also like to move towards PHP8 support and attributes. For us the best solution would be if those three constructors would allow null for their first argument, but I understand that would mean you'd need to check for that null at runtime which is a step backward. Optimally the Annotation would not be instanciated until it is used for mapping, but that would be too great a change.
Are there any other possible ways to "enhance" the annotations and hook into the instanciation process?

Originally created by @albe on GitHub (Jun 8, 2021). We (neos/flow framework) suffer from a slight change in expectations for some annotation classes since #8266 - namely the `ManyToOne`, `ManyToMany` and `Embedded` now have a required first constructor argument. While those arguments were always considered "required" in a pure doctrine environment, we partially enhanced the DX by extracting some information for the annotations from other reflection data (like the `targetEntity`/`class` the annotation targets). Hence we have usage of doctrine annotations like: ``` @var Collection<SomeEntity> @ORM\ManyToMany ``` which now breaks, because the ManyToMany is attempted to be constructed with `null` as `targetEntity` in the DocParser. Until now we solved the "missing" targetEntity (and other things) via a custom `AnnotationDriver` and that worked out quite nicely. With this change we are at a loss though, because we'd need to hook into the DocParser now, which happens way too early. See https://github.com/neos/flow-development-collection/issues/2487#issuecomment-850405128 While we can "solve" this by restricting doctrine/orm to <2.9, we would also like to move towards PHP8 support and attributes. For us the best solution would be if those three constructors would allow `null` for their first argument, but I understand that would mean you'd need to check for that `null` at runtime which is a step backward. Optimally the Annotation would not be instanciated until it is used for mapping, but that would be too great a change. Are there any other possible ways to "enhance" the annotations and hook into the instanciation process?

admin closed this issue

2026-01-22 15:38:00 +01:00

admin commented

2026-01-22 15:38:01 +01:00

@albe commented on GitHub (Jun 8, 2021):

friendly ping @beberlei @greg0ire

@albe commented on GitHub (Jun 8, 2021): friendly ping @beberlei @greg0ire

admin commented

2026-01-22 15:38:01 +01:00

@greg0ire commented on GitHub (Jun 9, 2021):

@var SomeEntity

Shouldn't that be @var ArrayCollection<int, SomeEntity>, or are you really using it that way?

Anyway, adding these constructors was technically a BC break, so it would make sense to me to allow null until next major to give you some wiggle room. A deprecation would have to be triggered when null is passed.

@greg0ire commented on GitHub (Jun 9, 2021): `@var SomeEntity` Shouldn't that be `@var ArrayCollection<int, SomeEntity>`, or are you really using it that way? Anyway, adding these constructors was technically a BC break, so it would make sense to me to allow `null` until next major to give you some wiggle room. A deprecation would have to be triggered when `null` is passed.

admin commented

2026-01-22 15:38:02 +01:00

@beberlei commented on GitHub (Jun 9, 2021):

Yes that makes sense, can you make a PR for that change? Including the deprecation about nullability if the value is passed this way to the attribute/annotation class:

Deprecation::trigger('doctrine/orm', 'https://github.com/doctrine/orm/issues/8753', 'Passing no target entity is deprecated.');

@beberlei commented on GitHub (Jun 9, 2021): Yes that makes sense, can you make a PR for that change? Including the deprecation about nullability if the value is passed this way to the attribute/annotation class: ```php Deprecation::trigger('doctrine/orm', 'https://github.com/doctrine/orm/issues/8753', 'Passing no target entity is deprecated.'); ```

admin commented

2026-01-22 15:38:02 +01:00

@greg0ire commented on GitHub (Jun 9, 2021):

I can give it a try, yes

@greg0ire commented on GitHub (Jun 9, 2021): I can give it a try, yes

admin commented

2026-01-22 15:38:02 +01:00

@greg0ire commented on GitHub (Jun 9, 2021):

Wait, this has already been fixed in a2230485b2, hasn't it? The only thing that seems to be missing are deprecations…

@greg0ire commented on GitHub (Jun 9, 2021): Wait, this has already been fixed in https://github.com/doctrine/orm/commit/a2230485b2f54b6046f7e1f736d15be18cfef2fa, hasn't it? The only thing that seems to be missing are deprecations…

admin commented

2026-01-22 15:38:02 +01:00

@albe commented on GitHub (Jun 9, 2021):

Shouldn't that be @var ArrayCollection<int, SomeEntity>, or are you really using it that way?

Yes, indeed. That was just an error in the manually typed example code :)

Wait, this has already been fixed in a223048

👀 That does indeed seem to fix it for ManyToOne annotation, but I can't see the same change for ManyToMany (https://github.com/doctrine/orm/blob/2.9.x/lib/Doctrine/ORM/Mapping/ManyToMany.php#L64)? Embedded seems to be changed already in https://github.com/doctrine/orm/pull/8724

@albe commented on GitHub (Jun 9, 2021): > Shouldn't that be @var ArrayCollection<int, SomeEntity>, or are you really using it that way? Yes, indeed. That was just an error in the manually typed example code :) > Wait, this has already been fixed in a223048 👀 That does indeed seem to fix it for ManyToOne annotation, but I can't see the same change for ManyToMany (https://github.com/doctrine/orm/blob/2.9.x/lib/Doctrine/ORM/Mapping/ManyToMany.php#L64)? Embedded seems to be changed already in https://github.com/doctrine/orm/pull/8724

admin commented

2026-01-22 15:38:03 +01:00

@greg0ire commented on GitHub (Jun 9, 2021):

Ah indeed, ManyToMany needs to be handled as well 👍

@greg0ire commented on GitHub (Jun 9, 2021): Ah indeed, `ManyToMany` needs to be handled as well 👍

admin referenced this issue

2026-01-22 16:06:30 +01:00

[PR #6750] use EntityManagerInterface instead of EntityManager #10123

Sign in to join this conversation.