Remove support to use a string as a service name

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## [Unreleased]
+
+### Removed
+
+- Using a `string` as a service name
+
+### Fixed
+
+- Service names in exception messages that used object hashes
+
 ## 2.1.0 - 2024-03-24
 
 ### Added

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": {

fixtures/Services.php

@@ -12,6 +12,8 @@
 enum Services implements Service
 {
     case a;
+    case name;
+    case dependency;
 
     /**
      * @return self<\Exception>

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();

src/Builder.php

@@ -3,20 +3,18 @@
 
 namespace Innmind\DI;
 
+use Innmind\Immutable\Map;
+
 /**
  * @psalm-immutable
  */
 final class Builder
 {
-    /** @var array<string, callable(Container): object> */
-    private array $definitions = [];
-
     /**
-     * @param array<string, callable(Container): object> $definitions
+     * @param Map<Service, callable(Container): object> $definitions
      */
-    private function __construct(array $definitions)
+    private function __construct(private Map $definitions)
     {
-        $this->definitions = $definitions;
     }
 
     /**
@@ -25,24 +23,19 @@ private function __construct(array $definitions)
     #[\NoDiscard]
     public static function new(): self
     {
-        return new self([]);
+        return new self(Map::of());
     }
 
     /**
-     * @param string|Service $name Using a string is deprecated
-     * @param callable(Container): object $definition
+     * @template T of object
+     *
+     * @param Service<T> $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;
-
-        return new self($definitions);
+        return new self($this->definitions->put($name, $definition));
     }
 
     #[\NoDiscard]

src/Container.php

@@ -7,76 +7,87 @@
     ServiceNotFound,
     CircularDependency,
 };
+use Innmind\Immutable\{
+    Map,
+    Sequence,
+    Str,
+};
 
 final class Container
 {
-    /** @var array<string, callable(self): object> */
-    private array $definitions;
     /** @var array<string, object> */
     private array $services = [];
-    /** @var list<string> */
-    private array $building = [];
 
     /**
      * @psalm-mutation-free
      *
-     * @param array<string, callable(self): object> $definitions
+     * @param Map<Service, callable(self): object> $definitions
+     * @param Sequence<Service> $building
      */
-    private function __construct(array $definitions)
-    {
-        $this->definitions = $definitions;
+    private function __construct(
+        private Map $definitions,
+        private Sequence $building,
+    ) {
     }
 
     /**
      * @template T of object
-     * @template N of string|Service<T>
      *
-     * @param N $name
+     * @param Service<T> $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);
-        }
-
-        /** @psalm-suppress PossiblyInvalidArgument */
-        if (!\array_key_exists($name, $this->definitions)) {
-            /** @psalm-suppress PossiblyInvalidArgument */
-            throw new ServiceNotFound($name);
-        }
+        $definition = $this->definitions->get($name)->match(
+            static fn($definition) => $definition,
+            static fn() => throw new ServiceNotFound(\sprintf(
+                '%s::%s',
+                $name::class,
+                $name->name,
+            )),
+        );
 
-        if (\in_array($name, $this->building, true)) {
-            $path = $this->building;
-            $path[] = $name;
-            $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[] = $name;
+        $this->building = $this->building->add($name);
 
         try {
-            /** @psalm-suppress InvalidPropertyAssignmentValue */
-            return $this->services[$name] ??= ($this->definitions[$name])($this);
+            /**
+             * @psalm-suppress InvalidPropertyAssignmentValue
+             * @var T
+             */
+            return $this->services[\spl_object_hash($name)] ??= $definition($this);
         } finally {
-            \array_pop($this->building);
+            $this->building = $this->building->dropEnd(1);
         }
     }
 
     /**
      * @psalm-pure
      *
-     * @param array<string, callable(self): object> $definitions
+     * @param Map<Service, callable(self): object> $definitions
      */
-    public static function of(array $definitions): self
+    public static function of(Map $definitions): self
     {
-        return new self($definitions);
+        return new self($definitions, Sequence::of());
     }
 }

tests/ContainerTest.php

@@ -9,96 +9,66 @@
     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('Fixtures\Innmind\DI\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) {
+            $this->assertSame("Fixtures\Innmind\DI\Services::name > Fixtures\Innmind\DI\Services::dependency > Fixtures\Innmind\DI\Services::name", $e->getMessage());
+        }
     }
 
     public function testEnumCaseCanBeUsedToReferenceAService()