-
-
Notifications
You must be signed in to change notification settings - Fork 190
BUGFIX: Support PHP 8+ named arguments in proxy constructors #3504
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
base: 9.0
Are you sure you want to change the base?
Conversation
Props to @lorenzulrich for this correct fix, which always gets the parent in relation to the method being called and not the parent of the instance at hand. Fixes: #3406
This allows classes without constructor injection to have a proper constructor signature.
This is an overhaul of how object serialization is prepared in proxies. Proxies can skip object serialization code if there is nothing to serialize, that is, if there are no entity properties, no injected, or transient properties. We were too eager prior to this patch with not using the serialization code, the checks are now way more detailed. Additionally the "Proxy" Annotation now allows to force serialization code for a class if the checks still fail to detect correctly, this should be rarely needed. This fix however broke some code in Neos that should have gotten the serialization code previously but didn't. Since the class in question is readonly, injecting a mutable property via trait resulted in PHP errors. Therefore we now use a mutable object to hold related entities for serialization purposes which is declared readonly in the proxy to avoid errors with readonly classes should they need serialization code. Other mutable properties were removed as they are not strictly needed. We should do the same refactoring for AOP as well. Proxies can use the original constructor argument signature if no constructor injection is used. Finally prototype autowiring is now a choice via setting, currently default enabled to not change behavior, in the future we should plan a breaking change to disable it and then remove the option altogether. Fixes: #3493 Related: #3212 Related: #3076
This change enables Flow proxy classes to support PHP 8+ named argument syntax by including the original constructor parameter signature in generated proxies, fixing compatibility issues with modern PHP code and reflection-based serializers. The proxy constructor now always includes parameters matching the original constructor signature, with all parameters made nullable and given null default values to maintain dependency injection flexibility. This allows both named and positional argument syntax while preserving backward compatibility with existing DI patterns. Resolves #3076
|
Note: This PR is currently based on #3494 - so merge that before this one. |
kitsunet
left a comment
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.
Just leaving this here so no one decides to spontanously merge this.
Neos is broken with this change applied, I will add some more test cases showing problems with this approach tomorrow.
This also avoids a bug with constructors accepting variadic arguments as they exist in Neos.Neos.
kitsunet
left a comment
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.
From my POV this should be a good improvement to the status quo now. Lets see what the tests say. I also ran Neos tests locally.
This change enables Flow proxy classes to support PHP 8+ named argument syntax by including the original constructor parameter signature in generated proxies, fixing compatibility issues with modern PHP code and reflection-based serializers.
The proxy constructor now always includes parameters matching the original constructor signature, with all parameters made nullable and given null default values to maintain dependency injection flexibility. This allows both named and positional argument syntax while preserving backward compatibility with existing DI patterns.
Resolves #3076