Fix return type to generic in ReflectionBasedAbstractFactory

[snapshotpl]

Q	A
Documentation	no
Bugfix	yes
BC Break	no
New Feature	no
RFC	no
QA	yes

Description

ReflectionBasedAbstractFactory has invalid return type when invoking. This PR adds also generics to return type, because it always create class directly from $requestedName

[snapshotpl]

Same generic can be applied to InvokableFactory and ConfigAbstractFactory. Do it in this or separate PR?

[snapshotpl]

I have a case where I use ReflectionBasedAbstractFactory in my factory to create new instance. Typically I use shared instance across system, but in this case I need new one. Using Factory reduce boilerplate code.

$container->get(ReflectionBasedAbstractFactory::class)($container, MyClass::class);

[boesing]

Why not registering MyClass properly to your config and use $container->get(MyClass::class) instead? That is actually how its meant to be used.

return [
    'factories' => [
         MyClass::class => ReflectionBasedAbstractFactory::class,
    ],
];

That would also enable v4 to actually AoT generate a real factory. With your example, thats not possible.

So yes, your code works, but that will always use reflection at runtime to determine what services need to be injected into MyClass::__construct. That can be improved for production code by creating AoT factories in CI/CD which will basically replace ReflectionBasedAbstractFactory with an actual (generated) factory instead. That will most probably improve production code, especially if you have a bunch of those reflection related factories.

[snapshotpl]

I have already registered this service (and share it in multiple places), but I need - in some case - new instance.

I can create new factory and pass by hand, I know, but now for me development time is more important than performance.

[froschdesign]

@snapshotpl

…but I need - in some case - new instance.

The build method is not an option here?

[boesing]

Yeah, I would also use build method instead of the current implementation but whatever floats your boat. This is still a good PR, thats why I approved it in the first place. No need to discuss this more than necessary. There are other ways to achieve your actual goal but I am still +1 on this.

Only problem with the build method is that it does only work if one asserts that the container is ServiceManager or ServiceLocatorInterface, so it might not be the best solution to use build here as it also requires an assertion.

[snapshotpl]

I forgot about build! That should works!