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

[DependencyInjection] Add #[AutowireInline] attribute to allow service definition at the class level #52820

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.

Sign up for GitHub

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
merged 2 commits into from
May 2, 2024
Merged

[DependencyInjection] Add #[AutowireInline] attribute to allow service definition at the class level #52820

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a596142
[DependencyInjection] Add `#[AutowireInline]` attribute to allow serv…
DaDeather Nov 30, 2023
b9a838e
Finish implementing AutowireInline attribute
nicolas-grekas Apr 22, 2024
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
Prev Previous commit
Finish implementing AutowireInline attribute
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed May 2, 2024
commit b9a838e61b297cca95b4ad0bd14d0617b2b55eb6
15 changes: 12 additions & 3 deletions 15 src/Symfony/Component/DependencyInjection/Attribute/AutowireInline.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,29 @@
namespace Symfony\Component\DependencyInjection\Attribute;

use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Exception\LogicException;
use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;

/**
* Allows inline service definition for a constructor argument.
* Using this attribute on a class autowires it as a new instance
* Allows inline service definition for an argument.
*
* Using this attribute on a class autowires a new instance
* which is not shared between different services.
*
* $class a FQCN, or an array to define a factory.
* Use the "@" prefix to reference a service.
*
* @author Ismail Özgün Turan <oezguen.turan@dadadev.com>
*/
#[\Attribute(\Attribute::TARGET_PARAMETER)]
class AutowireInline extends Autowire
Copy link
Member

@chalasr chalasr Mar 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

AutowireInline looks pretty cryptic, any newcomer wouldn't be able to get what the attribute is for by just looking at its name nor its description as it currently is. I think we either need to find a super self-explanatory name (I've no clue) or extend the description so that it tells what's the purpose of the attribute and when it should be used (inline service definition only means something to advanced Symfony's DIC hackers)

Copy link
Member

Choose a reason for hiding this comment

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

Naming things... :)
"inline" refers to "inline_service()" in the PHP-DSL

AutowireInlineService? but verbose
AutowireNew? AutowireObject? AutowireInstance? or keep AutowireInline?

of course, a top notch description is also desirable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a better description here feel free to request further suggestments to it 👍.

{
public function __construct(string|array $class, array $arguments = [], array $calls = [], array $properties = [], ?string $parent = null, bool|string $lazy = false)
public function __construct(string|array|null $class = null, array $arguments = [], array $calls = [], array $properties = [], ?string $parent = null, bool|string $lazy = false)
{
if (null === $class && null === $parent) {
throw new LogicException('#[AutowireInline] attribute should declare either $class or $parent.');
}

parent::__construct([
\is_array($class) ? 'factory' : 'class' => $class,
'arguments' => $arguments,
nicolas-grekas marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a

if ($attribute instanceof AutowireInline) {
$value = $attribute->buildDefinition($value, $type, $parameter);
$value = new Reference('.autowire_inline.'.ContainerBuilder::hash($value));
$value = $this->doProcessValue($value);
} elseif ($lazy = $attribute->lazy) {
$definition = (new Definition($type))
->setFactory('current')
Expand Down
3 changes: 3 additions & 0 deletions 3 src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ protected function processValue(mixed $value, bool $isRoot = false): mixed
*/
private function isInlineableDefinition(string $id, Definition $definition): bool
{
if (str_starts_with($id, '.autowire_inline.')) {
return true;
}
if ($definition->hasErrors() || $definition->isDeprecated() || $definition->isLazy() || $definition->isSynthetic() || $definition->hasTag('container.do_not_inline')) {
return false;
}
Expand Down
106 changes: 91 additions & 15 deletions 106 src/Symfony/Component/DependencyInjection/Compiler/ResolveAutowireInlineAttributesPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@
namespace Symfony\Component\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\Attribute\AutowireInline;
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\VarExporter\ProxyHelper;

/**
* Inspects existing autowired services for {@see AutowireInline} attribute and registers the definitions for reuse.
* Inspects existing autowired services for {@see AutowireInline} attributes and registers the definitions for reuse.
*
* @author Ismail Özgün Turan <oezguen.turan@dadadev.com>
*/
Expand All @@ -30,36 +32,110 @@ protected function processValue(mixed $value, bool $isRoot = false): mixed
{
$value = parent::processValue($value, $isRoot);

if (!$value instanceof Definition || !$value->isAutowired() || $value->isAbstract() || !$value->getClass()) {
if (!$value instanceof Definition || !$value->isAutowired() || !$value->getClass() || $value->hasTag('container.ignore_attributes')) {
return $value;
}

$isChildDefinition = $value instanceof ChildDefinition;

try {
$constructor = $this->getConstructor($value, false);
} catch (RuntimeException) {
$this->container->log($this, sprintf('Skipping service "%s": Class or interface "%s" cannot be loaded.', $this->currentId, $value->getClass()));

return $value;
}

if ($constructor === null) {
return $value;
if ($constructor) {
$arguments = $this->registerAutowireInlineAttributes($constructor, $value->getArguments(), $isChildDefinition);

if ($arguments !== $value->getArguments()) {
$value->setArguments($arguments);
}
}

$reflectionParameters = $constructor->getParameters();
foreach ($reflectionParameters as $reflectionParameter) {
$autowireInlineAttributes = $reflectionParameter->getAttributes(AutowireInline::class, \ReflectionAttribute::IS_INSTANCEOF);
foreach ($autowireInlineAttributes as $autowireInlineAttribute) {
/** @var AutowireInline $autowireInlineAttributeInstance */
$autowireInlineAttributeInstance = $autowireInlineAttribute->newInstance();
$dummy = $value;
while (null === $dummy->getClass() && $dummy instanceof ChildDefinition) {
$dummy = $this->container->findDefinition($dummy->getParent());
}

$methodCalls = $value->getMethodCalls();

$type = ProxyHelper::exportType($reflectionParameter, true);
$definition = $autowireInlineAttributeInstance->buildDefinition($autowireInlineAttributeInstance->value, $type, $reflectionParameter);
foreach ($methodCalls as $i => $call) {
[$method, $arguments] = $call;

$this->container->setDefinition('.autowire_inline.'.ContainerBuilder::hash($definition), $definition);
try {
$method = $this->getReflectionMethod($dummy, $method);
} catch (RuntimeException) {
continue;
}

$arguments = $this->registerAutowireInlineAttributes($method, $arguments, $isChildDefinition);

if ($arguments !== $call[1]) {
$methodCalls[$i][1] = $arguments;
}
}

if ($methodCalls !== $value->getMethodCalls()) {
$value->setMethodCalls($methodCalls);
}

return $value;
}

private function registerAutowireInlineAttributes(\ReflectionFunctionAbstract $method, array $arguments, bool $isChildDefinition): array
{
$parameters = $method->getParameters();

if ($method->isVariadic()) {
array_pop($parameters);
}
$dummyContainer = new ContainerBuilder($this->container->getParameterBag());

foreach ($parameters as $index => $parameter) {
if ($isChildDefinition) {
$index = 'index_'.$index;
}

$name = '$'.$parameter->name;
if (\array_key_exists($name, $arguments)) {
$arguments[$index] = $arguments[$name];
unset($arguments[$name]);
}
if (\array_key_exists($index, $arguments) && '' !== $arguments[$index]) {
continue;
}
if (!$attribute = $parameter->getAttributes(AutowireInline::class, \ReflectionAttribute::IS_INSTANCEOF)[0] ?? null) {
continue;
}

$type = ProxyHelper::exportType($parameter, true);

if (!$type && isset($arguments[$index])) {
continue;
}

$attribute = $attribute->newInstance();
$definition = $attribute->buildDefinition($attribute->value, $type, $parameter);

$dummyContainer->setDefinition('.autowire_inline', $definition);
(new ResolveParameterPlaceHoldersPass(false, false))->process($dummyContainer);

$id = '.autowire_inline.'.ContainerBuilder::hash([$this->currentId, $method->class ?? null, $method->name, (string) $parameter]);

$this->container->setDefinition($id, $definition);
$arguments[$index] = new Reference($id);

if ($definition->isAutowired()) {
$currentId = $this->currentId;
try {
$this->currentId = $id;
$this->processValue($definition, true);
} finally {
$this->currentId = $currentId;
}
}
}

return $arguments;
}
}
11 changes: 9 additions & 2 deletions 11 src/Symfony/Component/DependencyInjection/ContainerBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,9 @@ public function removeDefinition(string $id): void
{
if (isset($this->definitions[$id])) {
unset($this->definitions[$id]);
$this->removedIds[$id] = true;
if ('.' !== ($id[0] ?? '-')) {
Copy link
Member

Choose a reason for hiding this comment

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

I added this rule so that we don't use internal service ids in error messages. This matters to this PR because the added code creates new removed-ids, which means fixtures have to be updated with (what I consider as) noise.

$this->removedIds[$id] = true;
}
}
}

Expand Down Expand Up @@ -768,6 +770,9 @@ public function compile(bool $resolveEnvPlaceholders = false): void
parent::compile();

foreach ($this->definitions + $this->aliasDefinitions as $id => $definition) {
if ('.' === ($id[0] ?? '-')) {
continue;
}
if (!$definition->isPublic() || $definition->isPrivate()) {
$this->removedIds[$id] = true;
}
Expand Down Expand Up @@ -841,7 +846,9 @@ public function removeAlias(string $alias): void
{
if (isset($this->aliasDefinitions[$alias])) {
unset($this->aliasDefinitions[$alias]);
$this->removedIds[$alias] = true;
if ('.' !== ($alias[0] ?? '-')) {
$this->removedIds[$alias] = true;
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions 4 src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ class %s extends {$options['class']}
$preloadedFiles = [];
$ids = $this->container->getRemovedIds();
foreach ($this->container->getDefinitions() as $id => $definition) {
if (!$definition->isPublic()) {
if (!$definition->isPublic() && '.' !== ($id[0] ?? '-')) {
$ids[$id] = true;
}
}
Expand Down Expand Up @@ -1380,7 +1380,7 @@ private function addRemovedIds(): string
{
$ids = $this->container->getRemovedIds();
foreach ($this->container->getDefinitions() as $id => $definition) {
if (!$definition->isPublic()) {
if (!$definition->isPublic() && '.' !== ($id[0] ?? '-')) {
$ids[$id] = true;
}
}
Expand Down
25 changes: 16 additions & 9 deletions 25 .../Component/DependencyInjection/Tests/Compiler/ResolveAutowireInlineAttributesPassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use Symfony\Component\DependencyInjection\Compiler\AutowirePass;
use Symfony\Component\DependencyInjection\Compiler\ResolveAutowireInlineAttributesPass;
use Symfony\Component\DependencyInjection\Compiler\ResolveChildDefinitionsPass;
use Symfony\Component\DependencyInjection\Compiler\ResolveClassPass;
use Symfony\Component\DependencyInjection\Compiler\ResolveNamedArgumentsPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;

Expand All @@ -26,24 +25,32 @@ class ResolveAutowireInlineAttributesPassTest extends TestCase
public function testAttribute()
{
$container = new ContainerBuilder();
$container->register(Foo::class)->setAutowired(true);
$container->register(Foo::class, Foo::class)
->setAutowired(true);

$container->register('autowire_inline1', AutowireInlineAttributes1::class)
->setAutowired(true);

$container->register('autowire_inline2', AutowireInlineAttributes2::class)
->setArgument(1, 234)
->setAutowired(true);

$container->register('autowire_inline3', AutowireInlineAttributes3::class)
->setAutowired(true);

(new ResolveNamedArgumentsPass())->process($container);
(new ResolveClassPass())->process($container);
(new ResolveChildDefinitionsPass())->process($container);
(new ResolveAutowireInlineAttributesPass())->process($container);
(new ResolveChildDefinitionsPass())->process($container);
(new ResolveNamedArgumentsPass())->process($container);
(new AutowirePass())->process($container);

$autowireInlineAttributes1 = $container->get('autowire_inline1');
self::assertInstanceOf(AutowireInlineAttributes1::class, $autowireInlineAttributes1);
$a = $container->get('autowire_inline1');
self::assertInstanceOf(AutowireInlineAttributes1::class, $a);

$a = $container->get('autowire_inline2');
self::assertInstanceOf(AutowireInlineAttributes2::class, $a);

$autowireInlineAttributes2 = $container->get('autowire_inline2');
self::assertInstanceOf(AutowireInlineAttributes2::class, $autowireInlineAttributes2);
$a = $container->get('autowire_inline3');
self::assertInstanceOf(AutowireInlineAttributes2::class, $a->inlined);
self::assertSame(345, $a->inlined->bar);
}
}
20 changes: 18 additions & 2 deletions 20 src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes_80.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ class AutowireInlineAttributes1
{
public function __construct(
#[AutowireInline(AutowireInlineAttributesBar::class, [
'$foo' => Foo::class,
'$someString' => 'testString',
'$foo' => new Foo(),
])]
public AutowireInlineAttributesBar $inlined,
) {
Expand All @@ -176,9 +176,25 @@ class AutowireInlineAttributes2
{
public function __construct(
#[AutowireInline(AutowireInlineAttributesBar::class, [
'$someString' => 'testString',
new Foo(),
'testString',
])]
public AutowireInlineAttributesBar $inlined,
public int $bar,
) {
}
}

class AutowireInlineAttributes3
{
public function __construct(
#[AutowireInline(
parent: 'autowire_inline2',
arguments: [
'index_1' => 345,
],
)]
public AutowireInlineAttributes2 $inlined,
) {
}
}
1 change: 0 additions & 1 deletion 1 src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/lazy_autowire_attribute.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public function isCompiled(): bool
public function getRemovedIds(): array
{
return [
'.lazy.Symfony\\Component\\DependencyInjection\\Tests\\Compiler\\Foo' => true,
'Symfony\\Component\\DependencyInjection\\Tests\\Compiler\\Foo' => true,
];
}
Expand Down
7 changes: 0 additions & 7 deletions 7 ...nent/DependencyInjection/Tests/Fixtures/php/lazy_autowire_attribute_with_intersection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,6 @@ public function isCompiled(): bool
return true;
}

public function getRemovedIds(): array
{
return [
'.lazy.foo.qFdMZVK' => true,
];
}

protected function createProxy($class, \Closure $factory)
{
return $factory();
Expand Down
6 changes: 0 additions & 6 deletions 6 src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_deep_graph.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ public function isCompiled(): bool
return true;
}

public function getRemovedIds(): array
{
return [
];
}

/**
* Gets the public 'bar' shared service.
*
Expand Down
1 change: 0 additions & 1 deletion 1 ...mfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_duplicates.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ public function isCompiled(): bool
public function getRemovedIds(): array
{
return [
'.service_locator.lViPm9k' => true,
'foo' => true,
];
}
Expand Down
7 changes: 0 additions & 7 deletions 7 src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_rot13_env.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,6 @@ public function isCompiled(): bool
return true;
}

public function getRemovedIds(): array
{
return [
'.service_locator.DyWBOhJ' => true,
];
}

/**
* Gets the public 'Symfony\Component\DependencyInjection\Tests\Dumper\Rot13EnvVarProcessor' shared service.
*
Expand Down
1 change: 0 additions & 1 deletion 1 ...ny/Component/DependencyInjection/Tests/Fixtures/php/services_service_locator_argument.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ public function isCompiled(): bool
public function getRemovedIds(): array
{
return [
'.service_locator.X7o4UPP' => true,
'foo2' => true,
'foo3' => true,
'foo4' => true,
Expand Down
Loading
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.