[FrameworkBundle] Run the ResolveFactoryClassPass when lint:container builds the container from a dump
#50988
Conversation
|
I confirm your PR fix the issue 👍 🙏 Thanks a lot. |
|
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 ? |
|
TBH I didn’t consider updating the command because it looks like it must work without any compiler pass.
I may have missed something because I don’t understand why 🤔 |
It never did before we introduced ResolveFactoryClassPass so I don't think so. |
|
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. |
MatTheCat
force-pushed
the
null_factory_class_service
branch
3 times, most recently
from
fdd8a62 to
d8afb57
Compare
MatTheCat
requested review from
OskarStark,
chalasr,
dunglas,
lyrixx,
wouterj,
xabbuh and
yceruto
as code owners
|
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. |
…ner` builds the container from a dump
MatTheCat
force-pushed
the
null_factory_class_service
branch
from
d8afb57 to
5cf4b63
Compare
MatTheCat
changed the title
null factory class in ContainerBuilder::createServiceResolveFactoryClassPass when lint:container builds the container from a dump
|
Okay PR updated, thanks for the feedback 🙏 |
carsonbot
changed the title
ResolveFactoryClassPass when lint:container builds the container from a dumpResolveFactoryClassPass when lint:container builds the container from a dump
|
Thank you @MatTheCat. |
MatTheCat
changed the title
ResolveFactoryClassPass when lint:container builds the container from a dumpResolveFactoryClassPass when lint:container builds the container from a dump
4 participants
#49665 replaced the
factorynode by aconstructorattribute 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 isnull.As the
ResolveFactoryClassPasswill not run when thelint:containercommand builds the container from an XML dump, such factories would makeContainerBuilder::createServicecrash. This PR adds this compiler pass to the list.