[DependencyInjection] Fix parsing named autowiring aliases that conta…

…in underscores
symfony · Nov 30, 2023 · 69a115c · 69a115c
1 parent e3e3d5f
commit 69a115c
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 14 deletions.
diff --git a/src/Symfony/Component/DependencyInjection/Attribute/Target.php b/src/Symfony/Component/DependencyInjection/Attribute/Target.php
@@ -36,10 +36,12 @@ public function getParsedName(): string
         return lcfirst(str_replace(' ', '', ucwords(preg_replace('/[^a-zA-Z0-9\x7f-\xff]++/', ' ', $this->name))));
     }
 
-    public static function parseName(\ReflectionParameter $parameter, self &$attribute = null): string
+    public static function parseName(\ReflectionParameter $parameter, self &$attribute = null, string &$parsedName = null): string
     {
         $attribute = null;
         if (!$target = $parameter->getAttributes(self::class)[0] ?? null) {
+            $parsedName = (new self($parameter->name))->getParsedName();
+
             return $parameter->name;
         }
 
@@ -57,6 +59,6 @@ public static function parseName(\ReflectionParameter $parameter, self &$attribu
             throw new InvalidArgumentException(sprintf('Invalid #[Target] name "%s" on parameter "$%s" of "%s()": the first character must be a letter.', $name, $parameter->name, $function));
         }
 
-        return $parsedName;
+        return preg_match('/^[a-zA-Z0-9_\x7f-\xff]++$/', $name) ? $name : $parsedName;
     }
 }
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
@@ -454,20 +454,30 @@ private function getAutowiredReference(TypedReference $reference, bool $filterTy
         $name = $target = (array_filter($reference->getAttributes(), static fn ($a) => $a instanceof Target)[0] ?? null)?->name;
 
         if (null !== $name ??= $reference->getName()) {
+            if ($this->container->has($alias = $type.' $'.$name) && !$this->container->findDefinition($alias)->isAbstract()) {
+                return new TypedReference($alias, $type, $reference->getInvalidBehavior());
+            }
+
+            if (null !== ($alias = $this->getCombinedAlias($type, $name)) && !$this->container->findDefinition($alias)->isAbstract()) {
+                return new TypedReference($alias, $type, $reference->getInvalidBehavior());
+            }
+
             $parsedName = (new Target($name))->getParsedName();
 
             if ($this->container->has($alias = $type.' $'.$parsedName) && !$this->container->findDefinition($alias)->isAbstract()) {
                 return new TypedReference($alias, $type, $reference->getInvalidBehavior());
             }
 
-            if (null !== ($alias = $this->getCombinedAlias($type, $parsedName) ?? null) && !$this->container->findDefinition($alias)->isAbstract()) {
+            if (null !== ($alias = $this->getCombinedAlias($type, $parsedName)) && !$this->container->findDefinition($alias)->isAbstract()) {
                 return new TypedReference($alias, $type, $reference->getInvalidBehavior());
             }
 
-            if ($this->container->has($name) && !$this->container->findDefinition($name)->isAbstract()) {
+            if (($this->container->has($n = $name) && !$this->container->findDefinition($n)->isAbstract())
+                || ($this->container->has($n = $parsedName) && !$this->container->findDefinition($n)->isAbstract())
+            ) {
                 foreach ($this->container->getAliases() as $id => $alias) {
-                    if ($name === (string) $alias && str_starts_with($id, $type.' $')) {
-                        return new TypedReference($name, $type, $reference->getInvalidBehavior());
+                    if ($n === (string) $alias && str_starts_with($id, $type.' $')) {
+                        return new TypedReference($n, $type, $reference->getInvalidBehavior());
                     }
                 }
             }
@@ -481,7 +491,7 @@ private function getAutowiredReference(TypedReference $reference, bool $filterTy
             return new TypedReference($type, $type, $reference->getInvalidBehavior());
         }
 
-        if (null !== ($alias = $this->getCombinedAlias($type) ?? null) && !$this->container->findDefinition($alias)->isAbstract()) {
+        if (null !== ($alias = $this->getCombinedAlias($type)) && !$this->container->findDefinition($alias)->isAbstract()) {
             return new TypedReference($alias, $type, $reference->getInvalidBehavior());
         }
 

diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php
@@ -190,16 +190,19 @@ protected function processValue(mixed $value, bool $isRoot = false): mixed
 
                 $typeHint = ltrim(ProxyHelper::exportType($parameter) ?? '', '?');
 
-                $name = Target::parseName($parameter);
+                $name = Target::parseName($parameter, parsedName: $parsedName);
 
-                if ($typeHint && \array_key_exists($k = preg_replace('/(^|[(|&])\\\\/', '\1', $typeHint).' $'.$name, $bindings)) {
+                if ($typeHint && (
+                    \array_key_exists($k = preg_replace('/(^|[(|&])\\\\/', '\1', $typeHint).' $'.$name, $bindings)
+                    || \array_key_exists($k = preg_replace('/(^|[(|&])\\\\/', '\1', $typeHint).' $'.$parsedName, $bindings)
+                )) {
                     $arguments[$key] = $this->getBindingValue($bindings[$k]);
 
                     continue;
                 }
 
-                if (\array_key_exists('$'.$name, $bindings)) {
-                    $arguments[$key] = $this->getBindingValue($bindings['$'.$name]);
+                if (\array_key_exists($k = '$'.$name, $bindings) || \array_key_exists($k = '$'.$parsedName, $bindings)) {
+                    $arguments[$key] = $this->getBindingValue($bindings[$k]);
 
                     continue;
                 }
@@ -210,7 +213,7 @@ protected function processValue(mixed $value, bool $isRoot = false): mixed
                     continue;
                 }
 
-                if (isset($bindingNames[$name]) || isset($bindingNames[$parameter->name])) {
+                if (isset($bindingNames[$name]) || isset($bindingNames[$parsedName]) || isset($bindingNames[$parameter->name])) {
                     $bindingKey = array_search($binding, $bindings, true);
                     $argumentType = substr($bindingKey, 0, strpos($bindingKey, ' '));
                     $this->errorMessages[] = sprintf('Did you forget to add the type "%s" to argument "$%s" of method "%s::%s()"?', $argumentType, $parameter->name, $reflectionMethod->class, $reflectionMethod->name);

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
@@ -1301,6 +1301,18 @@ public function testAutowireWithNamedArgs()
         $this->assertEquals([new TypedReference(A::class, A::class), 'abc'], $container->getDefinition('foo')->getArguments());
     }
 
+    public function testAutowireUnderscoreNamedArgument()
+    {
+        $container = new ContainerBuilder();
+
+        $container->autowire(\DateTimeImmutable::class.' $now_datetime', \DateTimeImmutable::class);
+        $container->autowire('foo', UnderscoreNamedArgument::class)->setPublic(true);
+
+        (new AutowirePass())->process($container);
+
+        $this->assertInstanceOf(\DateTimeImmutable::class, $container->get('foo')->now_datetime);
+    }
+
     public function testAutowireDefaultValueParametersLike()
     {
         $container = new ContainerBuilder();

diff --git a/...mfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php b/...mfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php
@@ -403,7 +403,7 @@ public static function getSubscribedServices(): array
 
         $expected = [
             'some.service' => new ServiceClosureArgument(new TypedReference('stdClass $someService', 'stdClass')),
-            'some_service' => new ServiceClosureArgument(new TypedReference('stdClass $someService', 'stdClass')),
+            'some_service' => new ServiceClosureArgument(new TypedReference('stdClass $some_service', 'stdClass')),
             'another_service' => new ServiceClosureArgument(new TypedReference('stdClass $anotherService', 'stdClass')),
         ];
         $this->assertEquals($expected, $container->getDefinition((string) $locator->getFactory()[0])->getArgument(0));

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php
@@ -227,6 +227,14 @@ public function __construct(A $a, Lille $lille, $foo = 'some_val')
     }
 }
 
+class UnderscoreNamedArgument
+{
+    public function __construct(
+        public \DateTimeImmutable $now_datetime,
+    ) {
+    }
+}
+
 /*
  * Classes used for testing createResourceForClass
  */

diff --git a/...mfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php b/...mfony/Component/HttpKernel/DependencyInjection/RegisterControllerArgumentLocatorsPass.php
@@ -128,6 +128,8 @@ public function process(ContainerBuilder $container)
                     $type = preg_replace('/(^|[(|&])\\\\/', '\1', $target = ltrim(ProxyHelper::exportType($p) ?? '', '?'));
                     $invalidBehavior = ContainerInterface::IGNORE_ON_INVALID_REFERENCE;
                     $autowireAttributes = $autowire ? $emptyAutowireAttributes : [];
+                    $parsedName = $p->name;
+                    $k = null;
 
                     if (isset($arguments[$r->name][$p->name])) {
                         $target = $arguments[$r->name][$p->name];
@@ -138,7 +140,11 @@ public function process(ContainerBuilder $container)
                         } elseif ($p->allowsNull() && !$p->isOptional()) {
                             $invalidBehavior = ContainerInterface::NULL_ON_INVALID_REFERENCE;
                         }
-                    } elseif (isset($bindings[$bindingName = $type.' $'.$name = Target::parseName($p)]) || isset($bindings[$bindingName = '$'.$name]) || isset($bindings[$bindingName = $type])) {
+                    } elseif (isset($bindings[$bindingName = $type.' $'.$name = Target::parseName($p, $k, $parsedName)])
+                        || isset($bindings[$bindingName = $type.' $'.$parsedName])
+                        || isset($bindings[$bindingName = '$'.$name])
+                        || isset($bindings[$bindingName = $type])
+                    ) {
                         $binding = $bindings[$bindingName];
 
                         [$bindingValue, $bindingId, , $bindingType, $bindingFile] = $binding->getValues();