Skip to content

Autowire arguments using attributes #1

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
namespace Symfony\Component\DependencyInjection\Attribute;

#[\Attribute(\Attribute::TARGET_PARAMETER)]
class BindTaggedLocator
class TaggedIterator
{
public function __construct(
public string $tag,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
namespace Symfony\Component\DependencyInjection\Attribute;

#[\Attribute(\Attribute::TARGET_PARAMETER)]
class BindTaggedIterator
class TaggedLocator
{
public function __construct(
public string $tag,
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/DependencyInjection/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ CHANGELOG
* Add support for loading autoconfiguration rules via the `#[Autoconfigure]` and `#[AutoconfigureTag]` attributes on PHP 8
* Add `#[AsTaggedItem]` attribute for defining the index and priority of classes found in tagged iterators/locators
* Add autoconfigurable attributes
* Add support for binding tagged iterators and locators to constructor arguments via attributes
* Add support for autowiring tagged iterators and locators via attributes on PHP 8
* Add support for per-env configuration in loaders
* Add `ContainerBuilder::willBeAvailable()` to help with conditional configuration
* Add support an integer return value for default_index_method
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,102 +11,46 @@

namespace Symfony\Component\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\Argument\BoundArgument;
use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
use Symfony\Component\DependencyInjection\Attribute\BindTaggedIterator;
use Symfony\Component\DependencyInjection\Attribute\BindTaggedLocator;
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Exception\LogicException;

/**
* @author Alexander M. Turek <[email protected]>
*/
final class AttributeAutoconfigurationPass extends AbstractRecursivePass
{
/** @var array<string, callable>|null */
private $argumentConfigurators;

public function process(ContainerBuilder $container): void
{
if (80000 > \PHP_VERSION_ID) {
if (80000 > \PHP_VERSION_ID || !$container->getAutoconfiguredAttributes()) {
return;
}

$this->argumentConfigurators = [
BindTaggedIterator::class => static function (BindTaggedIterator $attribute) {
return new TaggedIteratorArgument($attribute->tag, $attribute->indexAttribute);
},
BindTaggedLocator::class => static function (BindTaggedLocator $attribute) {
return new ServiceLocatorArgument(new TaggedIteratorArgument($attribute->tag, $attribute->indexAttribute));
},
];

parent::process($container);

$this->argumentConfigurators = null;
}

protected function processValue($value, bool $isRoot = false)
{
if ($value instanceof Definition
&& $value->isAutoconfigured()
&& !$value->isAbstract()
&& !$value->hasTag('container.ignore_attributes')
if (!$value instanceof Definition
|| !$value->isAutoconfigured()
|| $value->isAbstract()
|| $value->hasTag('container.ignore_attributes')
|| !($reflector = $this->container->getReflectionClass($value->getClass(), false))
) {
$value = $this->processDefinition($value);
}

return parent::processValue($value, $isRoot);
}

private function processDefinition(Definition $definition): Definition
{
if (!$reflector = $this->container->getReflectionClass($definition->getClass(), false)) {
return $definition;
return parent::processValue($value, $isRoot);
}

$autoconfiguredAttributes = $this->container->getAutoconfiguredAttributes();

$instanceof = $definition->getInstanceofConditionals();
$instanceof = $value->getInstanceofConditionals();
$conditionals = $instanceof[$reflector->getName()] ?? new ChildDefinition('');
foreach ($reflector->getAttributes() as $attribute) {
if ($configurator = $autoconfiguredAttributes[$attribute->getName()] ?? null) {
$configurator($conditionals, $attribute->newInstance(), $reflector);
}
}

if ($constructor = $this->getConstructor($definition, false)) {
$definition = $this->bindArguments($definition, $constructor);
}

$instanceof[$reflector->getName()] = $conditionals;
$definition->setInstanceofConditionals($instanceof);

return $definition;
}

private function bindArguments(Definition $definition, \ReflectionFunctionAbstract $constructor): Definition
{
$bindings = $definition->getBindings();
foreach ($constructor->getParameters() as $reflectionParameter) {
$argument = null;
foreach ($reflectionParameter->getAttributes() as $attribute) {
if (!$configurator = $this->argumentConfigurators[$attribute->getName()] ?? null) {
continue;
}
if ($argument) {
throw new LogicException(sprintf('Cannot autoconfigure argument "$%s": More than one autoconfigurable attribute found.', $reflectionParameter->getName()));
}
$argument = $configurator($attribute->newInstance());
}
if ($argument) {
$bindings['$'.$reflectionParameter->getName()] = new BoundArgument($argument);
}
}
$value->setInstanceofConditionals($instanceof);

return $definition->setBindings($bindings);
return parent::processValue($value, $isRoot);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
namespace Symfony\Component\DependencyInjection\Compiler;

use Symfony\Component\Config\Resource\ClassExistenceResource;
use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
use Symfony\Component\DependencyInjection\Attribute\TaggedIterator;
use Symfony\Component\DependencyInjection\Attribute\TaggedLocator;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Exception\AutowiringFailedException;
Expand Down Expand Up @@ -123,7 +127,8 @@ private function doProcessValue($value, bool $isRoot = false)
array_unshift($this->methodCalls, [$constructor, $value->getArguments()]);
}

$this->methodCalls = $this->autowireCalls($reflectionClass, $isRoot);
$checkAttributes = 80000 <= \PHP_VERSION_ID && !$value->hasTag('container.ignore_attributes');
$this->methodCalls = $this->autowireCalls($reflectionClass, $isRoot, $checkAttributes);

if ($constructor) {
[, $arguments] = array_shift($this->methodCalls);
Expand All @@ -140,7 +145,7 @@ private function doProcessValue($value, bool $isRoot = false)
return $value;
}

private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot): array
private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot, bool $checkAttributes): array
{
$this->decoratedId = null;
$this->decoratedClass = null;
Expand Down Expand Up @@ -168,7 +173,7 @@ private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot):
}
}

$arguments = $this->autowireMethod($reflectionMethod, $arguments);
$arguments = $this->autowireMethod($reflectionMethod, $arguments, $checkAttributes);

if ($arguments !== $call[1]) {
$this->methodCalls[$i][1] = $arguments;
Expand All @@ -185,7 +190,7 @@ private function autowireCalls(\ReflectionClass $reflectionClass, bool $isRoot):
*
* @throws AutowiringFailedException
*/
private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, array $arguments): array
private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, array $arguments, bool $checkAttributes): array
{
$class = $reflectionMethod instanceof \ReflectionMethod ? $reflectionMethod->class : $this->currentId;
$method = $reflectionMethod->name;
Expand All @@ -201,6 +206,27 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a

$type = ProxyHelper::getTypeHint($reflectionMethod, $parameter, true);

if ($checkAttributes) {
$attribute = null;
foreach ($parameter->getAttributes() as $attribute) {
if (TaggedIterator::class === $attribute->getName()) {
$attribute = $attribute->newInstance();
$arguments[$index] = new TaggedIteratorArgument($attribute->tag, $attribute->indexAttribute);
break;
}

if (TaggedLocator::class === $attribute->getName()) {
$attribute = $attribute->newInstance();
$arguments[$index] = new ServiceLocatorArgument(new TaggedIteratorArgument($attribute->tag, $attribute->indexAttribute));
break;
}
}

if ('' !== ($arguments[$index] ?? '')) {
continue;
}
}

if (!$type) {
if (isset($arguments[$index])) {
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ public function __construct()
new AutowireRequiredMethodsPass(),
new AutowireRequiredPropertiesPass(),
new ResolveBindingsPass(),
new ServiceLocatorTagPass(),
new DecoratorServicePass(),
new CheckDefinitionValidityPass(),
new AutowirePass(false),
new ServiceLocatorTagPass(),
new ResolveTaggedIteratorArgumentPass(),
new ResolveServiceSubscribersPass(),
new ResolveReferencesToAliasesPass(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ public function testTaggedServiceWithIndexAttributeAndDefaultMethodConfiguredVia
->addTag('foo_bar', ['foo' => 'foo'])
;
$container->register(IteratorConsumer::class)
->setAutoconfigured(true)
->setAutowired(true)
->setPublic(true)
;

Expand Down Expand Up @@ -391,7 +391,7 @@ public function testTaggedLocatorConfiguredViaAttribute()
->addTag('foo_bar', ['foo' => 'foo'])
;
$container->register(LocatorConsumer::class)
->setAutoconfigured(true)
->setAutowired(true)
->setPublic(true)
;

Expand Down Expand Up @@ -419,7 +419,7 @@ public function testNestedDefinitionWithAutoconfiguredConstructorArgument()
->setPublic(true)
->setArguments([
(new Definition(LocatorConsumer::class))
->setAutoconfigured(true),
->setAutowired(true),
])
;

Expand All @@ -445,7 +445,7 @@ public function testFactoryWithAutoconfiguredArgument()
$container->register(LocatorConsumerFactory::class);
$container->register(LocatorConsumer::class)
->setPublic(true)
->setAutoconfigured(true)
->setAutowired(true)
->setFactory(new Reference(LocatorConsumerFactory::class))
;

Expand All @@ -458,22 +458,6 @@ public function testFactoryWithAutoconfiguredArgument()
self::assertSame($container->get(FooTagClass::class), $locator->get('my_service'));
}

/**
* @requires PHP 8
*/
public function testMultipleArgumentBindings()
Copy link
Author

@nicolas-grekas nicolas-grekas Apr 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that the behavior that this test case checks is desired. Better deal with this in a fifo way. That's less code for us, to deal with a situation that we don't have to deal with in the first place.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intention here was to make a developer's mistake visible. But we can remove the check if you think we don't need it.

{
$container = new ContainerBuilder();
$container->register(MultipleArgumentBindings::class)
->setPublic(true)
->setAutoconfigured(true)
;

$this->expectException(LogicException::class);
$this->expectExceptionMessage('Cannot autoconfigure argument "$collection": More than one autoconfigurable attribute found.');
$container->compile();
}

public function testTaggedServiceWithDefaultPriorityMethod()
{
$container = new ContainerBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@

namespace Symfony\Component\DependencyInjection\Tests\Fixtures;

use Symfony\Component\DependencyInjection\Attribute\BindTaggedIterator;
use Symfony\Component\DependencyInjection\Attribute\TaggedIterator;

final class IteratorConsumer
{
public function __construct(
#[BindTaggedIterator('foo_bar', indexAttribute: 'foo')]
#[TaggedIterator('foo_bar', indexAttribute: 'foo')]
private iterable $param,
) {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@
namespace Symfony\Component\DependencyInjection\Tests\Fixtures;

use Psr\Container\ContainerInterface;
use Symfony\Component\DependencyInjection\Attribute\BindTaggedLocator;
use Symfony\Component\DependencyInjection\Attribute\TaggedLocator;

final class LocatorConsumer
{
public function __construct(
#[BindTaggedLocator('foo_bar', indexAttribute: 'foo')]
#[TaggedLocator('foo_bar', indexAttribute: 'foo')]
private ContainerInterface $locator,
) {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@
namespace Symfony\Component\DependencyInjection\Tests\Fixtures;

use Psr\Container\ContainerInterface;
use Symfony\Component\DependencyInjection\Attribute\BindTaggedLocator;
use Symfony\Component\DependencyInjection\Attribute\TaggedLocator;

final class LocatorConsumerFactory
{
public function __invoke(
#[BindTaggedLocator('foo_bar', indexAttribute: 'key')]
#[TaggedLocator('foo_bar', indexAttribute: 'key')]
ContainerInterface $locator
): LocatorConsumer {
return new LocatorConsumer($locator);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

namespace Symfony\Component\DependencyInjection\Tests\Fixtures;

use Symfony\Component\DependencyInjection\Attribute\BindTaggedIterator;
use Symfony\Component\DependencyInjection\Attribute\BindTaggedLocator;
use Symfony\Component\DependencyInjection\Attribute\TaggedIterator;
use Symfony\Component\DependencyInjection\Attribute\TaggedLocator;

final class MultipleArgumentBindings
{
public function __construct(
#[BindTaggedIterator('my_tag'), BindTaggedLocator('another_tag')]
#[TaggedIterator('my_tag'), TaggedLocator('another_tag')]
object $collection
) {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public function getRemovedIds(): array
return [
'.service_locator.DlIAmAe' => true,
'.service_locator.DlIAmAe.foo_service' => true,
'.service_locator.t5IGRMW' => true,
'Psr\\Container\\ContainerInterface' => true,
'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
'Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition' => true,
Expand Down