From 48f936275e7f8af937e0740cdbbac0f9557ab4a3 Mon Sep 17 00:00:00 2001
From: Romain Canon <romain.hydrocanon@gmail.com>
Date: Wed, 30 Mar 2022 23:09:19 +0200
Subject: [PATCH] misc: introduce layer for object builder arguments

---
 src/Mapper/Object/Arguments.php               | 60 +++++++++++++++++++
 src/Mapper/Object/DateTimeObjectBuilder.php   | 37 ++++++------
 .../Object/Exception/InvalidArgumentIndex.php | 22 +++++++
 src/Mapper/Object/FunctionObjectBuilder.php   | 25 +++++---
 src/Mapper/Object/MethodObjectBuilder.php     | 25 +++++---
 src/Mapper/Object/ObjectBuilder.php           |  5 +-
 src/Mapper/Object/ObjectBuilderFilterer.php   |  8 +--
 src/Mapper/Object/ReflectionObjectBuilder.php | 24 +++++---
 src/Mapper/Tree/Builder/ClassNodeBuilder.php  | 10 ++--
 .../Fake/Mapper/Object/FakeObjectBuilder.php  |  5 +-
 tests/Unit/Mapper/Object/ArgumentsTest.php    | 38 ++++++++++++
 .../Object/DateTimeObjectBuilderTest.php      | 22 +++++++
 .../Object/FunctionObjectBuilderTest.php      | 23 +++++++
 .../Mapper/Object/MethodObjectBuilderTest.php | 20 +++++++
 .../Object/ReflectionObjectBuilderTest.php    | 11 ++++
 15 files changed, 277 insertions(+), 58 deletions(-)
 create mode 100644 src/Mapper/Object/Arguments.php
 create mode 100644 src/Mapper/Object/Exception/InvalidArgumentIndex.php
 create mode 100644 tests/Unit/Mapper/Object/ArgumentsTest.php
 create mode 100644 tests/Unit/Mapper/Object/DateTimeObjectBuilderTest.php
 create mode 100644 tests/Unit/Mapper/Object/FunctionObjectBuilderTest.php

diff --git a/src/Mapper/Object/Arguments.php b/src/Mapper/Object/Arguments.php
new file mode 100644
index 00000000..6e662eb1
--- /dev/null
+++ b/src/Mapper/Object/Arguments.php
@@ -0,0 +1,60 @@
+<?php
+
+declare(strict_types=1);
+
+namespace CuyZ\Valinor\Mapper\Object;
+
+use Countable;
+use CuyZ\Valinor\Mapper\Object\Exception\InvalidArgumentIndex;
+use IteratorAggregate;
+use Traversable;
+
+/**
+ * @internal
+ *
+ * @implements IteratorAggregate<Argument>
+ */
+final class Arguments implements IteratorAggregate, Countable
+{
+    /** @var Argument[] */
+    private array $arguments;
+
+    public function __construct(Argument ...$arguments)
+    {
+        $this->arguments = $arguments;
+    }
+
+    public function at(int $index): Argument
+    {
+        if ($index >= count($this->arguments)) {
+            throw new InvalidArgumentIndex($index, $this);
+        }
+
+        return $this->arguments[$index];
+    }
+
+    public function signature(): string
+    {
+        $parameters = array_map(
+            fn (Argument $argument) => $argument->isRequired()
+                ? "{$argument->name()}: {$argument->type()}"
+                : "{$argument->name()}?: {$argument->type()}",
+            $this->arguments
+        );
+
+        return 'array{' . implode(', ', $parameters) . '}';
+    }
+
+    public function count(): int
+    {
+        return count($this->arguments);
+    }
+
+    /**
+     * @return Traversable<Argument>
+     */
+    public function getIterator(): Traversable
+    {
+        yield from $this->arguments;
+    }
+}
diff --git a/src/Mapper/Object/DateTimeObjectBuilder.php b/src/Mapper/Object/DateTimeObjectBuilder.php
index 1f83bf20..9f508588 100644
--- a/src/Mapper/Object/DateTimeObjectBuilder.php
+++ b/src/Mapper/Object/DateTimeObjectBuilder.php
@@ -5,7 +5,6 @@
 namespace CuyZ\Valinor\Mapper\Object;
 
 use CuyZ\Valinor\Mapper\Object\Exception\CannotParseToDateTime;
-use CuyZ\Valinor\Type\Type;
 use CuyZ\Valinor\Type\Types\NonEmptyStringType;
 use CuyZ\Valinor\Type\Types\NullType;
 use CuyZ\Valinor\Type\Types\PositiveIntegerType;
@@ -29,11 +28,11 @@ final class DateTimeObjectBuilder implements ObjectBuilder
     public const DATE_MYSQL = 'Y-m-d H:i:s';
     public const DATE_PGSQL = 'Y-m-d H:i:s.u';
 
-    private static Type $argumentType;
-
     /** @var class-string<DateTime|DateTimeImmutable> */
     private string $className;
 
+    private Arguments $arguments;
+
     /**
      * @param class-string<DateTime|DateTimeImmutable> $className
      */
@@ -42,24 +41,24 @@ public function __construct(string $className)
         $this->className = $className;
     }
 
-    public function describeArguments(): iterable
+    public function describeArguments(): Arguments
     {
-        self::$argumentType ??= new UnionType(
-            new UnionType(PositiveIntegerType::get(), NonEmptyStringType::get()),
-            new ShapedArrayType(
-                new ShapedArrayElement(
-                    new StringValueType('datetime'),
-                    new UnionType(PositiveIntegerType::get(), NonEmptyStringType::get())
-                ),
-                new ShapedArrayElement(
-                    new StringValueType('format'),
-                    new UnionType(NullType::get(), NonEmptyStringType::get()),
-                    true
-                ),
-            )
+        return $this->arguments ??= new Arguments(
+            Argument::required('value', new UnionType(
+                new UnionType(PositiveIntegerType::get(), NonEmptyStringType::get()),
+                new ShapedArrayType(
+                    new ShapedArrayElement(
+                        new StringValueType('datetime'),
+                        new UnionType(PositiveIntegerType::get(), NonEmptyStringType::get())
+                    ),
+                    new ShapedArrayElement(
+                        new StringValueType('format'),
+                        new UnionType(NullType::get(), NonEmptyStringType::get()),
+                        true
+                    ),
+                )
+            ))
         );
-
-        yield Argument::required('value', self::$argumentType);
     }
 
     public function build(array $arguments): DateTimeInterface
diff --git a/src/Mapper/Object/Exception/InvalidArgumentIndex.php b/src/Mapper/Object/Exception/InvalidArgumentIndex.php
new file mode 100644
index 00000000..ea0083ab
--- /dev/null
+++ b/src/Mapper/Object/Exception/InvalidArgumentIndex.php
@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace CuyZ\Valinor\Mapper\Object\Exception;
+
+use CuyZ\Valinor\Mapper\Object\Arguments;
+use OutOfBoundsException;
+
+/** @internal */
+final class InvalidArgumentIndex extends OutOfBoundsException
+{
+    public function __construct(int $index, Arguments $arguments)
+    {
+        $max = $arguments->count() - 1;
+
+        parent::__construct(
+            "Index $index is out of range, it should be between 0 and $max.",
+            1648672136
+        );
+    }
+}
diff --git a/src/Mapper/Object/FunctionObjectBuilder.php b/src/Mapper/Object/FunctionObjectBuilder.php
index 022e90aa..8e8d4353 100644
--- a/src/Mapper/Object/FunctionObjectBuilder.php
+++ b/src/Mapper/Object/FunctionObjectBuilder.php
@@ -5,9 +5,14 @@
 namespace CuyZ\Valinor\Mapper\Object;
 
 use CuyZ\Valinor\Definition\FunctionDefinition;
+use CuyZ\Valinor\Definition\ParameterDefinition;
 use CuyZ\Valinor\Mapper\Tree\Message\ThrowableMessage;
 use Exception;
 
+use function array_map;
+use function array_values;
+use function iterator_to_array;
+
 /** @internal */
 final class FunctionObjectBuilder implements ObjectBuilder
 {
@@ -16,6 +21,8 @@ final class FunctionObjectBuilder implements ObjectBuilder
     /** @var callable(): object */
     private $callback;
 
+    private Arguments $arguments;
+
     /**
      * @param callable(): object $callback
      */
@@ -25,15 +32,17 @@ public function __construct(FunctionDefinition $function, callable $callback)
         $this->callback = $callback;
     }
 
-    public function describeArguments(): iterable
+    public function describeArguments(): Arguments
     {
-        foreach ($this->function->parameters() as $parameter) {
-            $argument = $parameter->isOptional()
-                ? Argument::optional($parameter->name(), $parameter->type(), $parameter->defaultValue())
-                : Argument::required($parameter->name(), $parameter->type());
-
-            yield $argument->withAttributes($parameter->attributes());
-        }
+        return $this->arguments ??= new Arguments(
+            ...array_map(function (ParameterDefinition $parameter) {
+                $argument = $parameter->isOptional()
+                    ? Argument::optional($parameter->name(), $parameter->type(), $parameter->defaultValue())
+                    : Argument::required($parameter->name(), $parameter->type());
+
+                return $argument->withAttributes($parameter->attributes());
+            }, array_values(iterator_to_array($this->function->parameters()))) // @PHP8.1 array unpacking
+        );
     }
 
     public function build(array $arguments): object
diff --git a/src/Mapper/Object/MethodObjectBuilder.php b/src/Mapper/Object/MethodObjectBuilder.php
index 8f89de10..daab0b0f 100644
--- a/src/Mapper/Object/MethodObjectBuilder.php
+++ b/src/Mapper/Object/MethodObjectBuilder.php
@@ -6,6 +6,7 @@
 
 use CuyZ\Valinor\Definition\ClassDefinition;
 use CuyZ\Valinor\Definition\MethodDefinition;
+use CuyZ\Valinor\Definition\ParameterDefinition;
 use CuyZ\Valinor\Mapper\Object\Exception\ConstructorMethodIsNotPublic;
 use CuyZ\Valinor\Mapper\Object\Exception\ConstructorMethodIsNotStatic;
 use CuyZ\Valinor\Mapper\Object\Exception\InvalidConstructorMethodClassReturnType;
@@ -13,6 +14,10 @@
 use CuyZ\Valinor\Mapper\Tree\Message\ThrowableMessage;
 use Exception;
 
+use function array_map;
+use function array_values;
+use function iterator_to_array;
+
 /** @api */
 final class MethodObjectBuilder implements ObjectBuilder
 {
@@ -20,6 +25,8 @@ final class MethodObjectBuilder implements ObjectBuilder
 
     private MethodDefinition $method;
 
+    private Arguments $arguments;
+
     public function __construct(ClassDefinition $class, string $methodName)
     {
         $methods = $class->methods();
@@ -48,15 +55,17 @@ public function __construct(ClassDefinition $class, string $methodName)
         }
     }
 
-    public function describeArguments(): iterable
+    public function describeArguments(): Arguments
     {
-        foreach ($this->method->parameters() as $parameter) {
-            $argument = $parameter->isOptional()
-                ? Argument::optional($parameter->name(), $parameter->type(), $parameter->defaultValue())
-                : Argument::required($parameter->name(), $parameter->type());
-
-            yield $argument->withAttributes($parameter->attributes());
-        }
+        return $this->arguments ??= new Arguments(
+            ...array_map(function (ParameterDefinition $parameter) {
+                $argument = $parameter->isOptional()
+                    ? Argument::optional($parameter->name(), $parameter->type(), $parameter->defaultValue())
+                    : Argument::required($parameter->name(), $parameter->type());
+
+                return $argument->withAttributes($parameter->attributes());
+            }, array_values(iterator_to_array($this->method->parameters()))) // @PHP8.1 array unpacking
+        );
     }
 
     public function build(array $arguments): object
diff --git a/src/Mapper/Object/ObjectBuilder.php b/src/Mapper/Object/ObjectBuilder.php
index 8974663e..815b282f 100644
--- a/src/Mapper/Object/ObjectBuilder.php
+++ b/src/Mapper/Object/ObjectBuilder.php
@@ -7,10 +7,7 @@
 /** @internal */
 interface ObjectBuilder
 {
-    /**
-     * @return iterable<Argument>
-     */
-    public function describeArguments(): iterable;
+    public function describeArguments(): Arguments;
 
     /**
      * @param array<string, mixed> $arguments
diff --git a/src/Mapper/Object/ObjectBuilderFilterer.php b/src/Mapper/Object/ObjectBuilderFilterer.php
index 6f8e3f50..70744dd3 100644
--- a/src/Mapper/Object/ObjectBuilderFilterer.php
+++ b/src/Mapper/Object/ObjectBuilderFilterer.php
@@ -54,7 +54,7 @@ public function filter($source, ObjectBuilder ...$builders): ObjectBuilder
      */
     private function filledArguments(ObjectBuilder $builder, $source)
     {
-        $arguments = [...$builder->describeArguments()];
+        $arguments = $builder->describeArguments();
 
         if (! is_array($source)) {
             return count($arguments) === 1;
@@ -63,10 +63,10 @@ private function filledArguments(ObjectBuilder $builder, $source)
         /** @infection-ignore-all */
         $filled = 0;
 
-        foreach ($arguments as $parameter) {
-            if (isset($source[$parameter->name()])) {
+        foreach ($arguments as $argument) {
+            if (isset($source[$argument->name()])) {
                 $filled++;
-            } elseif ($parameter->isRequired()) {
+            } elseif ($argument->isRequired()) {
                 return false;
             }
         }
diff --git a/src/Mapper/Object/ReflectionObjectBuilder.php b/src/Mapper/Object/ReflectionObjectBuilder.php
index f3e72eff..f5c0854a 100644
--- a/src/Mapper/Object/ReflectionObjectBuilder.php
+++ b/src/Mapper/Object/ReflectionObjectBuilder.php
@@ -5,29 +5,37 @@
 namespace CuyZ\Valinor\Mapper\Object;
 
 use CuyZ\Valinor\Definition\ClassDefinition;
+use CuyZ\Valinor\Definition\PropertyDefinition;
 use CuyZ\Valinor\Mapper\Object\Exception\MissingPropertyArgument;
 
 use function array_key_exists;
+use function array_map;
+use function array_values;
+use function iterator_to_array;
 
 /** @api */
 final class ReflectionObjectBuilder implements ObjectBuilder
 {
     private ClassDefinition $class;
 
+    private Arguments $arguments;
+
     public function __construct(ClassDefinition $class)
     {
         $this->class = $class;
     }
 
-    public function describeArguments(): iterable
+    public function describeArguments(): Arguments
     {
-        foreach ($this->class->properties() as $property) {
-            $argument = $property->hasDefaultValue()
-                ? Argument::optional($property->name(), $property->type(), $property->defaultValue())
-                : Argument::required($property->name(), $property->type());
-
-            yield $argument->withAttributes($property->attributes());
-        }
+        return $this->arguments ??= new Arguments(
+            ...array_map(function (PropertyDefinition $property) {
+                $argument = $property->hasDefaultValue()
+                    ? Argument::optional($property->name(), $property->type(), $property->defaultValue())
+                    : Argument::required($property->name(), $property->type());
+
+                return $argument->withAttributes($property->attributes());
+            }, array_values(iterator_to_array($this->class->properties()))) // @PHP8.1 array unpacking
+        );
     }
 
     public function build(array $arguments): object
diff --git a/src/Mapper/Tree/Builder/ClassNodeBuilder.php b/src/Mapper/Tree/Builder/ClassNodeBuilder.php
index d3fe5ce8..8d932c3c 100644
--- a/src/Mapper/Tree/Builder/ClassNodeBuilder.php
+++ b/src/Mapper/Tree/Builder/ClassNodeBuilder.php
@@ -5,7 +5,7 @@
 namespace CuyZ\Valinor\Mapper\Tree\Builder;
 
 use CuyZ\Valinor\Definition\Repository\ClassDefinitionRepository;
-use CuyZ\Valinor\Mapper\Object\Argument;
+use CuyZ\Valinor\Mapper\Object\Arguments;
 use CuyZ\Valinor\Mapper\Object\Exception\InvalidSourceForObject;
 use CuyZ\Valinor\Mapper\Object\Factory\ObjectBuilderFactory;
 use CuyZ\Valinor\Mapper\Object\Factory\SuitableObjectBuilderNotFound;
@@ -58,9 +58,9 @@ public function build(Shell $shell, RootNodeBuilder $rootBuilder): Node
         $source = $shell->value();
 
         $builder = $this->builder($source, ...$classTypes);
-        $arguments = [...$builder->describeArguments()];
+        $arguments = $builder->describeArguments();
 
-        $source = $this->transformSource($source, ...$arguments);
+        $source = $this->transformSource($source, $arguments);
         $children = [];
 
         foreach ($arguments as $argument) {
@@ -129,7 +129,7 @@ private function builder($source, ClassType ...$classTypes): ObjectBuilder
      * @param mixed $source
      * @return mixed[]
      */
-    private function transformSource($source, Argument ...$arguments): array
+    private function transformSource($source, Arguments $arguments): array
     {
         if ($source === null || count($arguments) === 0) {
             return [];
@@ -140,7 +140,7 @@ private function transformSource($source, Argument ...$arguments): array
         }
 
         if (count($arguments) === 1) {
-            $name = $arguments[0]->name();
+            $name = $arguments->at(0)->name();
 
             if (! is_array($source) || ! array_key_exists($name, $source)) {
                 $source = [$name => $source];
diff --git a/tests/Fake/Mapper/Object/FakeObjectBuilder.php b/tests/Fake/Mapper/Object/FakeObjectBuilder.php
index 6a2f0b26..9f363bf3 100644
--- a/tests/Fake/Mapper/Object/FakeObjectBuilder.php
+++ b/tests/Fake/Mapper/Object/FakeObjectBuilder.php
@@ -4,14 +4,15 @@
 
 namespace CuyZ\Valinor\Tests\Fake\Mapper\Object;
 
+use CuyZ\Valinor\Mapper\Object\Arguments;
 use CuyZ\Valinor\Mapper\Object\ObjectBuilder;
 use stdClass;
 
 final class FakeObjectBuilder implements ObjectBuilder
 {
-    public function describeArguments(): iterable
+    public function describeArguments(): Arguments
     {
-        return [];
+        return new Arguments();
     }
 
     public function build(array $arguments): object
diff --git a/tests/Unit/Mapper/Object/ArgumentsTest.php b/tests/Unit/Mapper/Object/ArgumentsTest.php
new file mode 100644
index 00000000..8a22c0d7
--- /dev/null
+++ b/tests/Unit/Mapper/Object/ArgumentsTest.php
@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+namespace CuyZ\Valinor\Tests\Unit\Mapper\Object;
+
+use CuyZ\Valinor\Mapper\Object\Argument;
+use CuyZ\Valinor\Mapper\Object\Arguments;
+use CuyZ\Valinor\Mapper\Object\Exception\InvalidArgumentIndex;
+use CuyZ\Valinor\Tests\Fake\Type\FakeType;
+use PHPUnit\Framework\TestCase;
+
+final class ArgumentsTest extends TestCase
+{
+    public function test_get_argument_at_invalid_index_throws_exception(): void
+    {
+        $this->expectException(InvalidArgumentIndex::class);
+        $this->expectExceptionCode(1648672136);
+        $this->expectExceptionMessage("Index 1 is out of range, it should be between 0 and 0.");
+
+        (new Arguments(
+            Argument::required('someArgument', FakeType::permissive())
+        ))->at(1);
+    }
+
+    public function test_signature_is_correct(): void
+    {
+        $typeA = FakeType::permissive();
+        $typeB = FakeType::permissive();
+
+        $arguments = new Arguments(
+            Argument::required('someArgumentA', $typeA),
+            Argument::optional('someArgumentB', $typeB, 'defaultValue')
+        );
+
+        self::assertSame("array{someArgumentA: $typeA, someArgumentB?: $typeB}", $arguments->signature());
+    }
+}
diff --git a/tests/Unit/Mapper/Object/DateTimeObjectBuilderTest.php b/tests/Unit/Mapper/Object/DateTimeObjectBuilderTest.php
new file mode 100644
index 00000000..dfc41662
--- /dev/null
+++ b/tests/Unit/Mapper/Object/DateTimeObjectBuilderTest.php
@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace CuyZ\Valinor\Tests\Unit\Mapper\Object;
+
+use CuyZ\Valinor\Mapper\Object\DateTimeObjectBuilder;
+use DateTimeImmutable;
+use PHPUnit\Framework\TestCase;
+
+final class DateTimeObjectBuilderTest extends TestCase
+{
+    public function test_arguments_instance_stays_the_same(): void
+    {
+        $objectBuilder = new DateTimeObjectBuilder(DateTimeImmutable::class);
+
+        $argumentsA = $objectBuilder->describeArguments();
+        $argumentsB = $objectBuilder->describeArguments();
+
+        self::assertSame($argumentsA, $argumentsB);
+    }
+}
diff --git a/tests/Unit/Mapper/Object/FunctionObjectBuilderTest.php b/tests/Unit/Mapper/Object/FunctionObjectBuilderTest.php
new file mode 100644
index 00000000..67b89a45
--- /dev/null
+++ b/tests/Unit/Mapper/Object/FunctionObjectBuilderTest.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace CuyZ\Valinor\Tests\Unit\Mapper\Object;
+
+use CuyZ\Valinor\Mapper\Object\FunctionObjectBuilder;
+use CuyZ\Valinor\Tests\Fake\Definition\FakeFunctionDefinition;
+use PHPUnit\Framework\TestCase;
+use stdClass;
+
+final class FunctionObjectBuilderTest extends TestCase
+{
+    public function test_arguments_instance_stays_the_same(): void
+    {
+        $objectBuilder = new FunctionObjectBuilder(FakeFunctionDefinition::new(), fn () => new stdClass());
+
+        $argumentsA = $objectBuilder->describeArguments();
+        $argumentsB = $objectBuilder->describeArguments();
+
+        self::assertSame($argumentsA, $argumentsB);
+    }
+}
diff --git a/tests/Unit/Mapper/Object/MethodObjectBuilderTest.php b/tests/Unit/Mapper/Object/MethodObjectBuilderTest.php
index bc4a4665..b3235c12 100644
--- a/tests/Unit/Mapper/Object/MethodObjectBuilderTest.php
+++ b/tests/Unit/Mapper/Object/MethodObjectBuilderTest.php
@@ -151,6 +151,26 @@ private static function someConstructor(): void
         $class = FakeClassDefinition::fromReflection(new ReflectionClass($classWithPrivateNativeConstructor));
         new MethodObjectBuilder($class, 'someConstructor');
     }
+
+    public function test_arguments_instance_stays_the_same(): void
+    {
+        $class = new class ('foo') {
+            public string $string;
+
+            public function __construct(string $string)
+            {
+                $this->string = $string;
+            }
+        };
+        $class = FakeClassDefinition::fromReflection(new ReflectionClass($class));
+
+        $objectBuilder = new MethodObjectBuilder($class, '__construct');
+
+        $argumentsA = $objectBuilder->describeArguments();
+        $argumentsB = $objectBuilder->describeArguments();
+
+        self::assertSame($argumentsA, $argumentsB);
+    }
 }
 
 final class ObjectWithPrivateNativeConstructor
diff --git a/tests/Unit/Mapper/Object/ReflectionObjectBuilderTest.php b/tests/Unit/Mapper/Object/ReflectionObjectBuilderTest.php
index 90c4d3d2..756d4a75 100644
--- a/tests/Unit/Mapper/Object/ReflectionObjectBuilderTest.php
+++ b/tests/Unit/Mapper/Object/ReflectionObjectBuilderTest.php
@@ -50,6 +50,17 @@ public function valueC(): string
         self::assertSame('valueC', $result->valueC()); // @phpstan-ignore-line
     }
 
+    public function test_arguments_instance_stays_the_same(): void
+    {
+        $class = FakeClassDefinition::new();
+        $objectBuilder = new ReflectionObjectBuilder($class);
+
+        $argumentsA = $objectBuilder->describeArguments();
+        $argumentsB = $objectBuilder->describeArguments();
+
+        self::assertSame($argumentsA, $argumentsB);
+    }
+
     public function test_missing_arguments_throws_exception(): void
     {
         $object = new class () {