Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix support for PHP 8.1 enums in embedded classes #9419

Merged
merged 1 commit into from
Jan 23, 2022
Merged

Fix support for PHP 8.1 enums in embedded classes #9419

merged 1 commit into from
Jan 23, 2022

Conversation

HypeMC
Copy link
Contributor

@HypeMC HypeMC commented Jan 23, 2022

Fixes using enumType that was added in #9304 with embedded classes.

@derrabus derrabus added the Bug label Jan 23, 2022
@derrabus derrabus added this to the 2.11.1 milestone Jan 23, 2022
@greg0ire greg0ire requested a review from beberlei January 23, 2022 22:52
@derrabus
Copy link
Member

Good catch, thank you!

@derrabus derrabus merged commit 6d5da83 into doctrine:2.11.x Jan 23, 2022
@HypeMC HypeMC deleted the enum-in-embeddable branch January 23, 2022 22:57
derrabus added a commit to derrabus/orm that referenced this pull request Jan 23, 2022
@derrabus
Merge branch '2.11.x' into 2.12.x
51c74ff 
* 2.11.x:
  Add support for PHP 8.1 enums in embedded classes (doctrine#9419)
  Added class-string typehint on $targetEntity (doctrine#9415)
  Allow DiscriminatorColumn with length=0 (doctrine#9410)
  Move UnderscoreNamingStrategyTest to correct namespace (doctrine#9414)
@beberlei
Copy link
Member

The code is wrong, getAccessibleProperty can actually be null when used with a StaticReflectionService during code generation or reverse engineering using DisconnectedClassMetadataFactory.

that is why in the case for a non declaredField there is an extra test for && $this->reflFields[$field] !== null.

derrabus added a commit that referenced this pull request Jan 23, 2022
@derrabus
Merge branch '2.11.x' into 2.12.x
eed031f 
* 2.11.x:
  Add support for PHP 8.1 enums in embedded classes (#9419)
  Added class-string typehint on $targetEntity (#9415)
  Allow DiscriminatorColumn with length=0 (#9410)
  Move UnderscoreNamingStrategyTest to correct namespace (#9414)
derrabus added a commit to derrabus/orm that referenced this pull request Jan 23, 2022
@derrabus
Merge branch '2.12.x' into 3.0.x
1d8dd53 
* 2.12.x:
  Add support for PHP 8.1 enums in embedded classes (doctrine#9419)
  Switch tests to the middleware logging system (doctrine#9418)
  Added class-string typehint on $targetEntity (doctrine#9415)
  Allow DiscriminatorColumn with length=0 (doctrine#9410)
  Move UnderscoreNamingStrategyTest to correct namespace (doctrine#9414)
@derrabus
Copy link
Member

derrabus commented Jan 23, 2022 •
edited
Loading

getAccessibleProperty can actually be null

But wouldn't the previous logic also fail in that case? The return value of getAccessibleProperty() was previously passed directly to the constructor of ReflectionEmbeddedProperty which does not accept null.

@beberlei
Copy link
Member

Hm indeed, its likely embedded and entity generator have never been used in combination or the bug is somewhere deep in the list of 1k tickets ;)

Since this gets all burned in a fire for 3.0 i am fine keeping it this way

@github-actions github-actions bot mentioned this pull request Jan 30, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus approved these changes

@beberlei beberlei Awaiting requested review from beberlei

Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
2.11.1
Development

Successfully merging this pull request may close these issues.
3 participants
@HypeMC @derrabus @beberlei