From 7034ff34ac5537803b7a4fad1b661362a9c3907f Mon Sep 17 00:00:00 2001 From: Baptiste Langlade Date: Sat, 23 Aug 2025 21:33:24 +0200 Subject: [PATCH 1/4] remove support to use a string as service name --- CHANGELOG.md | 6 +++ fixtures/Services.php | 2 + fixtures/psalm.php | 2 +- src/Builder.php | 14 +++--- src/Container.php | 16 +++---- tests/ContainerTest.php | 104 +++++++++++++++------------------------- 6 files changed, 61 insertions(+), 83 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55e48a5..1bea6f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased] + +### Removed + +- Using a `string` as a service name + ## 2.1.0 - 2024-03-24 ### Added diff --git a/fixtures/Services.php b/fixtures/Services.php index d9e5c44..72a9193 100644 --- a/fixtures/Services.php +++ b/fixtures/Services.php @@ -12,6 +12,8 @@ enum Services implements Service { case a; + case name; + case dependency; /** * @return self<\Exception> diff --git a/fixtures/psalm.php b/fixtures/psalm.php index a042b21..80af8eb 100644 --- a/fixtures/psalm.php +++ b/fixtures/psalm.php @@ -5,7 +5,7 @@ use Innmind\DI\Builder; $container = Builder::new() - ->add(Services::a, static fn() => new \Exception('foo')) + ->add(Services::a(), static fn() => new \Exception('foo')) ->build(); echo $container(Services::a())->getMessage(); diff --git a/src/Builder.php b/src/Builder.php index 9f6a12a..e9ba694 100644 --- a/src/Builder.php +++ b/src/Builder.php @@ -29,18 +29,16 @@ public static function new(): self } /** - * @param string|Service $name Using a string is deprecated - * @param callable(Container): object $definition + * @template T of object + * + * @param Service $name Using a string is deprecated + * @param callable(Container): T $definition */ #[\NoDiscard] - public function add(string|Service $name, callable $definition): self + public function add(Service $name, callable $definition): self { - if ($name instanceof Service) { - $name = \spl_object_hash($name); - } - $definitions = $this->definitions; - $definitions[$name] = $definition; + $definitions[\spl_object_hash($name)] = $definition; return new self($definitions); } diff --git a/src/Container.php b/src/Container.php index 57f60b2..b352f94 100644 --- a/src/Container.php +++ b/src/Container.php @@ -29,20 +29,17 @@ private function __construct(array $definitions) /** * @template T of object - * @template N of string|Service * - * @param N $name + * @param Service $name * * @throws ServiceNotFound * @throws CircularDependency * - * @return (N is string ? object : T) + * @return T */ - public function __invoke(string|Service $name): object + public function __invoke(Service $name): object { - if ($name instanceof Service) { - $name = \spl_object_hash($name); - } + $name = \spl_object_hash($name); /** @psalm-suppress PossiblyInvalidArgument */ if (!\array_key_exists($name, $this->definitions)) { @@ -63,7 +60,10 @@ public function __invoke(string|Service $name): object $this->building[] = $name; try { - /** @psalm-suppress InvalidPropertyAssignmentValue */ + /** + * @psalm-suppress InvalidPropertyAssignmentValue + * @var T + */ return $this->services[$name] ??= ($this->definitions[$name])($this); } finally { \array_pop($this->building); diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index 8f436de..4fc7556 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -9,96 +9,68 @@ Exception\ServiceNotFound, Exception\CircularDependency, }; -use Innmind\BlackBox\{ - PHPUnit\BlackBox, - PHPUnit\Framework\TestCase, - Set, -}; +use Innmind\BlackBox\PHPUnit\Framework\TestCase; use Fixtures\Innmind\DI\Services; class ContainerTest extends TestCase { - use BlackBox; - public function testInterface() { $this->assertInstanceOf(Container::class, Builder::new()->build()); } - public function testConstructingTheDefinitionsIsImmutable(): BlackBox\Proof + public function testConstructingTheDefinitionsIsImmutable() { - return $this - ->forAll(Set::strings()->unicode()) - ->prove(function($name) { - $container = Builder::new(); - $container2 = $container->add($name, static fn() => new \stdClass); + $container = Builder::new(); + $container2 = $container->add(Services::name, static fn() => new \stdClass); - $this->assertInstanceOf(Builder::class, $container2); - $this->assertNotSame($container2, $container); - $this->assertInstanceOf(\stdClass::class, $container2->build()($name)); + $this->assertInstanceOf(Builder::class, $container2); + $this->assertNotSame($container2, $container); + $this->assertInstanceOf(\stdClass::class, $container2->build()(Services::name)); - try { - $container->build()($name); - $this->fail('it should throw'); - } catch (\Exception $e) { - $this->assertInstanceOf(ServiceNotFound::class, $e); - $this->assertSame($name, $e->getMessage()); - } - }); + try { + $container->build()(Services::name); + $this->fail('it should throw'); + } catch (\Exception $e) { + $this->assertInstanceOf(ServiceNotFound::class, $e); + $this->assertSame(\spl_object_hash(Services::name), $e->getMessage()); + } } - public function testServiceIsOnlyBuiltOnce(): BlackBox\Proof + public function testServiceIsOnlyBuiltOnce() { - return $this - ->forAll(Set::strings()->unicode()) - ->prove(function($name) { - $container = Builder::new() - ->add($name, static fn() => new \stdClass) - ->build(); + $container = Builder::new() + ->add(Services::name, static fn() => new \stdClass) + ->build(); - $this->assertSame($container($name), $container($name)); - }); + $this->assertSame($container(Services::name), $container(Services::name)); } - public function testDependenciesCanBeAccesedWhenBuildingService(): BlackBox\Proof + public function testDependenciesCanBeAccesedWhenBuildingService() { - return $this - ->forAll( - Set::strings()->unicode(), - Set::strings()->unicode(), - ) - ->filter(static fn($a, $b) => $a !== $b) - ->prove(function($name, $dependency) { - $container = Builder::new() - ->add($name, static fn($get) => $get($dependency)) - ->add($dependency, static fn() => new \stdClass) - ->build(); + $container = Builder::new() + ->add(Services::name, static fn($get) => $get(Services::dependency)) + ->add(Services::dependency, static fn() => new \stdClass) + ->build(); - $this->assertSame($container($dependency), $container($name)); - }); + $this->assertSame($container(Services::dependency), $container(Services::name)); } - public function testCircularDependenciesAreIntercepted(): BlackBox\Proof + public function testCircularDependenciesAreIntercepted() { - return $this - ->forAll( - Set::strings()->unicode(), - Set::strings()->unicode(), - ) - ->filter(static fn($a, $b) => $a !== $b) - ->prove(function($name, $dependency) { - $container = Builder::new() - ->add($name, static fn($get) => $get($dependency)) - ->add($dependency, static fn($get) => $get($name)) - ->build(); + $container = Builder::new() + ->add(Services::name, static fn($get) => $get(Services::dependency)) + ->add(Services::dependency, static fn($get) => $get(Services::name)) + ->build(); - try { - $container($name); - $this->fail('it should throw'); - } catch (CircularDependency $e) { - $this->assertSame("$name > $dependency > $name", $e->getMessage()); - } - }); + try { + $container(Services::name); + $this->fail('it should throw'); + } catch (CircularDependency $e) { + $name = \spl_object_hash(Services::name); + $dependency = \spl_object_hash(Services::dependency); + $this->assertSame("$name > $dependency > $name", $e->getMessage()); + } } public function testEnumCaseCanBeUsedToReferenceAService() From 415ae62986d6546311994e1f9a9f7514f5ea89d4 Mon Sep 17 00:00:00 2001 From: Baptiste Langlade Date: Sat, 23 Aug 2025 21:43:22 +0200 Subject: [PATCH 2/4] use a Map to hold definitions --- composer.json | 3 ++- src/Builder.php | 17 ++++++----------- src/Container.php | 32 ++++++++++++++------------------ 3 files changed, 22 insertions(+), 30 deletions(-) diff --git a/composer.json b/composer.json index 18511b6..b6cefcd 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,8 @@ "issues": "http://github.com/innmind/di/issues" }, "require": { - "php": "~8.2" + "php": "~8.2", + "innmind/immutable": "~5.18" }, "autoload": { "psr-4": { diff --git a/src/Builder.php b/src/Builder.php index e9ba694..2a5620c 100644 --- a/src/Builder.php +++ b/src/Builder.php @@ -3,20 +3,18 @@ namespace Innmind\DI; +use Innmind\Immutable\Map; + /** * @psalm-immutable */ final class Builder { - /** @var array */ - private array $definitions = []; - /** - * @param array $definitions + * @param Map $definitions */ - private function __construct(array $definitions) + private function __construct(private Map $definitions) { - $this->definitions = $definitions; } /** @@ -25,7 +23,7 @@ private function __construct(array $definitions) #[\NoDiscard] public static function new(): self { - return new self([]); + return new self(Map::of()); } /** @@ -37,10 +35,7 @@ public static function new(): self #[\NoDiscard] public function add(Service $name, callable $definition): self { - $definitions = $this->definitions; - $definitions[\spl_object_hash($name)] = $definition; - - return new self($definitions); + return new self($this->definitions->put($name, $definition)); } #[\NoDiscard] diff --git a/src/Container.php b/src/Container.php index b352f94..2a1e0e1 100644 --- a/src/Container.php +++ b/src/Container.php @@ -7,11 +7,10 @@ ServiceNotFound, CircularDependency, }; +use Innmind\Immutable\Map; final class Container { - /** @var array */ - private array $definitions; /** @var array */ private array $services = []; /** @var list */ @@ -20,11 +19,10 @@ final class Container /** * @psalm-mutation-free * - * @param array $definitions + * @param Map $definitions */ - private function __construct(array $definitions) + private function __construct(private Map $definitions) { - $this->definitions = $definitions; } /** @@ -39,17 +37,15 @@ private function __construct(array $definitions) */ public function __invoke(Service $name): object { - $name = \spl_object_hash($name); + $hash = \spl_object_hash($name); + $definition = $this->definitions->get($name)->match( + static fn($definition) => $definition, + static fn() => throw new ServiceNotFound($hash), + ); - /** @psalm-suppress PossiblyInvalidArgument */ - if (!\array_key_exists($name, $this->definitions)) { - /** @psalm-suppress PossiblyInvalidArgument */ - throw new ServiceNotFound($name); - } - - if (\in_array($name, $this->building, true)) { + if (\in_array($hash, $this->building, true)) { $path = $this->building; - $path[] = $name; + $path[] = $hash; $this->building = []; /** @psalm-suppress InvalidArgument */ @@ -57,14 +53,14 @@ public function __invoke(Service $name): object } /** @psalm-suppress InvalidPropertyAssignmentValue */ - $this->building[] = $name; + $this->building[] = $hash; try { /** * @psalm-suppress InvalidPropertyAssignmentValue * @var T */ - return $this->services[$name] ??= ($this->definitions[$name])($this); + return $this->services[$hash] ??= $definition($this); } finally { \array_pop($this->building); } @@ -73,9 +69,9 @@ public function __invoke(Service $name): object /** * @psalm-pure * - * @param array $definitions + * @param Map $definitions */ - public static function of(array $definitions): self + public static function of(Map $definitions): self { return new self($definitions); } From b725433e03deca262d2d67683021a72298111295 Mon Sep 17 00:00:00 2001 From: Baptiste Langlade Date: Sat, 23 Aug 2025 21:55:55 +0200 Subject: [PATCH 3/4] fix service names displayed in exception messages --- src/Container.php | 47 +++++++++++++++++++++++++++-------------- tests/ContainerTest.php | 6 ++---- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/Container.php b/src/Container.php index 2a1e0e1..56805ff 100644 --- a/src/Container.php +++ b/src/Container.php @@ -7,22 +7,27 @@ ServiceNotFound, CircularDependency, }; -use Innmind\Immutable\Map; +use Innmind\Immutable\{ + Map, + Sequence, + Str, +}; final class Container { /** @var array */ private array $services = []; - /** @var list */ - private array $building = []; /** * @psalm-mutation-free * * @param Map $definitions + * @param Sequence $building */ - private function __construct(private Map $definitions) - { + private function __construct( + private Map $definitions, + private Sequence $building, + ) { } /** @@ -37,32 +42,42 @@ private function __construct(private Map $definitions) */ public function __invoke(Service $name): object { - $hash = \spl_object_hash($name); $definition = $this->definitions->get($name)->match( static fn($definition) => $definition, - static fn() => throw new ServiceNotFound($hash), + static fn() => throw new ServiceNotFound(\sprintf( + '%s::%s', + $name::class, + $name->name, + )), ); - if (\in_array($hash, $this->building, true)) { - $path = $this->building; - $path[] = $hash; - $this->building = []; + if ($this->building->contains($name)) { + $path = $this->building->add($name); + $this->building = $this->building->clear(); /** @psalm-suppress InvalidArgument */ - throw new CircularDependency(\implode(' > ', $path)); + throw new CircularDependency( + Str::of(' > ') + ->join($path->map(static fn($service) => \sprintf( + '%s::%s', + $service::class, + $service->name, + ))) + ->toString(), + ); } /** @psalm-suppress InvalidPropertyAssignmentValue */ - $this->building[] = $hash; + $this->building = $this->building->add($name); try { /** * @psalm-suppress InvalidPropertyAssignmentValue * @var T */ - return $this->services[$hash] ??= $definition($this); + return $this->services[\spl_object_hash($name)] ??= $definition($this); } finally { - \array_pop($this->building); + $this->building = $this->building->dropEnd(1); } } @@ -73,6 +88,6 @@ public function __invoke(Service $name): object */ public static function of(Map $definitions): self { - return new self($definitions); + return new self($definitions, Sequence::of()); } } diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index 4fc7556..47f7225 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -33,7 +33,7 @@ public function testConstructingTheDefinitionsIsImmutable() $this->fail('it should throw'); } catch (\Exception $e) { $this->assertInstanceOf(ServiceNotFound::class, $e); - $this->assertSame(\spl_object_hash(Services::name), $e->getMessage()); + $this->assertSame('Fixtures\Innmind\DI\Services::name', $e->getMessage()); } } @@ -67,9 +67,7 @@ public function testCircularDependenciesAreIntercepted() $container(Services::name); $this->fail('it should throw'); } catch (CircularDependency $e) { - $name = \spl_object_hash(Services::name); - $dependency = \spl_object_hash(Services::dependency); - $this->assertSame("$name > $dependency > $name", $e->getMessage()); + $this->assertSame("Fixtures\Innmind\DI\Services::name > Fixtures\Innmind\DI\Services::dependency > Fixtures\Innmind\DI\Services::name", $e->getMessage()); } } From 0bef6599f9b042fe71494889e413ee62f503d3d4 Mon Sep 17 00:00:00 2001 From: Baptiste Langlade Date: Sat, 23 Aug 2025 21:57:32 +0200 Subject: [PATCH 4/4] mention service name fix --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1bea6f1..8fd2d10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Using a `string` as a service name +### Fixed + +- Service names in exception messages that used object hashes + ## 2.1.0 - 2024-03-24 ### Added