[DI] Fix FactoryReturnTypePassTest on PHP 5

[przemyslaw-bogusz]

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR

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

	public function process(ContainerBuilder $container)
	{
	// works only since php 7.0 and hhvm 3.11
	if (!method_exists(\ReflectionMethod::class, 'getReturnType')) {
	return;
	}

I modified FactoryReturnTypePassTest accordingly.

[xabbuh]

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.

[przemyslaw-bogusz]

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:

symfony/src/Symfony/Component/DependencyInjection/Tests/Compiler/FactoryReturnTypePassTest.php

Line 34 in aca3d2c

$factory->setFactory([FactoryDummy::class, 'createFactory']);

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 ...

[nicolas-grekas]

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?

[przemyslaw-bogusz]

You mean that #29944 is forcing FactoryReturnTypePassTest to be loaded, or forcing the test to load FactoryDummy?

[nicolas-grekas]

Closing as if there is anything to change here (right now I don't think so), it should be done in #29944