Skip to content

Conversation

@mficzel
Copy link
Member

@mficzel mficzel commented Oct 22, 2025

Handle Enums nested Attributes (actually any Objects) in attribute arguments for proxy classes.

Currently the Attributes for generated proxy classes have some limitations:

  • Class Attributes: attribute values are exported via var_export which creates invalid code for attributes containing Enums or other Attributes as arguments
  • Method Attributes: attributes of type Object will yield an UnsupportedAttribute Exception

This change addresses that by:

  • creating Enums in generated code
  • creating Objects via new () by inferring the constructor arguments from properties and getters
  • handling this recursively in arrays

!!! This on itself does not add attribute support for persistence. See #3508 for that !!!

resolves: #3511
relates: #3075

Upgrade instructions

No changes are needed only cases that previously yielded errors should work now.

Review instructions

The inference of constructor arguments from values obtained via ObjectAccess is close but not perfect. However it works in the cases we want to support like Doctrine Table Attributes and probably most other cases aswell.

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@github-actions github-actions bot added the 8.4 label Oct 22, 2025
@mficzel mficzel force-pushed the bugfix/nestedAttributeSupportForProxyClasses branch 2 times, most recently from 572c495 to 6feb72b Compare October 23, 2025 09:24
@mficzel mficzel force-pushed the bugfix/nestedAttributeSupportForProxyClasses branch 2 times, most recently from 42949ba to a37bea6 Compare October 23, 2025 09:42
The code generated by var_export for objects (and enums) is not valid to be used in attributes as those require a constant expression.

This change addresses that by:

- rendering Enums correctly
- rendering other objects by filling constructor arguments with public property or getter values
- handling array arguments by calling the above recursively
@mficzel mficzel force-pushed the bugfix/nestedAttributeSupportForProxyClasses branch 4 times, most recently from 045e04f to 4251d77 Compare October 23, 2025 10:14
@mficzel mficzel changed the title WIP BUGFIX: Nested attribute support for proxy classes BUGFIX: Handle enums and attributes (Objects) in attribute arguments for proxy classes Oct 23, 2025
@github-actions github-actions bot added the Bug label Oct 23, 2025
This is done by using the method Compiler::renderAttribute instead of a local implementation
@mficzel mficzel force-pushed the bugfix/nestedAttributeSupportForProxyClasses branch from 4251d77 to 34c8208 Compare October 23, 2025 10:17
@mficzel mficzel marked this pull request as ready for review October 23, 2025 11:01
Copy link
Member

@kdambekalns kdambekalns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 by 👀

@mficzel
Copy link
Member Author

mficzel commented Oct 23, 2025

Please Note: This change reverts a change from the proxyMethod builder that used the Compiler::renderAttribute in 8.3 but got its own methods for that in 8.4. I am not sure why as the logic is a bit tricky and it should be beneficial to only have one source of issues.

Copy link
Member

@robertlemke robertlemke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by reading, but to be honest, I can't tell without further testing, if it works as intended. The test looks good at least ;-)

@mficzel mficzel mentioned this pull request Oct 23, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants