-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] Run the ResolveFactoryClassPass
when lint:container
builds the container from a dump
#50988
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[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. |
fdd8a62
to
d8afb57
Compare
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
d8afb57
to
5cf4b63
Compare
null
factory class in ContainerBuilder::createService
ResolveFactoryClassPass
when lint:container
builds the container from a dump
Okay PR updated, thanks for the feedback 🙏 |
ResolveFactoryClassPass
when lint:container
builds the container from a dumpResolveFactoryClassPass
when lint:container
builds the container from a dump
Thank you @MatTheCat. |
ResolveFactoryClassPass
when lint:container
builds the container from a dumpResolveFactoryClassPass
when lint:container
builds the container from a dump
#49665 replaced the
factory
node by aconstructor
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 isnull
.As the
ResolveFactoryClassPass
will not run when thelint:container
command builds the container from an XML dump, such factories would makeContainerBuilder::createService
crash. This PR adds this compiler pass to the list.