[DI] Fix FactoryReturnTypePassTest on PHP 5 #29949
Conversation
| @@ -44,13 +45,8 @@ public function testProcess() | |||
| $pass = new FactoryReturnTypePass(); | |||
| $pass->process($container); | |||
|
|
|||
| if (method_exists(\ReflectionMethod::class, 'getReturnType')) { | |||
There was a problem hiding this comment.
Seeing this change it looks like the test was expected to pass on older PHP versions too. And since the test doesn't fail always we should probably rather investigate why they sometimes fail and why they pass under other circumstances.
There was a problem hiding this comment.
Yes there is an if/else clause in the 3.4 branch, however a few lines above that, there is reference to FactoryDummy fixture, that causes the syntax (parse) error, because it does not have a valid PHP 5 syntax:
AppVeyor CI test also points out the same issue:
https://ci.appveyor.com/project/fabpot/symfony/builds/21760919?fullLog=true#L2048
Anyway, what can I do to help solve this issue? If there is an issue, to begin with ...
|
This did work before, and this file didn't change recently. I think this change is wrong and that there is an issue in #29944 instead: yes, the referenced factory class is PHP7-only. BUT service/factory classes shouldn't be loaded unless needed. That's what I would look for in #29944: why does it force loading the class now? |
|
You mean that #29944 is forcing |
|
Closing as if there is anything to change here (right now I don't think so), it should be done in #29944 |
One of my PRs failed a Travis CI test on PHP 5 due to a syntax error resulting from loading a class with return type declarations:
https://travis-ci.org/symfony/symfony/jobs/482202091#L2865-L2867
Since FactoryReturnTypePass works only PHP 7
symfony/src/Symfony/Component/DependencyInjection/Compiler/FactoryReturnTypePass.php
Lines 38 to 43 in aca3d2c
I modified FactoryReturnTypePassTest accordingly.