-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Support for nesting attributes with PHP 8.1 #9241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…edConstructorArguments with AnnotationDriver.
021be30 to
fc592e1
Compare
Supported/new attributes are #[AttributeOverrides], #[AssociationOverrides], #[JoinTable] with nested joinColumns, inverseJoinColumns.
…o #[Table] on PHP 8.1
fc592e1 to
087ef2b
Compare
|
Is this worth documenting anywhere? |
|
That would be https://github.com/doctrine/orm/blob/2.11.x/docs/en/reference/attributes-reference.rst is this stays the target branch. |
|
I must admit, I like the flattened version of the attribute mapping that has been built for PHP 8.0. Why do we want to nest attributes again? 🤔 |
|
@derrabus you mean for |
|
@greg0ire added docs now |
| self::assertEquals(255, $valueField['length']); | ||
| } | ||
|
|
||
| public function testCompositeKeyForJoinTableInManyToManyCreation(): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unrelated, but made tests fail due to PHP 8,0 and no nesting. Saw the original DDC3711Test.php where this came from re-executed all YamlMappingDriver tests again, so this skips about 50 tests here that were duplicated.
| */ | ||
| public function testAssociationOverridesMapping(): void | ||
| { | ||
| if (PHP_VERSION_ID >= 80000 && PHP_VERSION_ID < 80100) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kind of condition is annoying, but required for some models we interact with. This skips all assocation/attribute override tests on PHP 8, but since we also run against 7.4 and 8.1 this is not a problem.
| as part of the lifecycle of the instance variables entity-class. | ||
|
|
||
| Required attributes: | ||
| Required parameters: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This renames are unrelated, but seemed sane to do, since naming the fields attributes on Attribute classes is redundant and confusing as @greg0ire pointed out.
|
Shall we backport the documentation fixes to 2.10? |
|
@derrabus Yes I can do that. |
* 2.11.x: Enable UnusedUse sniff again (doctrine#9267) Whitelist composer plugins used by this repository (doctrine#9286) Fix XML export for `change-tracking-policy` (doctrine#9285) Allow symfony/cache 6 (doctrine#9283) Put actual value instead of index inside $originalEntityData. (doctrine#9244) Fix return types of cache interfaces (doctrine#9271) Better explain limitations of DQL "DELETE" (doctrine#9281) Fix docblocks on nullable EM properties (doctrine#9273) Docs: use canonical order for phpdoc tags, add missed semicolon (doctrine#9190) Make PrimaryReadReplicaConnection enforcement explicit (doctrine#9239) Regenerate Psalm baseline (doctrine#9272) Improve compatibility with Doctrine DBAL 4 (doctrine#9266) [docs] Fix wording for attributes=>parameters. (doctrine#9265) Support for nesting attributes with PHP 8.1 (doctrine#9241) Revert "Fix SchemaValidator with abstract child class in discriminator map (doctrine#9096)" (doctrine#9262) Docs: consistency for FQCN, spacing, etc (doctrine#9232)
This adds a new mapping case that are possible because of nesting attributes in PHP 8.1
#[AssociationOverrides]and#[AttributeOverrides]We initially thought to also add nested supported for
#[JoinTable(joinColumns: ...)]and#[Table(indexes: ...)]as is possible with annotaitons. However during documentation and test adjustment found that its getting super clumsy and complicated.Fixes #9240