Test coverage 100% + minor refactoring (#27)

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

Hide file tree

Showing 13 changed files with 352 additions and 18 deletions.
diff --git a/composer.json b/composer.json
@@ -31,9 +31,9 @@
     },
     "require-dev": {
         "phpunit/phpunit": "^9.5",
-        "roave/infection-static-analysis-plugin": "^1.9",
+        "roave/infection-static-analysis-plugin": "^1.11",
         "spatie/phpunit-watcher": "^1.23",
-        "vimeo/psalm": "^4.11",
+        "vimeo/psalm": "^4.13",
         "yiisoft/test-support": "^1.3"
     },
     "autoload": {

diff --git a/src/ArrayDefinition.php b/src/ArrayDefinition.php
@@ -7,10 +7,13 @@
 use Psr\Container\ContainerInterface;
 use Yiisoft\Definitions\Contract\DefinitionInterface;
 use Yiisoft\Definitions\Exception\InvalidConfigException;
-
 use Yiisoft\Definitions\Helpers\DefinitionExtractor;
 use Yiisoft\Definitions\Helpers\DefinitionResolver;
+
+use function array_key_exists;
+use function call_user_func_array;
 use function count;
+use function is_string;
 
 /**
  * Builds an object by array config.

diff --git a/src/Helpers/DefinitionResolver.php b/src/Helpers/DefinitionResolver.php
@@ -61,7 +61,9 @@ public static function resolveArray(ContainerInterface $container, ?ContainerInt
     public static function resolve(ContainerInterface $container, ?ContainerInterface $referenceContainer, $definition)
     {
         if ($definition instanceof DefinitionInterface) {
-            $container = $referenceContainer !== null && $definition instanceof ReferenceInterface ? $referenceContainer : $container;
+            $container = $referenceContainer !== null && $definition instanceof ReferenceInterface
+                ? $referenceContainer
+                : $container;
             /** @var mixed $definition */
             $definition = $definition->resolve($container);
         } elseif (is_array($definition)) {

diff --git a/src/ParameterDefinition.php b/src/ParameterDefinition.php
@@ -62,7 +62,7 @@ public function resolve(ContainerInterface $container)
         $type = $this->parameter->getType();
 
         if ($type === null || $this->isVariadic()) {
-            return $this->resolveBuiltin();
+            return $this->resolveVariadicOrBuiltinOrNonTyped();
         }
 
         if ($this->isUnionType()) {
@@ -98,13 +98,13 @@ public function resolve(ContainerInterface $container)
             return $result;
         }
 
-        return $this->resolveBuiltin();
+        return $this->resolveVariadicOrBuiltinOrNonTyped();
     }
 
     /**
      * @return mixed
      */
-    private function resolveBuiltin()
+    private function resolveVariadicOrBuiltinOrNonTyped()
     {
         if ($this->parameter->isDefaultValueAvailable()) {
             return $this->parameter->getDefaultValue();
@@ -121,12 +121,25 @@ private function resolveBuiltin()
             );
         }
 
+        $type = $this->getType();
+
+        if ($type === null) {
+            throw new NotInstantiableException(
+                sprintf(
+                    'Can not determine value of the "%s" parameter without type when instantiating "%s". ' .
+                    'Please specify argument explicitly.',
+                    $this->parameter->getName(),
+                    $this->getCallable(),
+                )
+            );
+        }
+
         throw new NotInstantiableException(
             sprintf(
                 'Can not determine value of the "%s" parameter of type "%s" when instantiating "%s". ' .
                 'Please specify argument explicitly.',
                 $this->parameter->getName(),
-                $this->getType(),
+                $type,
                 $this->getCallable(),
             )
         );
@@ -188,7 +201,7 @@ private function resolveUnionType(ContainerInterface $container)
         }
 
         if (!isset($error)) {
-            return $this->resolveBuiltin();
+            return $this->resolveVariadicOrBuiltinOrNonTyped();
         }
 
         throw $error;
@@ -200,14 +213,8 @@ private function isUnionType(): bool
         return $this->parameter->getType() instanceof ReflectionUnionType;
     }
 
-    private function getType(): string
+    private function getType(): ?string
     {
-        /**
-         * @psalm-suppress UndefinedDocblockClass
-         *
-         * @var ReflectionNamedType|ReflectionUnionType $type Could not be `null`
-         * because in self::resolve() checked `$this->parameter->allowsNull()`.
-         */
         $type = $this->parameter->getType();
 
         /** @psalm-suppress UndefinedClass, TypeDoesNotContainType */
@@ -221,9 +228,11 @@ private function getType(): string
             return implode('|', $names);
         }
 
-        /** @var ReflectionNamedType $type */
+        if ($type instanceof ReflectionNamedType) {
+            return $type->getName();
+        }
 
-        return $type->getName();
+        return null;
     }
 
     private function getCallable(): string

diff --git a/tests/Php8/DefinitionStorageTest.php b/tests/Php8/DefinitionStorageTest.php
@@ -0,0 +1,51 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Definitions\Tests\Php8;
+
+use PHPUnit\Framework\TestCase;
+use RuntimeException;
+use Yiisoft\Definitions\DefinitionStorage;
+use Yiisoft\Definitions\Tests\Support\ColorInterface;
+use Yiisoft\Definitions\Tests\Support\ColorPink;
+use Yiisoft\Definitions\Tests\Support\UnionCar;
+use Yiisoft\Definitions\Tests\Support\UnionSelfDependency;
+use Yiisoft\Test\Support\Container\SimpleContainer;
+
+final class DefinitionStorageTest extends TestCase
+{
+    public function testUnresolvableUnionSelfDependency(): void
+    {
+        $storage = new DefinitionStorage();
+
+        $this->expectException(RuntimeException::class);
+        $this->expectExceptionMessage(
+            'Service ' . UnionSelfDependency::class . ' doesn\'t exist in DefinitionStorage.'
+        );
+        $storage->get(UnionSelfDependency::class);
+    }
+
+    public function testResolvableUnionTypeDependency(): void
+    {
+        $storage = new DefinitionStorage();
+
+        $definition = $storage->get(UnionCar::class);
+
+        $this->assertSame(UnionCar::class, $definition);
+    }
+
+    public function testResolvableFromContainerUnionTypeDependency(): void
+    {
+        $storage = new DefinitionStorage();
+
+        $container = new SimpleContainer([
+            ColorInterface::class => new ColorPink(),
+        ]);
+        $storage->setDelegateContainer($container);
+
+        $definition = $storage->get(UnionSelfDependency::class);
+
+        $this->assertSame(UnionSelfDependency::class, $definition);
+    }
+}
diff --git a/tests/Php8/ParameterDefinitionTest.php b/tests/Php8/ParameterDefinitionTest.php
@@ -12,9 +12,14 @@
 use ReflectionParameter;
 use stdClass;
 use Yiisoft\Definitions\Exception\InvalidConfigException;
+use Yiisoft\Definitions\Exception\NotInstantiableException;
 use Yiisoft\Definitions\ParameterDefinition;
 use Yiisoft\Definitions\Tests\Support\GearBox;
+use Yiisoft\Definitions\Tests\Support\UnionBuiltinDependency;
 use Yiisoft\Definitions\Tests\Support\UnionCar;
+use Yiisoft\Definitions\Tests\Support\UnionOptionalDependency;
+use Yiisoft\Definitions\Tests\Support\UnionSelfDependency;
+use Yiisoft\Test\Support\Container\Exception\NotFoundException;
 use Yiisoft\Test\Support\Container\SimpleContainer;
 
 final class ParameterDefinitionTest extends TestCase
@@ -67,6 +72,41 @@ public function testResolveOptionalUnionTypeWithIncorrectTypeInContainer(): void
         $this->assertNull($result);
     }
 
+    public function testResolveOptionalUnionType(): void
+    {
+        $definition = new ParameterDefinition(
+            $this->getFirstConstructorParameter(UnionOptionalDependency::class)
+        );
+        $container = new SimpleContainer();
+
+        $this->assertNull($definition->resolve($container));
+    }
+
+    public function testResolveUnionBuiltin(): void
+    {
+        $definition = new ParameterDefinition(
+            $this->getFirstConstructorParameter(UnionBuiltinDependency::class)
+        );
+        $container = new SimpleContainer();
+
+        $this->expectException(NotInstantiableException::class);
+        $this->expectExceptionMessage(
+            'Can not determine value of the "value" parameter of type "string|int" when instantiating '
+        );
+        $definition->resolve($container);
+    }
+
+    public function testResolveUnionSelf(): void
+    {
+        $definition = new ParameterDefinition(
+            $this->getFirstConstructorParameter(UnionSelfDependency::class)
+        );
+        $container = new SimpleContainer();
+
+        $this->expectException(NotFoundException::class);
+        $definition->resolve($container);
+    }
+
     /**
      * @return ReflectionParameter[]
      */
@@ -80,4 +120,11 @@ private function getFirstParameter(Closure $closure): ReflectionParameter
     {
         return $this->getParameters($closure)[0];
     }
+
+    private function getFirstConstructorParameter(string $class): ReflectionParameter
+    {
+        return (new ReflectionClass($class))
+            ->getConstructor()
+            ->getParameters()[0];
+    }
 }
diff --git a/tests/Support/Circular/Chicken.php b/tests/Support/Circular/Chicken.php
@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Definitions\Tests\Support\Circular;
+
+class Chicken
+{
+    public function __construct(Egg $egg)
+    {
+    }
+}
diff --git a/tests/Support/Circular/Egg.php b/tests/Support/Circular/Egg.php
@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Definitions\Tests\Support\Circular;
+
+class Egg
+{
+    public function __construct(Chicken $chicken)
+    {
+    }
+}
diff --git a/tests/Support/UnionBuiltinDependency.php b/tests/Support/UnionBuiltinDependency.php
@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Definitions\Tests\Support;
+
+final class UnionBuiltinDependency
+{
+    private $value;
+
+    public function __construct(string|int $value)
+    {
+        $this->value = $value;
+    }
+
+    public function getValue()
+    {
+        return $this->value;
+    }
+}
diff --git a/tests/Support/UnionOptionalDependency.php b/tests/Support/UnionOptionalDependency.php
@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Definitions\Tests\Support;
+
+final class UnionOptionalDependency
+{
+    private $value;
+
+    public function __construct(string|ColorInterface $value = null)
+    {
+        $this->value = $value;
+    }
+
+    public function getValue()
+    {
+        return $this->value;
+    }
+}
diff --git a/tests/Unit/ArrayDefinitionTest.php b/tests/Unit/ArrayDefinitionTest.php
@@ -7,6 +7,13 @@
 use PHPUnit\Framework\TestCase;
 use Yiisoft\Definitions\ArrayDefinition;
 use Yiisoft\Definitions\Exception\InvalidConfigException;
+use Yiisoft\Definitions\Reference;
+use Yiisoft\Definitions\Tests\Support\Car;
+use Yiisoft\Definitions\Tests\Support\ColorInterface;
+use Yiisoft\Definitions\Tests\Support\ColorPink;
+use Yiisoft\Definitions\Tests\Support\EngineInterface;
+use Yiisoft\Definitions\Tests\Support\EngineMarkOne;
+use Yiisoft\Definitions\Tests\Support\EngineMarkTwo;
 use Yiisoft\Definitions\Tests\Support\Phone;
 use Yiisoft\Test\Support\Container\SimpleContainer;
 
@@ -230,4 +237,27 @@ public function testArgumentsIndexedBothByNameAndByPosition(): void
         );
         $definition->resolve($container);
     }
+
+    public function testReferenceContainer(): void
+    {
+        $container = new SimpleContainer([
+            EngineInterface::class => new EngineMarkOne(),
+        ]);
+        $referenceContainer = new SimpleContainer([
+            ColorInterface::class => new ColorPink(),
+            EngineInterface::class => new EngineMarkTwo(),
+        ]);
+
+        $definition = ArrayDefinition::fromConfig([
+            'class' => Car::class,
+            'setColor()' => [Reference::to(ColorInterface::class)],
+        ]);
+        $definition->setReferenceContainer($referenceContainer);
+
+        /** @var Car $object */
+        $object = $definition->resolve($container);
+
+        $this->assertInstanceOf(Car::class, $object);
+        $this->assertInstanceOf(EngineMarkOne::class, $object->getEngine());
+    }
 }