neos
BUGFIX: Correctly reflect builtin types in method arguments
Before builtin types like object or iterable in method arguments would lead to being reflected as if they were class types, leading to the AOP proxy builder prefixing those types with \ which results in a PHP error "Type declaration 'object' must be unqualified". Only "simple" types were handled correctly as they had a separate check.
This adds a test that verifies that builtin types like object and iterable are correctly reflected as non-class types in the ReflectionService.
Ok, the change subtly changes the way "simple types" are reflected on method parameters… A "simple type", e.g. a string was not declared "scalar" before. In combination with the AOP proxy building, this now leads to type declarations being stricter than in the original class.
The reason is we now enter this branch:
https://github.com/neos/flow-development-collection/blob/f8aec784fd283e1e9fce707ee5032b3f924f8b59/Neos.Flow/Classes/ObjectManagement/Proxy/ProxyMethod.php#L275-L276
which leads to e.g. ?string being added in this case from Session:
Fatal error: Declaration of Neos\Flow\Session\Session::destroy(?string $reason = NULL) must be compatible with Neos\Flow\Session\Session_Original::destroy($reason = NULL) in /…/Flow_Object_Classes/Neos_Flow_Session_Session.php on line 1064
A "simple type", e.g. a string was not declared "scalar" before.
🤯 I totally misread the code there
A "simple type", e.g. a string was not declared "scalar" before.
🤯 I totally misread the code there
To be fair, that scalar types are not makred as such is… counter-intuitive, at least. 🙃
Should we maybe adapt https://github.com/neos/flow-development-collection/blob/c4df7ec2520fc8cf85ae15c3570e0483d8c3782b/Neos.Flow/Classes/Reflection/ReflectionService.php#L1384-L1386 as well in this PR to keep the two places more in line? Note though that return types have self and static, which are NOT "builtin" (for whatever obscure reason, void is builtin though): https://3v4l.org/gCd1H#v8.0.17
PS: Thinking more about this, the builtin check is probably insufficient for union/intersection types https://github.com/neos/flow-development-collection/blob/21f4cd0a6a12f8fdd5d14e3ff088b6051cc592e0/Neos.Flow/Classes/Reflection/ParameterReflection.php#L60 but I guess that's more a thing for a feature PR that makes union/intersection types fully supported in reflection and proxy building
I dont understand why we have this and #2819 xD
tbh. I don't 100% know any more, but AFAIS the 6.3 PR only cares about the 'parent'/'static' cases and was supposed to be a quick fix for this common case, while this handles the more generic "isBuiltinType()" case, which could only be fixed properly in 7.0+
I think we wont need this fix for Flow 9 and even 8.3 seems to work fine?