8000 [DependencyInjection] Optimize autowiring logic by telling it about e… · symfony/symfony@5215f18 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5215f18

Browse files
nicolas-grekasnicolas-grekas
committed
[DependencyInjection] Optimize autowiring logic by telling it about excluded symbols
1 parent 68771c5 commit 5215f18

File tree

16 files changed

+123
-29
lines changed

16 files changed

+123
-29
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: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,19 @@ 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+
$i = null;
509+
$namespace = $type;
510+
do {
511+
$namespace = substr($namespace, 0, $i);
512+
513+
if ($this->container->hasDefinition($namespace) && $tag = $this->container->getDefinition($namespace)->getTag('container.excluded')) {
514+
return sprintf('Cannot autowire service "%s": %s needs an instance of "%s" but this type has been excluded %s.', $currentId, $label, $type, $tag[0]['source'] ?? 'from autowiring');
515+
}
516+
} while (false !== $i = strrpos($namespace, '\\'));
517+
518+
if (!$r = $this->container->getReflectionClass($type, false)) {
507519
// either $type does not exist or a parent class does not exist
508520
try {
509521
$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/Configurator/PrototypeConfigurator.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ class PrototypeConfigurator extends AbstractServiceConfigurator
4141
private string $resource;
4242
private ?array $excludes = null;
4343
private bool $allowParent;
44+
private ?string $path;
4445

45-
public function __construct(ServicesConfigurator $parent, PhpFileLoader $loader, Definition $defaults, string $namespace, string $resource, bool $allowParent)
46+
public function __construct(ServicesConfigurator $parent, PhpFileLoader $loader, Definition $defaults, string $namespace, string $resource, bool $allowParent, string $path = null)
4647
{
4748
$definition = new Definition();
4849
if (!$defaults->isPublic() || !$defaults->isPrivate()) {
@@ -57,6 +58,7 @@ public function __construct(ServicesConfigurator $parent, PhpFileLoader $loader,
5758
$this->loader = $loader;
5859
$this->resource = $resource;
5960
$this->allowParent = $allowParent;
61+
$this->path = $path;
6062

6163
parent::__construct($parent, $definition, $namespace, $defaults->getTags());
6264
}
@@ -66,7 +68,7 @@ public function __destruct()
6668
parent::__destruct();
6769

6870
if (isset($this->loader)) {
69-
$this->loader->registerClasses($this->definition, $this->id, $this->resource, $this->excludes);
71+
$this->loader->registerClasses($this->definition, $this->id, $this->resource, $this->excludes, $this->path);
7072
}
7173
unset($this->loader);
7274
}

‎src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ final public function alias(string $id, string $referencedId): AliasConfigurator
129129
*/
130130
final public function load(string $namespace, string $resource): PrototypeConfigurator
131131
{
132-
return new PrototypeConfigurator($this, $this->loader, $this->defaults, $namespace, $resource, true);
132+
return new PrototypeConfigurator($this, $this->loader, $this->defaults, $namespace, $resource, true, $this->path);
133133
}
134134

135135
/**

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

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,9 @@ public function import(mixed $resource, string $type = null, bool|string $ignore
9090
* @param string $namespace The namespace prefix of classes in the scanned directory
9191
* @param string $resource The directory to look for classes, glob-patterns allowed
9292
* @param string|string[]|null $exclude A globbed path of files to exclude or an array of globbed paths of files to exclude
93+
* @param string|null $source The path to the file that defines the auto-discovery rule
9394
*/
94-
public function registerClasses(Definition $prototype, string $namespace, string $resource, string|array $exclude = null)
95+
public function registerClasses(Definition $prototype, string $namespace, string $resource, string|array $exclude = null/*, string $source = null*/)
9596
{
9697
if (!str_ends_with($namespace, '\\')) {
9798
throw new InvalidArgumentException(sprintf('Namespace prefix must end with a "\\": "%s".', $namespace));
@@ -100,9 +101,10 @@ public function registerClasses(Definition $prototype, string $namespace, string
100101
throw new InvalidArgumentException(sprintf('Namespace is not a valid PSR-4 prefix: "%s".', $namespace));
101102
}
102103

104+
$source = \func_num_args() > 4 ? func_get_arg(4) : null;
103105
$autoconfigureAttributes = new RegisterAutoconfigureAttributesPass();
104106
$autoconfigureAttributes = $autoconfigureAttributes->accept($prototype) ? $autoconfigureAttributes : null;
105-
$classes = $this->findClasses($namespace, $resource, (array) $exclude, $autoconfigureAttributes);
107+
$classes = $this->findClasses($namespace, $resource, (array) $exclude, $autoconfigureAttributes, $source);
106108
// prepare for deep cloning
107109
$serializedPrototype = serialize($prototype);
108110

@@ -169,7 +171,7 @@ protected function setDefinition(string $id, Definition $definition)
169171
}
170172
}
171173

172-
private function findClasses(string $namespace, string $pattern, array $excludePatterns, ?RegisterAutoconfigureAttributesPass $autoconfigureAttributes): array
174+
private function findClasses(string $namespace, string $pattern, array $excludePatterns, ?RegisterAutoconfigureAttributesPass $autoconfigureAttributes, ?string $source): array
173175
{
174176
$parameterBag = $this->container->getParameterBag();
175177

@@ -189,7 +191,6 @@ private function findClasses(string $namespace, string $pattern, array $excludeP
189191

190192
$pattern = $parameterBag->unescapeValue($parameterBag->resolveValue($pattern));
191193
$classes = [];
192-
$extRegexp = '/\\.php$/';
193194
$prefixLen = null;
194195
foreach ($this->glob($pattern, true, $resource, false, false, $excludePaths) as $path => $info) {
195196
if (null === $prefixLen) {
@@ -204,10 +205,10 @@ private function findClasses(string $namespace, string $pattern, array $excludeP
204205
continue;
205206
}
206207

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

212213
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)) {
213214
continue;
@@ -242,6 +243,19 @@ private function findClasses(string $namespace, string $pattern, array $excludeP
242243
}
243244
}
244245

246+
if (null !== $prefixLen) {
247+
$attributes = null !== $source ? ['source' => sprintf('in "%s/%s"', basename(\dirname($source)), basename($source))] : [];
248+
249+
foreach ($excludePaths as $path => $_) {
250+
$class = $namespace.ltrim(str_replace('/', '\\', substr($path, $prefixLen, str_ends_with($path, '.php') ? -4 : null)), '\\');
251+
if (!$this->container->has($class)) {
252+
$this->container->register($class)
253+
->setAbstract(true)
254+
->addTag('container.excluded', $attributes);
255+
}
256+
}
257+
}
258+
245259
return $classes;
246260
}
247261
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ private function parseDefinitions(\DOMDocument $xml, string $file, Definition $d
184184
}
185185
$excludes = [$service->getAttribute('exclude')];
186186
}
187-
$this->registerClasses($definition, (string) $service->getAttribute('namespace'), (string) $service->getAttribute('resource'), $excludes);
187+
$this->registerClasses($definition, (string) $service->getAttribute('namespace'), (string) $service->getAttribute('resource'), $excludes, $file);
188188
} else {
189189
$this->setDefinition((string) $service->getAttribute('id'), $definition);
190190
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,7 @@ private function parseDefinition(string $id, array|string|null $service, string
692692
}
693693
$exclude = $service['exclude'] ?? null;
694694
$namespace = $service['namespace'] ?? $id;
695-
$this->registerClasses($definition, $namespace, $service['resource'], $exclude);
695+
$this->registerClasses($definition, $namespace, $service['resource'], $exclude, $file);
696696
} else {
697697
$this->setDefinition($id, $definition);
698698
}

‎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', ['source' => 'for tests']);
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()" needs an instance of "Symfony\Component\DependencyInjection\Tests\Compiler\Foo" but this type has been excluded for tests.', (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()" needs an instance of "Symfony\Component\DependencyInjection\Tests\Compiler\Foo" but this type has been excluded from autowiring.', (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(' 17AE 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

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

Lines 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([

0 commit comments

Comments
 (0)
0