Fix #16: Incorrect resolving of a named variadic parameter

yiisoft · Nov 29, 2021 · 76d43e8 · 76d43e8
1 parent de32eb8
commit 76d43e8
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 11 deletions.
diff --git a/src/ArrayDefinition.php b/src/ArrayDefinition.php
@@ -140,6 +140,7 @@ public function resolve(ContainerInterface $container): object
 
         $this->injectArguments($dependencies, $constructorArguments);
 
+        /** @psalm-suppress MixedArgumentTypeCoercion */
         $resolved = DefinitionResolver::resolveArray($container, $this->referenceContainer, $dependencies);
 
         /** @psalm-suppress MixedMethodCall */
@@ -166,7 +167,8 @@ public function resolve(ContainerInterface $container): object
     }
 
     /**
-     * @psalm-param array<string, DefinitionInterface> $dependencies
+     * @psalm-param array<string, ParameterDefinition> $dependencies
+     * @psalm-param-out array<array-key, Yiisoft\Definitions\ParameterDefinition|mixed> $dependencies
      *
      * @throws InvalidConfigException
      */
@@ -175,10 +177,10 @@ private function injectArguments(array &$dependencies, array $arguments): void
         $isIntegerIndexed = $this->isIntegerIndexed($arguments);
         $dependencyIndex = 0;
         $usedArguments = [];
-        $isVariadic = false;
+        $variadicKey = null;
         foreach ($dependencies as $key => &$value) {
-            if ($value instanceof ParameterDefinition && $value->isVariadic()) {
-                $isVariadic = true;
+            if ($value->isVariadic()) {
+                $variadicKey = $key;
             }
             $index = $isIntegerIndexed ? $dependencyIndex : $key;
             if (array_key_exists($index, $arguments)) {
@@ -188,15 +190,20 @@ private function injectArguments(array &$dependencies, array $arguments): void
             $dependencyIndex++;
         }
         unset($value);
-        if ($isVariadic) {
+        if ($variadicKey !== null) {
+            if (!$isIntegerIndexed && isset($arguments[$variadicKey]) && is_array($arguments[$variadicKey])) {
+                unset($dependencies[$variadicKey]);
+                $dependencies += $arguments[$variadicKey];
+                return;
+            }
+
             /** @var mixed $value */
             foreach ($arguments as $index => $value) {
                 if (!isset($usedArguments[$index])) {
                     $dependencies[$index] = DefinitionResolver::ensureResolvable($value);
                 }
             }
         }
-        /** @psalm-var array<string, DefinitionInterface> $dependencies */
     }
 
     /**

diff --git a/src/Helpers/DefinitionResolver.php b/src/Helpers/DefinitionResolver.php
@@ -21,13 +21,11 @@ final class DefinitionResolver
     /**
      * Resolves dependencies by replacing them with the actual object instances.
      *
-     * @param array $dependencies The dependencies.
-     *
+     * @param ContainerInterface $container Container to get dependencies from.
+     * @param ContainerInterface|null $referenceContainer Container to get references from.
      * @psalm-param array<string,mixed> $definitions Definitions to resolve.
      *
      * @return array The resolved dependencies.
-     *
-     * @psalm-return array<string,mixed>
      */
     public static function resolveArray(ContainerInterface $container, ?ContainerInterface $referenceContainer, array $definitions): array
     {

diff --git a/tests/Support/Phone.php b/tests/Support/Phone.php
@@ -17,7 +17,7 @@ final class Phone
     public bool $dev = false;
     public ?string $codeName = null;
 
-    public function __construct(?string $name = null, ?string $version = null, string ...$colors)
+    public function __construct(?string $name = null, ?string $version = null, ...$colors)
     {
         $this->name = $name;
         $this->version = $version;

diff --git a/tests/Unit/ArrayDefinitionTest.php b/tests/Unit/ArrayDefinitionTest.php
@@ -84,6 +84,47 @@ public function testConstructorWithVariadicAndIntKeys(): void
         self::assertSame($colors, $phone->getColors());
     }
 
+    public function testConstructorWithVariadicArrayAndIntKeys(): void
+    {
+        $container = new SimpleContainer();
+
+        $colors = ['red', 'green', 'blue'];
+        $definition = ArrayDefinition::fromConfig([
+            ArrayDefinition::CLASS_NAME => Phone::class,
+            ArrayDefinition::CONSTRUCTOR => [
+                null,
+                null,
+                $colors,
+            ],
+        ]);
+
+        /** @var Phone $phone */
+        $phone = $definition->resolve($container);
+
+
+        self::assertSame([$colors], $phone->getColors());
+    }
+
+    public function testConstructorWithVariadicAndNamedKeys(): void
+    {
+        $container = new SimpleContainer();
+
+        $colors = ['red', 'green', 'blue'];
+        $definition = ArrayDefinition::fromConfig([
+            ArrayDefinition::CLASS_NAME => Phone::class,
+            ArrayDefinition::CONSTRUCTOR => [
+                'name' => null,
+                'version' => null,
+                'colors' => $colors,
+            ],
+        ]);
+
+        /** @var Phone $phone */
+        $phone = $definition->resolve($container);
+
+        self::assertSame($colors, $phone->getColors());
+    }
+
     public function dataSetProperties(): array
     {
         return [