Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 93ed461

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
[DependencyInjection] Optimize autowiring logic by telling it about excluded symbols
1 parent c0dbb90 commit 93ed461
Copy full SHA for 93ed461

File tree

12 files changed

+109
-21
lines changed
Filter options

12 files changed

+109
-21
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php
+1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Resources/config/services.php
+10Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php
+3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
+16-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/ContainerBuilder.php
+5-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Loader/FileLoader.php
+13-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
+34-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
+12Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,18 @@ public function testMerge()
631631
$childDefB = $config->registerForAutoconfiguration('BInterface');
632632
$container->merge($config);
633633
$this->assertSame(['AInterface' => $childDefA, 'BInterface' => $childDefB], $container->getAutoconfiguredInstanceof());
634+
635+
$container = new ContainerBuilder();
636+
$container->setAlias('bar', 'foo');
637+
$container->register('foo', 'Bar\FooClass');
638+
$config = new ContainerBuilder();
639+
$config->register('bar', 'Bar')->addTag('container.excluded');
640+
$config->register('foo', 'Bar')->addTag('container.excluded');
641+
$config->register('baz', 'Bar')->addTag('container.excluded');
642+
$container->merge($config);
643+
$this->assertEquals(['service_container', 'foo', 'baz'], array_keys($container->getDefinitions()), '->merge() skips excluded definitions');
644+
$this->assertFalse($container->getDefinition('foo')->hasTag('container.excluded'));
645+
$this->assertTrue($container->getDefinition('baz')->hasTag('container.excluded'));
634646
}
635647

636648
public function testMergeThrowsExceptionForDuplicateAutomaticInstanceofDefinitions()

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php
+10-5Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
+2-2Lines 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

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ public function testPrototype()
497497
$loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
498498
$loader->load('services_prototype.yml');
499499

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

@@ -528,7 +528,7 @@ public function testPrototypeWithNamespace()
528528
$loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
529529
$loader->load('services_prototype_namespace.yml');
530530

531-
$ids = array_keys($container->getDefinitions());
531+
$ids = array_keys(array_filter($container->getDefinitions(), fn ($def) => !$def->hasTag('container.excluded')));
532532
sort($ids);
533533

534534
$this->assertSame([

‎src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php
+1-4Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,11 @@
1111

1212
namespace Symfony\Component\HttpKernel\DependencyInjection;
1313

14-
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
1514
use Symfony\Component\DependencyInjection\Attribute\Autowire;
1615
use Symfony\Component\DependencyInjection\Attribute\Target;
1716
use Symfony\Component\DependencyInjection\ChildDefinition;
1817
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
1918
use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
20-
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
2119
use Symfony\Component\DependencyInjection\ContainerBuilder;
2220
use Symfony\Component\DependencyInjection\ContainerInterface;
2321
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
@@ -70,13 +68,12 @@ public function process(ContainerBuilder $container)
7068
if (!$r = $container->getReflectionClass($class)) {
7169
throw new InvalidArgumentException(sprintf('Class "%s" used for service "%s" cannot be found.', $class, $id));
7270
}
73-
$isContainerAware = $r->implementsInterface(ContainerAwareInterface::class) || is_subclass_of($class, AbstractController::class);
7471

7572
// get regular public methods
7673
$methods = [];
7774
$arguments = [];
7875
foreach ($r->getMethods(\ReflectionMethod::IS_PUBLIC) as $r) {
79-
if ('setContainer' === $r->name && $isContainerAware) {
76+
if ('setContainer' === $r->name) {
8077
continue;
8178
}
8279
if (!$r->isConstructor() && !$r->isDestructor() && !$r->isAbstract()) {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.