From ae7ddcf3caa77f2c4b4ad99dcdbb5f609de55335 Mon Sep 17 00:00:00 2001 From: Romain Canon Date: Tue, 2 Aug 2022 11:01:28 +0200 Subject: [PATCH] fix: properly handle callable objects of the same class Using two instances of the same class implementing the `__invoke()` method in one of the mapper builder methods will now be properly handled by the library --- src/Definition/Exception/CallbackNotFound.php | 20 ---------- src/Definition/Exception/FunctionNotFound.php | 22 ----------- src/Definition/FunctionObject.php | 30 ++++++++++++++ src/Definition/FunctionsContainer.php | 39 +++++-------------- .../ConstructorObjectBuilderFactory.php | 4 +- .../Factory/DateTimeObjectBuilderFactory.php | 8 ++-- src/Mapper/Object/FunctionObjectBuilder.php | 21 ++++------ .../Tree/Builder/ObjectImplementations.php | 9 ++--- .../Tree/Builder/ValueAlteringNodeBuilder.php | 4 +- .../Definition/FunctionsContainerTest.php | 33 +++++----------- .../Object/FunctionObjectBuilderTest.php | 3 +- 11 files changed, 71 insertions(+), 122 deletions(-) delete mode 100644 src/Definition/Exception/CallbackNotFound.php delete mode 100644 src/Definition/Exception/FunctionNotFound.php create mode 100644 src/Definition/FunctionObject.php diff --git a/src/Definition/Exception/CallbackNotFound.php b/src/Definition/Exception/CallbackNotFound.php deleted file mode 100644 index 1c772d9..0000000 --- a/src/Definition/Exception/CallbackNotFound.php +++ /dev/null @@ -1,20 +0,0 @@ -signature()}` could not be found.", - 1647523495 - ); - } -} diff --git a/src/Definition/Exception/FunctionNotFound.php b/src/Definition/Exception/FunctionNotFound.php deleted file mode 100644 index afa7502..0000000 --- a/src/Definition/Exception/FunctionNotFound.php +++ /dev/null @@ -1,22 +0,0 @@ -definition = $definition; + $this->callback = $callback; + } + + public function definition(): FunctionDefinition + { + return $this->definition; + } + + public function callback(): callable + { + return $this->callback; + } +} diff --git a/src/Definition/FunctionsContainer.php b/src/Definition/FunctionsContainer.php index d9e8a9a..68ec0df 100644 --- a/src/Definition/FunctionsContainer.php +++ b/src/Definition/FunctionsContainer.php @@ -4,8 +4,6 @@ declare(strict_types=1); namespace CuyZ\Valinor\Definition; -use CuyZ\Valinor\Definition\Exception\CallbackNotFound; -use CuyZ\Valinor\Definition\Exception\FunctionNotFound; use CuyZ\Valinor\Definition\Repository\FunctionDefinitionRepository; use IteratorAggregate; use Traversable; @@ -15,7 +13,7 @@ use function array_keys; /** * @internal * - * @implements IteratorAggregate + * @implements IteratorAggregate */ final class FunctionsContainer implements IteratorAggregate { @@ -24,7 +22,7 @@ final class FunctionsContainer implements IteratorAggregate /** @var array */ private array $callables; - /** @var array */ + /** @var array */ private array $functions = []; /** @@ -47,43 +45,26 @@ final class FunctionsContainer implements IteratorAggregate /** * @param string|int $key */ - public function get($key): FunctionDefinition + public function get($key): FunctionObject { - if (! $this->has($key)) { - throw new FunctionNotFound($key); - } - - return $this->function($key)['definition']; - } - - public function callback(FunctionDefinition $function): callable - { - foreach ($this->functions as $data) { - if ($function === $data['definition']) { - return $data['callback']; - } - } - - throw new CallbackNotFound($function); + return $this->function($key); } public function getIterator(): Traversable { foreach (array_keys($this->callables) as $key) { - yield $key => $this->function($key)['definition']; + yield $key => $this->function($key); } } /** * @param string|int $key - * @return array{definition: FunctionDefinition, callback: callable} */ - private function function($key): array + private function function($key): FunctionObject { - /** @infection-ignore-all */ - return $this->functions[$key] ??= [ - 'callback' => $this->callables[$key], - 'definition' => $this->functionDefinitionRepository->for($this->callables[$key]), - ]; + return $this->functions[$key] ??= new FunctionObject( + $this->functionDefinitionRepository->for($this->callables[$key]), + $this->callables[$key] + ); } } diff --git a/src/Mapper/Object/Factory/ConstructorObjectBuilderFactory.php b/src/Mapper/Object/Factory/ConstructorObjectBuilderFactory.php index b770c9b..c638804 100644 --- a/src/Mapper/Object/Factory/ConstructorObjectBuilderFactory.php +++ b/src/Mapper/Object/Factory/ConstructorObjectBuilderFactory.php @@ -70,8 +70,8 @@ final class ConstructorObjectBuilderFactory implements ObjectBuilderFactory $methods = $class->methods(); foreach ($this->constructors as $constructor) { - if ($constructor->returnType()->matches($type)) { - $builders[] = new FunctionObjectBuilder($constructor, $this->constructors->callback($constructor)); + if ($constructor->definition()->returnType()->matches($type)) { + $builders[] = new FunctionObjectBuilder($constructor); } } diff --git a/src/Mapper/Object/Factory/DateTimeObjectBuilderFactory.php b/src/Mapper/Object/Factory/DateTimeObjectBuilderFactory.php index 4072301..e523895 100644 --- a/src/Mapper/Object/Factory/DateTimeObjectBuilderFactory.php +++ b/src/Mapper/Object/Factory/DateTimeObjectBuilderFactory.php @@ -56,15 +56,17 @@ final class DateTimeObjectBuilderFactory implements ObjectBuilderFactory $this->builders[$key] = []; foreach ($this->functions as $function) { - if (! $function->returnType()->matches($type)) { + $definition = $function->definition(); + + if (! $definition->returnType()->matches($type)) { continue; } - if (count($function->parameters()) === 1) { + if (count($definition->parameters()) === 1) { $overridesDefault = true; } - $this->builders[$key][] = new FunctionObjectBuilder($function, $this->functions->callback($function)); + $this->builders[$key][] = new FunctionObjectBuilder($function); } if (! $overridesDefault) { diff --git a/src/Mapper/Object/FunctionObjectBuilder.php b/src/Mapper/Object/FunctionObjectBuilder.php index 29eb741..6b7c1a0 100644 --- a/src/Mapper/Object/FunctionObjectBuilder.php +++ b/src/Mapper/Object/FunctionObjectBuilder.php @@ -4,40 +4,33 @@ declare(strict_types=1); namespace CuyZ\Valinor\Mapper\Object; -use CuyZ\Valinor\Definition\FunctionDefinition; +use CuyZ\Valinor\Definition\FunctionObject; use CuyZ\Valinor\Mapper\Tree\Message\UserlandError; use Exception; /** @internal */ final class FunctionObjectBuilder implements ObjectBuilder { - private FunctionDefinition $function; - - /** @var callable(): object */ - private $callback; + private FunctionObject $function; private Arguments $arguments; - /** - * @param callable(): object $callback - */ - public function __construct(FunctionDefinition $function, callable $callback) + public function __construct(FunctionObject $function) { $this->function = $function; - $this->callback = $callback; } public function describeArguments(): Arguments { - return $this->arguments ??= Arguments::fromParameters($this->function->parameters()); + return $this->arguments ??= Arguments::fromParameters($this->function->definition()->parameters()); } public function build(array $arguments): object { - $arguments = new MethodArguments($this->function->parameters(), $arguments); + $arguments = new MethodArguments($this->function->definition()->parameters(), $arguments); try { - return ($this->callback)(...$arguments); + return ($this->function->callback())(...$arguments); } catch (Exception $exception) { throw UserlandError::from($exception); } @@ -45,6 +38,6 @@ final class FunctionObjectBuilder implements ObjectBuilder public function signature(): string { - return $this->function->signature(); + return $this->function->definition()->signature(); } } diff --git a/src/Mapper/Tree/Builder/ObjectImplementations.php b/src/Mapper/Tree/Builder/ObjectImplementations.php index 6b4f9cb..835e59d 100644 --- a/src/Mapper/Tree/Builder/ObjectImplementations.php +++ b/src/Mapper/Tree/Builder/ObjectImplementations.php @@ -49,7 +49,7 @@ final class ObjectImplementations throw new CannotResolveObjectType($name); } - return $this->functions->get($name); + return $this->functions->get($name)->definition(); } /** @@ -72,11 +72,8 @@ final class ObjectImplementations */ private function call(string $name, array $arguments): string { - $function = $this->functions->get($name); - $callback = $this->functions->callback($function); - try { - $signature = $callback(...$arguments); + $signature = ($this->functions->get($name)->callback())(...$arguments); } catch (Exception $exception) { throw new ObjectImplementationCallbackError($name, $exception); } @@ -93,7 +90,7 @@ final class ObjectImplementations */ private function implementations(string $name): array { - $function = $this->functions->get($name); + $function = $this->functions->get($name)->definition(); try { $type = $this->typeParser->parse($name); diff --git a/src/Mapper/Tree/Builder/ValueAlteringNodeBuilder.php b/src/Mapper/Tree/Builder/ValueAlteringNodeBuilder.php index 77fbf8f..959aa1a 100644 --- a/src/Mapper/Tree/Builder/ValueAlteringNodeBuilder.php +++ b/src/Mapper/Tree/Builder/ValueAlteringNodeBuilder.php @@ -31,7 +31,7 @@ final class ValueAlteringNodeBuilder implements NodeBuilder $value = $node->value(); foreach ($this->functions as $function) { - $parameters = $function->parameters(); + $parameters = $function->definition()->parameters(); if (count($parameters) === 0) { continue; @@ -43,7 +43,7 @@ final class ValueAlteringNodeBuilder implements NodeBuilder continue; } - $value = ($this->functions->callback($function))($value); + $value = ($function->callback())($value); $node = $node->withValue($value); } diff --git a/tests/Unit/Definition/FunctionsContainerTest.php b/tests/Unit/Definition/FunctionsContainerTest.php index 1c83bd8..97f9fb2 100644 --- a/tests/Unit/Definition/FunctionsContainerTest.php +++ b/tests/Unit/Definition/FunctionsContainerTest.php @@ -4,10 +4,7 @@ declare(strict_types=1); namespace CuyZ\Valinor\Tests\Unit\Definition; -use CuyZ\Valinor\Definition\Exception\CallbackNotFound; -use CuyZ\Valinor\Definition\Exception\FunctionNotFound; use CuyZ\Valinor\Definition\FunctionsContainer; -use CuyZ\Valinor\Tests\Fake\Definition\FakeFunctionDefinition; use CuyZ\Valinor\Tests\Fake\Definition\Repository\FakeFunctionDefinitionRepository; use PHPUnit\Framework\TestCase; @@ -15,26 +12,6 @@ use function iterator_to_array; final class FunctionsContainerTest extends TestCase { - public function test_get_unknown_function_throws_exception(): void - { - $this->expectException(FunctionNotFound::class); - $this->expectExceptionCode(1647523444); - $this->expectExceptionMessage('The function `unknown` was not found.'); - - (new FunctionsContainer(new FakeFunctionDefinitionRepository(), []))->get('unknown'); - } - - public function test_get_unknown_callback_throws_exception(): void - { - $function = FakeFunctionDefinition::new(); - - $this->expectException(CallbackNotFound::class); - $this->expectExceptionCode(1647523495); - $this->expectExceptionMessage("The callback associated to `{$function->signature()}` could not be found."); - - (new FunctionsContainer(new FakeFunctionDefinitionRepository(), []))->callback($function); - } - public function test_keys_are_kept_when_iterating(): void { $functions = (new FunctionsContainer(new FakeFunctionDefinitionRepository(), [ @@ -47,4 +24,14 @@ final class FunctionsContainerTest extends TestCase self::assertArrayHasKey('foo', $functions); self::assertArrayHasKey('bar', $functions); } + + public function test_function_object_remains_the_same(): void + { + $functions = (new FunctionsContainer(new FakeFunctionDefinitionRepository(), [fn () => 'foo'])); + + $functionA = $functions->get(0); + $functionB = $functions->get(0); + + self::assertSame($functionA, $functionB); + } } diff --git a/tests/Unit/Mapper/Object/FunctionObjectBuilderTest.php b/tests/Unit/Mapper/Object/FunctionObjectBuilderTest.php index 67b89a4..0036b62 100644 --- a/tests/Unit/Mapper/Object/FunctionObjectBuilderTest.php +++ b/tests/Unit/Mapper/Object/FunctionObjectBuilderTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace CuyZ\Valinor\Tests\Unit\Mapper\Object; +use CuyZ\Valinor\Definition\FunctionObject; use CuyZ\Valinor\Mapper\Object\FunctionObjectBuilder; use CuyZ\Valinor\Tests\Fake\Definition\FakeFunctionDefinition; use PHPUnit\Framework\TestCase; @@ -13,7 +14,7 @@ final class FunctionObjectBuilderTest extends TestCase { public function test_arguments_instance_stays_the_same(): void { - $objectBuilder = new FunctionObjectBuilder(FakeFunctionDefinition::new(), fn () => new stdClass()); + $objectBuilder = new FunctionObjectBuilder(new FunctionObject(FakeFunctionDefinition::new(), fn () => new stdClass())); $argumentsA = $objectBuilder->describeArguments(); $argumentsB = $objectBuilder->describeArguments();