8000 feature #50718 [DependencyInjection] Improve reporting named autowiri… · symfony/symfony@6a65ebd · GitHub
[go: up one dir, main page]
Skip to content

Commit 6a65ebd

Browse files
fabpotfabpot
committed
feature #50718 [DependencyInjection] Improve reporting named autowiring aliases (nicolas-grekas)
This PR was merged into the 6.4 branch. Discussion ---------- [DependencyInjection] Improve reporting named autowiring aliases | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - This PR started as a fix of #48707 to improve the error message reported when `#[Target]` is used with an invalid target, and ended up with many other related improvements: - Allow using `#[Target]` with no arguments. This has the effect of throwing an exception if the name of the argument that has the attribute doesn't match a named autowiring alias. - Improve the error message when a target doesn't match: ![image](https://github.com/symfony/symfony/assets/243674/e93bb23d-67e5-4c6f-9972-2d388124a7d9) - Improve `debug:autowiring` to display the target name to use with `#[Target]`: ![image](https://github.com/symfony/symfony/assets/243674/951f5dae-9584-4cae-b60f-736afa824da0) Commits ------- 8d60a7e [DependencyInjection] Improve reporting named autowiring aliases
2 parents acd9e3b + 8d60a7e commit 6a65ebd

File tree

11 files changed

+140
-37
lines changed

11 files changed

+140
-37
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Command/DebugAutowiringCommand.php

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Symfony\Component\Console\Input\InputOption;
2222
use Symfony\Component\Console\Output\OutputInterface;
2323
use Symfony\Component\Console\Style\SymfonyStyle;
24+
use Symfony\Component\DependencyInjection\Attribute\Target;
2425
use Symfony\Component\HttpKernel\Debug\FileLinkFormatter;
2526

2627
/**
@@ -86,6 +87,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int
8687
}
8788
}
8889

90+
$reverseAliases = [];
91+
92+
foreach ($container->getAliases() as $id => $alias) {
93+
if ('.' === ($id[0] ?? null)) {
94+
$reverseAliases[(string) $alias][] = $id;
95+
}
96+
}
97+
8998
uasort($serviceIds, 'strnatcmp');
9099

91100
$io->title('Autowirable Types');
@@ -103,30 +112,47 @@ protected function execute(InputInterface $input, OutputInterface $output): int
103112
}
104113
$text = [];
105114
$resolvedServiceId = $serviceId;
106-
if (!str_starts_with($serviceId, $previousId)) {
115+
if (!str_starts_with($serviceId, $previousId.' $')) {
107116
$text[] = '';
108-
if ('' !== $description = Descriptor::getClassDescription($serviceId, $resolvedServiceId)) {
109-
if (isset($hasAlias[$serviceId])) {
117+
$previousId = preg_replace('/ \$.*/', '', $serviceId);
118+
if ('' !== $description = Descriptor::getClassDescription($previousId, $resolvedServiceId)) {
119+
if (isset($hasAlias[$previousId])) {
110120
continue;
111121
}
112122
$text[] = $description;
113123
}
114-
$previousId = $serviceId.' $';
115124
}
116125

117126
$serviceLine = sprintf('<fg=yellow>%s</>', $serviceId);
118-
if ($this->supportsHref && '' !== $fileLink = $this->getFileLink($serviceId)) {
119-
$serviceLine = sprintf('<fg=yellow;href=%s>%s</>', $fileLink, $serviceId);
127+
if ($this->supportsHref && '' !== $fileLink = $this->getFileLink($previousId)) {
128+
$serviceLine = substr($serviceId, \strlen($previousId));
129+
$serviceLine = sprintf('<fg=yellow;href=%s>%s</>', $fileLink, $previousId).('' !== $serviceLine ? sprintf('<fg=yellow>%s</>', $serviceLine) : '');
120130
}
121131

122132
if ($container->hasAlias($serviceId)) {
123133
$hasAlias[$serviceId] = true;
124134
$serviceAlias = $container->getAlias($serviceId);
135+
$alias = (string) $serviceAlias;
136+
137+
$target = null;
138+
foreach ($reverseAliases[(string) $serviceAlias] ?? [] as $id) {
139+
if (!str_starts_with($id, '.'.$previousId.' $')) {
140+
continue;
141+
}
142+
$target = substr($id, \strlen($previousId) + 3);
143+
144+
if ($previousId.' $'.(new Target($target))->getParsedName() === $serviceId) {
145+
$serviceLine .= ' - <fg=magenta>target:</><fg=cyan>'.$target.'</>';
146+
break;
147+
}
148+
}
125149

126150
if ($container->hasDefinition($serviceAlias) && $decorated = $container->getDefinition($serviceAlias)->getTag('container.decorator')) {
127-
$serviceLine .= ' <fg=cyan>('.$decorated[0]['id'].')</>';
128-
} else {
129-
$serviceLine .= ' <fg=cyan>('.$serviceAlias.')</>';
151+
$alias = $decorated[0]['id'];
152+
}
153+
154+
if ($alias !== $target) {
155+
$serviceLine .= ' - <fg=magenta>alias:</><fg=cyan>'.$alias.'</>';
130156
}
131157

132158
if ($serviceAlias->isDeprecated()) {

‎src/Symfony/Bundle/FrameworkBundle/Tests/Functional/DebugAutowiringCommandTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function testBasicFunctionality()
3636
$tester->run(['command' => 'debug:autowiring']);
3737

3838
$this->assertStringContainsString(HttpKernelInterface::class, $tester->getDisplay());
39-
$this->assertStringContainsString('(http_kernel)', $tester->getDisplay());
39+
$this->assertStringContainsString('alias:http_kernel', $tester->getDisplay());
4040
}
4141

4242
public function testSearchArgument()

‎src/Symfony/Component/DependencyInjection/Attribute/Target.php

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\DependencyInjection\Attribute;
1313

1414
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
15+
use Symfony\Component\DependencyInjection\Exception\LogicException;
1516

1617
/**
1718
* An attribute to tell how a dependency is used and hint named autowiring aliases.
@@ -21,11 +22,18 @@
2122
#[\Attribute(\Attribute::TARGET_PARAMETER)]
2223
final class Target
2324
{
24-
public string $name;
25+
public function __construct(
26+
public ?string $name = null,
27+
) {
28+
}
2529

26-
public function __construct(string $name)
30+
public function getParsedName(): string
2731
{
28-
$this->name = lcfirst(str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $name))));
32+
if (null === $this->name) {
33+
throw new LogicException(sprintf('Cannot parse the name of a #[Target] attribute that has not been resolved. Did you forget to call "%s::parseName()"?', __CLASS__));
34+
}
35+
36+
return lcfirst(str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $this->name))));
2937
}
3038

3139
public static function parseName(\ReflectionParameter $parameter, self &$attribute = null): string
@@ -36,9 +44,10 @@ public static function parseName(\ReflectionParameter $parameter, self &$attribu
3644
}
3745

3846
$attribute = $target->newInstance();
39-
$name = $attribute->name;
47+
$name = $attribute->name ??= $parameter->name;
48+
$parsedName = $attribute->getParsedName();
4049

41-
if (!preg_match('/^[a-zA-Z_\x7f-\xff]/', $name)) {
50+
if (!preg_match('/^[a-zA-Z_\x7f-\xff]/', $parsedName)) {
4251
if (($function = $parameter->getDeclaringFunction()) instanceof \ReflectionMethod) {
4352
$function = $function->class.'::'.$function->name;
4453
} else {
@@ -48,6 +57,6 @@ public static function parseName(\ReflectionParameter $parameter, self &$attribu
4857
throw new InvalidArgumentException(sprintf('Invalid #[Target] name "%s" on parameter "$%s" of "%s()": the first character must be a letter.', $name, $parameter->name, $function));
4958
}
5059

51-
return $name;
60+
return $parsedName;
5261
}
5362
}

‎src/Symfony/Component/DependencyInjection/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ CHANGELOG
44
6.4
55
---
66

7+
* Allow using `#[Target]` with no arguments to state that a parameter must match a named autowiring alias
78
* Deprecate `ContainerAwareInterface` and `ContainerAwareTrait`, use dependency injection instead
89
* Add `defined` env var processor that returns `true` for defined and neither null nor empty env vars
910

‎src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -450,14 +450,16 @@ private function getAutowiredReference(TypedReference $reference, bool $filterTy
450450
$type = implode($m[0], $types);
451451
}
452452

453-
$name = (array_filter($reference->getAttributes(), static fn ($a) => $a instanceof Target)[0] ?? null)?->name;
453+
$name = $target = (array_filter($reference->getAttributes(), static fn ($a) => $a instanceof Target)[0] ?? null)?->name;
454454

455455
if (null !== $name ??= $reference->getName()) {
456-
if ($this->container->has($alias = $type.' $'.$name) && !$this->container->findDefinition($alias)->isAbstract()) {
456+
$parsedName = (new Target($name))->getParsedName();
457+
458+
if ($this->container->has($alias = $type.' $'.$parsedName) && !$this->container->findDefinition($alias)->isAbstract()) {
457459
return new TypedReference($alias, $type, $reference->getInvalidBehavior());
458460
}
459461

460-
if (null !== ($alias = $this->getCombinedAlias($type, $name) ?? null) && !$this->container->findDefinition($alias)->isAbstract()) {
462+
if (null !== ($alias = $this->getCombinedAlias($type, $parsedName) ?? null) && !$this->container->findDefinition($alias)->isAbstract()) {
461463
return new TypedReference($alias, $type, $reference->getInvalidBehavior());
462464
}
463465

@@ -469,7 +471,7 @@ private function getAutowiredReference(TypedReference $reference, bool $filterTy
469471
}
470472
}
471473

472-
if ($reference->getAttributes()) {
474+
if (null !== $target) {
473475
return null;
474476
}
475477
}
@@ -498,8 +500,10 @@ private function populateAvailableTypes(ContainerBuilder $container): void
498500
$this->populateAvailableType($container, $id, $definition);
499501
}
500502

503+
$prev = null;
501504
foreach ($container->getAliases() as $id => $alias) {
502-
$this->populateAutowiringAlias($id);
505+
$this->populateAutowiringAlias($id, $prev);
506+
$prev = $id;
503507
}
504508
}
505509

@@ -598,13 +602,16 @@ private function createTypeNotFoundMessage(TypedReference $reference, string $la
598602
}
599603

600604
$message = sprintf('has type "%s" but this class %s.', $type, $parentMsg ?: 'was not found');
601-
} elseif ($reference->getAttributes()) {
602-
$message = $label;
603-
$label = sprintf('"#[Target(\'%s\')" on', $reference->getName());
604605
} else {
605606
$alternatives = $this->createTypeAlternatives($this->container, $reference);
606-
$message = $this->container->has($type) ? 'this service is abstract' : 'no such service exists';
607-
$message = sprintf('references %s "%s" but %s.%s', $r->isInterface() ? 'interface' : 'class', $type, $message, $alternatives);
607+
608+
if (null !== $target = (array_filter($reference->getAttributes(), static fn ($a) => $a instanceof Target)[0] ?? null)) {
609+
$target = null !== $target->name ? "('{$target->name}')" : '';
610+
$message = sprintf('has "#[Target%s]" but no such target exists.%s', $target, $alternatives);
611+
} else {
612+
$message = $this->container->has($type) ? 'this service is abstract' : 'no such service exists';
613+
$message = sprintf('references %s "%s" but %s.%s', $r->isInterface() ? 'interface' : 'class', $type, $message, $alternatives);
614+
}
608615

609616
if ($r->isInterface() && !$alternatives) {
610617
$message .= ' Did you create a class that implements this interface?';
@@ -632,8 +639,11 @@ private function createTypeAlternatives(ContainerBuilder $container, TypedRefere
632639
}
633640

634641
$servicesAndAliases = $container->getServiceIds();
635-
if (null !== ($autowiringAliases = $this->autowiringAliases[$type] ?? null) && !isset($autowiringAliases[''])) {
636-
return sprintf(' Available autowiring aliases for this %s are: "$%s".', class_exists($type, false) ? 'class' : 'interface', implode('", "$', $autowiringAliases));
642+
$autowiringAliases = $this->autowiringAliases[$type] ?? [];
643+
unset($autowiringAliases['']);
644+
645+
if ($autowiringAliases) {
646+
return sprintf(' Did you mean to target%s "%s" instead?', 1 < \count($autowiringAliases) ? ' one of' : '', implode('", "', $autowiringAliases));
637647
}
638648

639649
if (!$container->has($type) && false !== $key = array_search(strtolower($type), array_map('strtolower', $servicesAndAliases))) {
@@ -675,7 +685,7 @@ private function getAliasesSuggestionForType(ContainerBuilder $container, string
675685
return null;
676686
}
677687

678-
private function populateAutowiringAlias(string $id): void
688+
private function populateAutowiringAlias(string $id, string $target = null): void
679689
{
680690
if (!preg_match('/(?(DEFINE)(?<V>[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+))^((?&V)(?:\\\\(?&V))*+)(?: \$((?&V)))?$/', $id, $m)) {
681691
return;
@@ -685,6 +695,12 @@ private function populateAutowiringAlias(string $id): void
685695
$name = $m[3] ?? '';
686696

687697
if (class_exists($type, false) || interface_exists($type, false)) {
698+
if (null !== $target && str_starts_with($target, '.'.$type.' $')
699+
&& (new Target($target = substr($target, \strlen($type) + 3)))->getParsedName() === $name
700+
) {
701+
$name = $target;
702+
}
703+
688704
$this->autowiringAliases[$type][$name] = $name;
689705
}
690706
}

‎src/Symfony/Component/DependencyInjection/ContainerBuilder.php

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,13 +1382,21 @@ public function registerAttributeForAutoconfiguration(string $attributeClass, ca
13821382
*/
13831383
public function registerAliasForArgument(string $id, string $type, string $name = null): Alias
13841384
{
1385-
$name = (new Target($name ?? $id))->name;
1385+
$parsedName = (new Target($name ??= $id))->getParsedName();
13861386

1387-
if (!preg_match('/^[a-zA-Z_\x7f-\xff]/', $name)) {
1388-
throw new InvalidArgumentException(sprintf('Invalid argument name "%s" for service "%s": the first character must be a letter.', $name, $id));
1387+
if (!preg_match('/^[a-zA-Z_\x7f-\xff]/', $parsedName)) {
1388+
if ($id !== $name) {
1389+
$id = sprintf(' for service "%s"', $id);
1390+
}
1391+
1392+
throw new InvalidArgumentException(sprintf('Invalid argument name "%s"'.$id.': the first character must be a letter.', $name));
1393+
}
1394+
1395+
if ($parsedName !== $name) {
1396+
$this->setAlias('.'.$type.' $'.$name, $type.' $'.$parsedName);
13891397
}
13901398

1391-
return $this->setAlias($type.' $'.$name, $id);
1399+
return $this->setAlias($type.' $'.$parsedName, $id);
13921400
}
13931401

13941402
/**

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass;
3737
use Symfony\Component\DependencyInjection\Tests\Fixtures\includes\FooVariadic;
3838
use Symfony\Component\DependencyInjection\Tests\Fixtures\WithTarget;
39+
use Symfony\Component\DependencyInjection\Tests\Fixtures\WithTargetAnonymous;
3940
use Symfony\Component\DependencyInjection\TypedReference;
4041
use Symfony\Component\ExpressionLanguage\Expression;
4142

@@ -1240,12 +1241,27 @@ public function testArgumentWithTypoTarget()
12401241
$container = new ContainerBuilder();
12411242

12421243
$container->register(BarInterface::class, BarInterface::class);
1243-
$container->register(BarInterface::class.' $iamgeStorage', BarInterface::class);
1244+
$container->registerAliasForArgument('images.storage', BarInterface::class);
12441245
$container->register('with_target', WithTarget::class)
12451246
->setAutowired(true);
12461247

12471248
$this->expectException(AutowiringFailedException::class);
1248-
$this->expectExceptionMessage('Cannot autowire service "with_target": "#[Target(\'imageStorage\')" on argument "$bar" of method "Symfony\Component\DependencyInjection\Tests\Fixtures\WithTarget::__construct()"');
1249+
$this->expectExceptionMessage('Cannot autowire service "with_target": argument "$bar" of method "Symfony\Component\DependencyInjection\Tests\Fixtures\WithTarget::__construct()" has "#[Target(\'image.storage\')]" but no such target exists. Did you mean to target "images.storage" instead?');
1250+
1251+
(new AutowirePass())->process($container);
1252+
}
1253+
1254+
public function testArgumentWithTypoTargetAnonymous()
1255+
{
1256+
$container = new ContainerBuilder();
1257+
1258+
$container->register(BarInterface::class, BarInterface::class);
1259+
$container->registerAliasForArgument('bar', BarInterface::class);
1260+
$container->register('with_target', WithTargetAnonymous::class)
1261+
->setAutowired(true);
1262+
1263+
$this->expectException(AutowiringFailedException::class);
1264+
$this->expectExceptionMessage('Cannot autowire service "with_target": argument "$baz" of method "Symfony\Component\DependencyInjection\Tests\Fixtures\WithTargetAnonymous::__construct()" has "#[Target(\'baz\')]" but no such target exists. Did you mean to target "bar" instead?');
12491265

12501266
(new AutowirePass())->process($container);
12511267
}

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,8 +402,8 @@ public static function getSubscribedServices(): array
402402
(new AutowirePass())->process($container);
403403

404404
$expected = [
405-
'some.service' => new ServiceClosureArgument(new TypedReference('some.service', 'stdClass')),
406-
'some_service' => new ServiceClosureArgument(new TypedReference('stdClass $some_service', 'stdClass')),
405+
'some.service' => new ServiceClosureArgument(new TypedReference('stdClass $someService', 'stdClass')),
406+
'some_service' => new ServiceClosureArgument(new TypedReference('stdClass $someService', 'stdClass')),
407407
'another_service' => new ServiceClosureArgument(new TypedReference('stdClass $anotherService', 'stdClass')),
408408
];
409409
$this->assertEquals($expected, $container->getDefinition((string) $locator->getFactory()[0])->getArgument(0));

‎src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1662,9 +1662,11 @@ public function testRegisterAliasForArgument()
16621662

16631663
$container->registerAliasForArgument('Foo.bar_baz', 'Some\FooInterface');
16641664
$this->assertEquals(new Alias('Foo.bar_baz'), $container->getAlias('Some\FooInterface $fooBarBaz'));
1665+
$this->assertEquals(new Alias('Some\FooInterface $fooBarBaz'), $container->getAlias('.Some\FooInterface $Foo.bar_baz'));
16651666

16661667
$container->registerAliasForArgument('Foo.bar_baz', 'Some\FooInterface', 'Bar_baz.foo');
16671668
$this->assertEquals(new Alias('Foo.bar_baz'), $container->getAlias('Some\FooInterface $barBazFoo'));
1669+
$this->assertEquals(new Alias('Some\FooInterface $barBazFoo'), $container->getAlias('.Some\FooInterface $Bar_baz.foo'));
16681670
}
16691671

16701672
public function testCaseSensitivity()

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/WithTargetAnonymous.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
13+
14+
use Symfony\Component\DependencyInjection\Attribute\Target;
15+
16+
class WithTargetAnonymous
17+
{
18+
public function __construct(
19+
#[Target]
20+
BarInterface $baz
21+
) {
22+
}
23+
}

‎src/Symfony/Component/Workflow/WorkflowInterface.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
use Symfony\Component\Workflow\Metadata\MetadataStoreInterface;
1717

1818
/**
19+
* Describes a workflow instance.
20+
*
1921
* @author Amrouche Hamza <hamza.simperfit@gmail.com>
2022
*/
2123
interface WorkflowInterface

0 commit comments

Comments
 (0)
0