[FrameworkBundle] Run the ResolveFactoryClassPass when lint:container builds the container from a dump

[MatTheCat]

Q	A
Branch?	6.3
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix #50622
License	MIT
Doc PR	N/A

#49665 replaced the factory node by a constructor attribute in the XML and YAML dumper when the factory’s class is the same as the definition’s. The corresponding loader then creates a definition where the factory class is null.

As the ResolveFactoryClassPass will not run when the lint:container command builds the container from an XML dump, such factories would make ContainerBuilder::createService crash. This PR adds this compiler pass to the list.

[lugosium]

I confirm your PR fix the issue 👍 🙏

Thanks a lot.

[nicolas-grekas]

Did you consider enabling the pass on the command? That'd make more sense to me because if we do this, then we should do the same in PHP dumper, and the pass becomes useless. Also, this misses the check for $class that the pass has. WDYT ?

[MatTheCat]

TBH I didn’t consider updating the command because it looks like it must work without any compiler pass.

then we should do the same in PHP dumper

I may have missed something because I don’t understand why 🤔

[nicolas-grekas]

it looks like it must work without any compiler pass.

It never did before we introduced ResolveFactoryClassPass so I don't think so.

[MatTheCat]

Is this what you’re asking for?

diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php
--- a/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php	(revision 6f2e603c176724873f92942ea7461150d0e32040)
+++ b/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php	(date 1689424425782)
@@ -20,6 +20,7 @@
 use Symfony\Component\Console\Style\SymfonyStyle;
 use Symfony\Component\DependencyInjection\Compiler\CheckTypeDeclarationsPass;
 use Symfony\Component\DependencyInjection\Compiler\PassConfig;
+use Symfony\Component\DependencyInjection\Compiler\ResolveFactoryClassPass;
 use Symfony\Component\DependencyInjection\Container;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
@@ -120,7 +121,7 @@
             }
 
             $container->getCompilerPassConfig()->setBeforeOptimizationPasses([]);
-            $container->getCompilerPassConfig()->setOptimizationPasses([]);
+            $container->getCompilerPassConfig()->setOptimizationPasses([new ResolveFactoryClassPass()]);
             $container->getCompilerPassConfig()->setBeforeRemovingPasses([]);
         }
 

It feels weird needing this particular pass to build the container from a dump.

[nicolas-grekas]

Yes that's what I mean. Dealing with th dumped XML requires special care, this makes sense to me. At least more than making the pass half-needeed. I prefer keeping the symmetry between the dumped container and a compiled container builder.

[MatTheCat]

Okay PR updated, thanks for the feedback 🙏

[nicolas-grekas]

Thank you @MatTheCat.