diff --git a/UPGRADE-7.4.md b/UPGRADE-7.4.md index 753592c7163d3..1686bc71c4c36 100644 --- a/UPGRADE-7.4.md +++ b/UPGRADE-7.4.md @@ -27,6 +27,7 @@ DependencyInjection ------------------- * Add argument `$target` to `ContainerBuilder::registerAliasForArgument()` + * Add argument `$throwOnAbstract` to `ContainerBuilder::findTaggedResourceIds()` * Deprecate registering a service without a class when its id is a non-existing FQCN DoctrineBridge diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md index 7212720f826af..8e40854be3e6b 100644 --- a/src/Symfony/Component/DependencyInjection/CHANGELOG.md +++ b/src/Symfony/Component/DependencyInjection/CHANGELOG.md @@ -6,6 +6,7 @@ CHANGELOG * Allow adding resource tags using any config format * Allow `#[AsAlias]` to be extended + * Parse attributes found on abstract classes for resource definitions * Add argument `$target` to `ContainerBuilder::registerAliasForArgument()` * Deprecate registering a service without a class when its id is a non-existing FQCN diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AttributeAutoconfigurationPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AttributeAutoconfigurationPass.php index bbf341913e4d8..b61931e5a3f21 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AttributeAutoconfigurationPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AttributeAutoconfigurationPass.php @@ -79,14 +79,18 @@ public function process(ContainerBuilder $container): void } } - parent::process($container); + $this->container = $container; + foreach ($container->getDefinitions() as $id => $definition) { + $this->currentId = $id; + $this->processValue($definition, true); + } } protected function processValue(mixed $value, bool $isRoot = false): mixed { if (!$value instanceof Definition || !$value->isAutoconfigured() - || $value->isAbstract() + || ($value->isAbstract() && !$value->hasTag('container.excluded')) || $value->hasTag('container.ignore_attributes') || !($classReflector = $this->container->getReflectionClass($value->getClass(), false)) ) { diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php index 3752f82233b00..e4954776896df 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php @@ -62,7 +62,7 @@ private function processValue(mixed $value, int $rootLevel = 0, int $level = 0): } elseif ($value instanceof ArgumentInterface) { $value->setValues($this->processValue($value->getValues(), $rootLevel, 1 + $level)); } elseif ($value instanceof Definition) { - if ($value->isSynthetic() || $value->isAbstract()) { + if ($value->isSynthetic() || $value->isAbstract() || $value->hasTag('container.excluded')) { return $value; } $value->setArguments($this->processValue($value->getArguments(), 0)); diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 08fda703c8aee..5c1d8845da26e 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -1366,19 +1366,30 @@ public function findTaggedServiceIds(string $name, bool $throwOnAbstract = false * } * } * + * @param bool $throwOnAbstract + * * @return array An array of tags with the tagged service as key, holding a list of attribute arrays */ - public function findTaggedResourceIds(string $tagName): array + public function findTaggedResourceIds(string $tagName/* , bool $throwOnAbstract = true */): array { + $throwOnAbstract = \func_num_args() > 1 ? func_get_arg(1) : true; $this->usedTags[] = $tagName; $tags = []; foreach ($this->getDefinitions() as $id => $definition) { - if ($definition->hasTag($tagName)) { - if (!$definition->hasTag('container.excluded')) { - throw new InvalidArgumentException(\sprintf('The resource "%s" tagged "%s" is missing the "container.excluded" tag.', $id, $tagName)); - } - $tags[$id] = $definition->getTag($tagName); + if (!$definition->hasTag($tagName)) { + continue; + } + if (!$definition->hasTag('container.excluded')) { + throw new InvalidArgumentException(\sprintf('The resource "%s" tagged "%s" is missing the "container.excluded" tag.', $id, $tagName)); + } + $class = $this->parameterBag->resolveValue($definition->getClass()); + if (!$class || $throwOnAbstract && $definition->isAbstract()) { + throw new InvalidArgumentException(\sprintf('The resource "%s" tagged "%s" must have a class and not be abstract.', $id, $tagName)); + } + if ($definition->getClass() !== $class) { + $definition->setClass($class); } + $tags[$id] = $definition->getTag($tagName); } return $tags; diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php index b410d02204636..9f5ae0a5822f3 100644 --- a/src/Symfony/Component/DependencyInjection/Definition.php +++ b/src/Symfony/Component/DependencyInjection/Definition.php @@ -465,8 +465,7 @@ public function addTag(string $name, array $attributes = []): static public function addResourceTag(string $name, array $attributes = []): static { return $this->addTag($name, $attributes) - ->addTag('container.excluded', ['source' => \sprintf('by tag "%s"', $name)]) - ->setAbstract(true); + ->addTag('container.excluded', ['source' => \sprintf('by tag "%s"', $name)]); } /** diff --git a/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php index 3cf23cf98eab4..3647d184c2070 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php @@ -189,15 +189,9 @@ public function registerClasses(Definition $prototype, string $namespace, string } $r = null === $errorMessage ? $this->container->getReflectionClass($class) : null; - if ($r?->isAbstract() || $r?->isInterface()) { - if ($r->isInterface()) { - $this->interfaces[] = $class; - } - $autoconfigureAttributes?->processClass($this->container, $r); - continue; - } - $this->setDefinition($class, $definition = $getPrototype()); + $abstract = $r?->isAbstract() || $r?->isInterface() ? '.abstract.' : ''; + $this->setDefinition($abstract.$class, $definition = $getPrototype()); if (null !== $errorMessage) { $definition->addError($errorMessage); @@ -205,6 +199,16 @@ public function registerClasses(Definition $prototype, string $namespace, string } $definition->setClass($class); + if ($abstract) { + if ($r->isInterface()) { + $this->interfaces[] = $class; + } + $autoconfigureAttributes?->processClass($this->container, $r); + $definition->setAbstract(true) + ->addTag('container.excluded', ['source' => 'because the class is abstract']); + continue; + } + $interfaces = []; foreach (class_implements($class, false) as $interface) { $this->singlyImplemented[$interface] = ($this->singlyImplemented[$interface] ?? $class) !== $class ? false : $class; diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AttributeAutoconfigurationPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AttributeAutoconfigurationPassTest.php index dd58a86d1d659..cc4071ea4aa07 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AttributeAutoconfigurationPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AttributeAutoconfigurationPassTest.php @@ -45,4 +45,31 @@ public function testAttributeConfiguratorCallableMissingType() $this->expectExceptionMessage('Argument "$reflector" of attribute autoconfigurator should have a type, use one or more of "\ReflectionClass|\ReflectionMethod|\ReflectionProperty|\ReflectionParameter|\Reflector" in '); (new AttributeAutoconfigurationPass())->process($container); } + + public function testProcessesAbstractServicesWithContainerExcludedTag() + { + $container = new ContainerBuilder(); + $container->registerAttributeForAutoconfiguration(AsTaggedItem::class, static function (ChildDefinition $definition, AsTaggedItem $attribute, \ReflectionClass $reflector) { + $definition->addTag('processed.tag'); + }); + + // Create an abstract service with container.excluded tag and attributes + $abstractService = $container->register('abstract_service', TestServiceWithAttributes::class) + ->setAutoconfigured(true) + ->setAbstract(true) + ->addTag('container.excluded', ['source' => 'test']); + + (new AttributeAutoconfigurationPass())->process($container); + + // Abstract service with container.excluded tag should be processed + $expected = [ + TestServiceWithAttributes::class => (new ChildDefinition(''))->addTag('processed.tag'), + ]; + $this->assertEquals($expected, $abstractService->getInstanceofConditionals()); + } +} + +#[AsTaggedItem] +class TestServiceWithAttributes +{ } diff --git a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php index e1f292d54573f..9abdb94e7be2b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php @@ -262,7 +262,7 @@ public function testAddResourceTag() $def->addResourceTag('foo', ['bar' => true]); $this->assertSame([['bar' => true]], $def->getTag('foo')); - $this->assertTrue($def->isAbstract()); + $this->assertFalse($def->isAbstract()); $this->assertSame([['source' => 'by tag "foo"']], $def->getTag('container.excluded')); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/AbstractClass.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/AbstractClass.php new file mode 100644 index 0000000000000..0fd7737f92f47 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/AbstractClass.php @@ -0,0 +1,7 @@ +registerAliasesForSinglyImplementedInterfaces(); $this->assertEquals( - ['service_container', Bar::class], + ['service_container', Bar::class, '.abstract.'.BarInterface::class], array_keys($container->getDefinitions()) ); $this->assertEquals([BarInterface::class], array_keys($container->getAliases())); @@ -214,6 +215,24 @@ public function testNestedRegisterClasses() $this->assertEquals([FooInterface::class => (new ChildDefinition(''))->addTag('foo')], $container->getAutoconfiguredInstanceof()); } + public function testRegisterClassesWithAbstractClassesAndAutoconfigure() + { + $container = new ContainerBuilder(); + $loader = new TestFileLoader($container, new FileLocator(self::$fixturesPath.'/Fixtures')); + + $loader->registerClasses( + (new Definition())->setAutoconfigured(true), + 'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\\', + 'Prototype/*', + 'Prototype/{StaticConstructor}' + ); + + $definition = $container->getDefinition('.abstract.'.AbstractClass::class); + $this->assertTrue($definition->isAbstract()); + $this->assertTrue($definition->hasTag('container.excluded')); + $this->assertTrue($definition->isAutoconfigured()); + } + public function testMissingParentClass() { $container = new ContainerBuilder(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/PhpFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/PhpFileLoaderTest.php index d62d54be97d07..5702dba9fc97d 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/PhpFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/PhpFileLoaderTest.php @@ -156,7 +156,7 @@ public function testResourceTags() $this->assertTrue($def->hasTag('another.tag')); $this->assertSame([['foo' => 'bar']], $def->getTag('my.tag')); $this->assertSame([[]], $def->getTag('another.tag')); - $this->assertTrue($def->isAbstract()); + $this->assertFalse($def->isAbstract()); } public function testAutoConfigureAndChildDefinition() diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php index feb200292a757..ed385ffa08151 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php @@ -381,7 +381,7 @@ public function testResourceTags() $this->assertTrue($def->hasTag('another.tag')); $this->assertSame([['foo' => 'bar']], $def->getTag('my.tag')); $this->assertSame([[]], $def->getTag('another.tag')); - $this->assertTrue($def->isAbstract()); + $this->assertFalse($def->isAbstract()); } public function testParsesIteratorArgument() diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index f36b8645df8fe..e7e7a534cbd19 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -259,7 +259,7 @@ public function testResourceTags() $this->assertTrue($def->hasTag('another.tag')); $this->assertSame([['foo' => 'bar']], $def->getTag('my.tag')); $this->assertSame([[]], $def->getTag('another.tag')); - $this->assertTrue($def->isAbstract()); + $this->assertFalse($def->isAbstract()); } public function testLoadShortSyntax() diff --git a/src/Symfony/Component/JsonStreamer/DependencyInjection/StreamablePass.php b/src/Symfony/Component/JsonStreamer/DependencyInjection/StreamablePass.php index babf962bfcdaa..196cbe6f21c7a 100644 --- a/src/Symfony/Component/JsonStreamer/DependencyInjection/StreamablePass.php +++ b/src/Symfony/Component/JsonStreamer/DependencyInjection/StreamablePass.php @@ -13,6 +13,7 @@ use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; /** * Sets the streamable metadata to the services that need them. @@ -31,16 +32,20 @@ public function process(ContainerBuilder $container): void // retrieve concrete services tagged with "json_streamer.streamable" tag foreach ($container->getDefinitions() as $id => $definition) { - if (!$tag = ($definition->getTag('json_streamer.streamable')[0] ?? null)) { + if (!$tag = $definition->getTag('json_streamer.streamable')[0] ?? null) { continue; } - - if (($className = $container->getDefinition($id)->getClass()) && !$container->getDefinition($id)->isAbstract()) { - $streamable[$className] = [ - 'object' => $tag['object'], - 'list' => $tag['list'], - ]; + if (!$definition->hasTag('container.excluded')) { + throw new InvalidArgumentException(\sprintf('The resource "%s" tagged "json_streamer.streamable" is missing the "container.excluded" tag.', $id)); + } + $class = $container->getParameterBag()->resolveValue($definition->getClass()); + if (!$class || $definition->isAbstract()) { + throw new InvalidArgumentException(\sprintf('The resource "%s" tagged "json_streamer.streamable" must have a class and not be abstract.', $id)); } + $streamable[$class] = [ + 'object' => $tag['object'], + 'list' => $tag['list'], + ]; $container->removeDefinition($id); } diff --git a/src/Symfony/Component/JsonStreamer/Tests/DependencyInjection/StreamablePassTest.php b/src/Symfony/Component/JsonStreamer/Tests/DependencyInjection/StreamablePassTest.php index f58bc9ce81998..8901a9b8101d6 100644 --- a/src/Symfony/Component/JsonStreamer/Tests/DependencyInjection/StreamablePassTest.php +++ b/src/Symfony/Component/JsonStreamer/Tests/DependencyInjection/StreamablePassTest.php @@ -13,6 +13,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\JsonStreamer\DependencyInjection\StreamablePass; class StreamablePassTest extends TestCase @@ -25,8 +26,7 @@ public function testSetStreamable() $container->register('.json_streamer.cache_warmer.streamer')->setArguments([null]); $container->register('.json_streamer.cache_warmer.lazy_ghost')->setArguments([null]); - $container->register('streamable')->setClass('Foo')->addTag('json_streamer.streamable', ['object' => true, 'list' => true]); - $container->register('abstractStreamable')->setClass('Bar')->addTag('json_streamer.streamable', ['object' => true, 'list' => true])->setAbstract(true); + $container->register('streamable')->setClass('Foo')->addTag('json_streamer.streamable', ['object' => true, 'list' => true])->addTag('container.excluded'); $container->register('notStreamable')->setClass('Baz'); $pass = new StreamablePass(); @@ -37,5 +37,9 @@ public function testSetStreamable() $this->assertSame(['Foo' => ['object' => true, 'list' => true]], $streamerCacheWarmer->getArgument(0)); $this->assertSame(['Foo'], $lazyGhostCacheWarmer->getArgument(0)); + + $container->register('abstractStreamable')->setClass('Bar')->addTag('json_streamer.streamable', ['object' => true, 'list' => true])->addTag('container.excluded')->setAbstract(true); + $this->expectException(InvalidArgumentException::class); + $pass->process($container); } }