Skip to content

Commit bb3f5d1

Browse files
nicolas-grekasnicolas-grekas
committed
[DependencyInjection] Optimize autowiring logic by telling it about excluded symbols
1 parent c0dbb90 commit bb3f5d1

File tree

12 files changed

+112
-21
lines changed

12 files changed

+112
-21
lines changed

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class UnusedTagsPass implements CompilerPassInterface
3333
'console.command',
3434
'container.env_var_loader',
3535
'container.env_var_processor',
36+
'container.excluded',
3637
'container.hot_path',
3738
'container.no_preload',
3839
'container.preload',

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/services.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Symfony\Bundle\FrameworkBundle\CacheWarmer\ConfigBuilderCacheWarmer;
1515
use Symfony\Bundle\FrameworkBundle\HttpCache\HttpCache;
16+
use Symfony\Component\Config\Loader\LoaderInterface;
1617
use Symfony\Component\Config\Resource\SelfCheckingResourceChecker;
1718
use Symfony\Component\Config\ResourceCheckerConfigCacheFactory;
1819
use Symfony\Component\Console\ConsoleEvents;
@@ -26,7 +27,10 @@
2627
use Symfony\Component\EventDispatcher\EventDispatcherInterface as EventDispatcherInterfaceComponentAlias;
2728
use Symfony\Component\Filesystem\Filesystem;
2829
use Symfony\Component\Form\FormEvents;
30+
use Symfony\Component\HttpFoundation\Request;
2931
use Symfony\Component\HttpFoundation\RequestStack;
32+
use Symfony\Component\HttpFoundation\Response;
33+
use Symfony\Component\HttpFoundation\Session\SessionInterface;
3034
use Symfony\Component\HttpFoundation\UrlHelper;
3135
use Symfony\Component\HttpKernel\CacheClearer\ChainCacheClearer;
3236
use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerAggregate;
@@ -218,5 +222,11 @@ class_exists(WorkflowEvents::class) ? WorkflowEvents::ALIASES : []
218222
->set('config_builder.warmer', ConfigBuilderCacheWarmer::class)
219223
->args([service(KernelInterface::class), service('logger')->nullOnInvalid()])
220224
->tag('kernel.cache_warmer')
225+
226+
// register as abstract and excluded, aka not-autowirable types
227+
->set(LoaderInterface::class)->abstract()->tag('container.excluded')
228+
->set(Request::class)->abstract()->tag('container.excluded')
229+
->set(Response::class)->abstract()->tag('container.excluded')
230+
->set(SessionInterface::class)->abstract()->tag('container.excluded')
221231
;
222232
};

‎src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ protected function processValue(mixed $value, bool $isRoot = false)
7575
if (\is_array($value)) {
7676
foreach ($value as $k => $v) {
7777
if ($isRoot) {
78+
if ($v->hasTag('container.excluded')) {
79+
continue;
80+
}
7881
$this->currentId = $k;
7982
}
8083
if ($v !== $processedValue = $this->processValue($v, $isRoot)) {

‎src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,22 @@ private function createTypeNotFoundMessageCallback(TypedReference $reference, st
503503

504504
private function createTypeNotFoundMessage(TypedReference $reference, string $label, string $currentId): string
505505
{
506-
if (!$r = $this->container->getReflectionClass($type = $reference->getType(), false)) {
506+
$type = $reference->getType();
507+
508+
if ($this->container->hasDefinition($type) && $this->container->getDefinition($type)->hasTag('container.excluded')) {
509+
return sprintf('Cannot autowire service "%s": %s has type "%s" but this symbol has been excluded.', $currentId, $label, $type);
510+
}
511+
512+
$namespace = $type;
513+
while (false !== $i = strrpos($namespace, '\\')) {
514+
$namespace = substr($namespace, 0, $i);
515+
516+
if ($this->container->hasDefinition($namespace) && $this->container->getDefinition($namespace)->hasTag('container.excluded')) {
517+
return sprintf('Cannot autowire service "%s": %s has type "%s" but the "%s" namespace has been excluded.', $currentId, $label, $type, $namespace);
518+
}
519+
}
520+
521+
if (!$r = $this->container->getReflectionClass($type, false)) {
507522
// either $type does not exist or a parent class does not exist
508523
try {
509524
$resource = new ClassExistenceResource($type, false);

‎src/Symfony/Component/DependencyInjection/ContainerBuilder.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,11 @@ public function merge(self $container)
598598
throw new BadMethodCallException('Cannot merge on a compiled container.');
599599
}
600600

601-
$this->addDefinitions($container->getDefinitions());
601+
foreach ($container->getDefinitions() as $id => $definition) {
602+
if (!$definition->hasTag('container.excluded') || !$this->has($id)) {
603+
$this->setDefinition($id, $definition);
604+
}
605+
}
602606
$this->addAliases($container->getAliases());
603607
$this->getParameterBag()->add($container->getParameterBag()->all());
604608

‎src/Symfony/Component/DependencyInjection/Loader/FileLoader.php‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,6 @@ private function findClasses(string $namespace, string $pattern, array $excludeP
189189

190190
$pattern = $parameterBag->unescapeValue($parameterBag->resolveValue($pattern));
191191
$classes = [];
192-
$extRegexp = '/\\.php$/';
193192
$prefixLen = null;
194193
foreach ($this->glob($pattern, true, $resource, false, false, $excludePaths) as $path => $info) {
195194
if (null === $prefixLen) {
@@ -204,10 +203,10 @@ private function findClasses(string $namespace, string $pattern, array $excludeP
204203
continue;
205204
}
206205

207-
if (!preg_match($extRegexp, $path, $m) || !$info->isReadable()) {
206+
if (!str_ends_with($path, '.php') || !$info->isReadable()) {
208207
continue;
209208
}
210-
$class = $namespace.ltrim(str_replace('/', '\\', substr($path, $prefixLen, -\strlen($m[0]))), '\\');
209+
$class = $namespace.ltrim(str_replace('/', '\\', substr($path, $prefixLen, -4)), '\\');
211210

212211
if (!preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+)*+$/', $class)) {
213212
continue;
@@ -242,6 +241,17 @@ private function findClasses(string $namespace, string $pattern, array $excludeP
242241
}
243242
}
244243

244+
if (null !== $prefixLen) {
245+
foreach ($excludePaths as $path => $_) {
246+
$class = $namespace.ltrim(str_replace('/', '\\', substr($path, $prefixLen, str_ends_with($path, '.php') ? -4 : null)), '\\');
247+
if (!$this->container->has($class)) {
248+
$this->container->register($class)
249+
->setAbstract(true)
250+
->addTag('container.excluded');
251+
}
252+
}
253+
}
254+
245255
return $classes;
246256
}
247257
}

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,6 @@
3636

3737
require_once __DIR__.'/../Fixtures/includes/autowiring_classes.php';
3838

39-
/**
40-
* @author Kévin Dunglas <dunglas@gmail.com>
41-
*/
4239
class AutowirePassTest extends TestCase
4340
{
4441
public function testProcess()
@@ -1186,4 +1183,38 @@ public function testAsDecoratorAttribute()
11861183
$this->assertSame(AsDecoratorBaz::class.'.inner', (string) $container->getDefinition(AsDecoratorBaz::class)->getArgument(0));
11871184
$this->assertSame(2, $container->getDefinition(AsDecoratorBaz::class)->getArgument(0)->getInvalidBehavior());
11881185
}
1186+
1187+
public function testTypeSymbolExcluded()
1188+
{
1189+
$container = new ContainerBuilder();
1190+
1191+
$container->register(Foo::class)->setAbstract(true)->addTag('container.excluded');
1192+
$aDefinition = $container->register('a', NotGuessableArgument::class);
1193+
$aDefinition->setAutowired(true);
1194+
1195+
$pass = new AutowirePass();
1196+
try {
1197+
$pass->process($container);
1198+
$this->fail('AutowirePass should have thrown an exception');
1199+
} catch (AutowiringFailedException $e) {
1200+
$this->assertSame('Cannot autowire service "a": argument "$k" of method "Symfony\Component\DependencyInjection\Tests\Compiler\NotGuessableArgument::__construct()" has type "Symfony\Component\DependencyInjection\Tests\Compiler\Foo" but this symbol has been excluded.', (string) $e->getMessage());
1201+
}
1202+
}
1203+
1204+
public function testTypeNamespaceExcluded()
1205+
{
1206+
$container = new ContainerBuilder();
1207+
1208+
$container->register(__NAMESPACE__)->setAbstract(true)->addTag('container.excluded');
1209+
$aDefinition = $container->register('a', NotGuessableArgument::class);
1210+
$aDefinition->setAutowired(true);
1211+
1212+
$pass = new AutowirePass();
1213+
try {
1214+
$pass->process($container);
1215+
$this->fail('AutowirePass should have thrown an exception');
1216+
} catch (AutowiringFailedException $e) {
1217+
$this->assertSame('Cannot autowire service "a": argument "$k" of method "Symfony\Component\DependencyInjection\Tests\Compiler\NotGuessableArgument::__construct()" has type "Symfony\Component\DependencyInjection\Tests\Compiler\Foo" but the "Symfony\Component\DependencyInjection\Tests\Compiler" namespace has been excluded.', (string) $e->getMessage());
1218+
}
1219+
}
11891220
}

‎src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,21 @@ public function testMerge()
633633
$this->assertSame(['AInterface' => $childDefA, 'BInterface' => $childDefB], $container->getAutoconfiguredInstanceof());
634634
}
635635

636+
public function testMergeWithExcludedServices
637+
{
638+
$container = new ContainerBuilder();
639+
$container->setAlias('bar', 'foo');
640+
$container->register('foo', 'Bar\FooClass');
641+
$config = new ContainerBuilder();
642+
$config->register('bar', 'Bar')->addTag('container.excluded');
643+
$config->register('foo', 'Bar')->addTag('container.excluded');
644+
$config->register('baz', 'Bar')->addTag('container.excluded');
645+
$container->merge($config);
646+
$this->assertEquals(['service_container', 'foo', 'baz'], array_keys($container->getDefinitions()));
647+
$this->assertFalse($container->getDefinition('foo')->hasTag('container.excluded'));
648+
$this->assertTrue($container->getDefinition('baz')->hasTag('container.excluded'));
649+
}
650+
636651
public function testMergeThrowsExceptionForDuplicateAutomaticInstanceofDefinitions()
637652
{
638653
$this->expectException(InvalidArgumentException::class);

‎src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\BadClasses\MissingParent;
2828
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo;
2929
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\FooInterface;
30-
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\AnotherSub\DeeperBaz;
30+
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\AnotherSub;
3131
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\Baz;
3232
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar;
3333
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\BarInterface;
@@ -116,10 +116,15 @@ public function testRegisterClassesWithExclude()
116116
'Prototype/{%other_dir%/AnotherSub,Foo.php}'
117117
);
118118

119-
$this->assertTrue($container->has(Bar::class));
120-
$this->assertTrue($container->has(Baz::class));
121-
$this->assertFalse($container->has(Foo::class));
122-
$this->assertFalse($container->has(DeeperBaz::class));
119+
$this->assertFalse($container->getDefinition(Bar::class)->isAbstract());
120+
$this->assertFalse($container->getDefinition(Baz::class)->isAbstract());
121+
$this->assertTrue($container->getDefinition(Foo::class)->isAbstract());
122+
$this->assertTrue($container->getDefinition(AnotherSub::class)->isAbstract());
123+
124+
$this->assertFalse($container->getDefinition(Bar::class)->hasTag('container.excluded'));
125+
$this->assertFalse($container->getDefinition(Baz::class)->hasTag('container.excluded'));
126+
$this->assertTrue($container->getDefinition(Foo::class)->hasTag('container.excluded'));
127+
$this->assertTrue($container->getDefinition(AnotherSub::class)->hasTag('container.excluded'));
123128

124129
$this->assertEquals([BarInterface::class], array_keys($container->getAliases()));
125130

‎src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,7 @@ public function testPrototype()
718718
$loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
719719
$loader->load('services_prototype.xml');
720720

721-
$ids = array_keys($container->getDefinitions());
721+
$ids = array_keys(array_filter($container->getDefinitions(), fn ($def) => !$def->hasTag('container.excluded')));
722722
sort($ids);
723723
$this->assertSame([Prototype\Foo::class, Prototype\Sub\Bar::class, 'service_container'], $ids);
724724

@@ -750,7 +750,7 @@ public function testPrototypeExcludeWithArray()
750750
$loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
751751
$loader->load('services_prototype_array.xml');
752752

753-
$ids = array_keys($container->getDefinitions());
753+
$ids = array_keys(array_filter($container->getDefinitions(), fn ($def) => !$def->hasTag('container.excluded')));
754754
sort($ids);
755755
$this->assertSame([Prototype\Foo::class, Prototype\Sub\Bar::class, 'service_container'], $ids);
756756

0 commit comments

Comments
 (0)