Flow creates faulty proxy classes when certain interfaces are used

[ifx-cw]

Description

As desccribed in this forum thread (https://discuss.neos.io/t/flow-generates-faulty-proxy-classes/5592/8) Flow oviously is not capable of creating working proxy classes for classes that implement interfaces defining constructors with parameters (like public function __construct(array $variables = []) ).

Steps to Reproduce

1. Create a fresh Flow installation
2. Kickstart a package
3. Create an interface defining a constructor as described above
4. Add the interface to the generated StandardController and create the constructor as defined by the interface

Expected behavior

An interface should have no impact on whether or not a working proxy class can be created.

Actual behavior

Flow will generate a proxy class with a constructor without parameters, thus violating the interface definition and rendering the controller (in this case) unusable.

Affected Versions

Tested in Flow 7.1.2 and Flow 5.3.0, but I presume all versions are affected

[albe]

Thanks a lot for the helpful explanation and elaborate digging into the issue! I agree there is a bug with Flow creating different signature constructors in proxies - at least since some PHP 7.x version I recall this is no longer valid (something co-/contra-variance). We need to fix this in the proxy generation and stick to the original constructor arguments.

[ifx-cw]

Thanks for your feedback. I'll try to find the time to create a fix for this (I guess it shouldn't be that complicated given that flow correctly creates signatures for all other methods) and submit a pull request.

[ifx-cw]

Sadly I was being a bit too optimistic. Duplicating the behavior from ProxyMethod to create the method signature works, but leads to followup-problems. Obviously this interferes with Flows ObjectManager - this is the error I get once constructor signatures are rendered into proxy classes: "Too few arguments to function Neos\Flow\Security\Authorization\FilterFirewall::__construct(), 0 passed in [...]/Packages/Framework/Neos.Flow/Classes/ObjectManagement/ObjectManager.php on line 539 and exactly 3 expected"

Maybe this helps someone looking into this. I'll try to continue work on this issue, but right now I have to quit.

[kdambekalns]

Regarding constructors we have some requirements, at least for singletons it is

not possible to pass arguments to the constructor of the object, as the object might be already instantiated when you call get(). If the object needs constructor arguments, these must be configured in Objects.yaml.

see https://flowframework.readthedocs.io/en/stable/TheDefinitiveGuide/PartIII/ObjectManagement.html#retrieving-singletons

And even though I cannot find it right now, I remember we have documentation (maybe a comment in code somewhere), on why a constructor is not possible under some circumstance.

Besides, (as mentioned by @bwaidelich in #2553) constructors in interfaces are bad, (not only) because a) interfaces are about behaviour, not implementation and b) what about implementing multiple interfaces (ok, that potentially leads to conflicts not only with constructors). Anyway, it's even impossible in most other languages. Nice read: https://www.alainschlesser.com/including-constructor-interface/

Maybe we should leave classes implementing interfaces with constructors alone? Log a message? Throw an error? We are opinionated anyways… 😎

[ifx-cw]

@kdambekalns Thanks for your insights on this. I totally agree with you and I think it would be perfectly fine for Flow to just "not allow" interfaces with constructors. Yet I also agree this should be handled in some way (other than Flow just not working), especially since Fluid, which might not be part of Flow, but still is a de facto default und part of the base distribution, uses an interface definition like this.

If it were just up to me (and disregarding all the efforts and side-effects that might arise from this) I would prefer throwing an exception when Flow tries to proxy a class implementing an interface like that. With a useful-enough error message (maybe even containing a link to a detailed explanation on why this happens) every developer using Flow could easily see what's happening and wouldn't have to go into the depths of proxy generation to find the cause. And (since Sebastian Kurfürst is a member of the core team) maybe that interface within Fluid could even be changed also ;-)

[bwaidelich]

I think it would be perfectly fine for Flow to just "not allow" interfaces with constructors

That was already the case, but the exception was pretty hard to decode of course. I tried to address that with #2555

And (since Sebastian Kurfürst is a member of the core team) maybe that interface within Fluid could even be changed also ;-)

Since a while Fluid is exclusively managed by the TYPO3 team so we don't have any more influence to the core than any other community member *hint :)

[albe]

Well, we do have a little bit more of weight when we create PRs or issues, but given the constructor in the interface exists since the interface was introduced in 2015 https://github.com/TYPO3/Fluid/commit/e63f0c93c1353d6b7aca3d1fc61693084d816bcc#diff-cbcfbfe49092b027e2b48ef78cba136c2e48ca566c83deec8c3a6948d35ed9d5 and focus is on Fluid 3.0 I'm not sure it's so easy to change in the current 2.x line.