fix(container): detect nested circular dependencies

packages/container/src/DependencyChain.php

@@ -32,6 +32,19 @@ public function add(Reflector|Closure|string $dependency): self
         return $this;
     }
 
+    /**
+     * Removes the last added dependency from the active resolution chain.
+     *
+     * Because dependencies are resolved in nested order, the most recently
+     * added item is always the one that just finished.
+     */
+    public function pop(): self
+    {
+        array_pop($this->dependencies);
+
+        return $this;
+    }
+
     public function first(): Dependency
     {
         return array_first($this->dependencies);

packages/container/src/GenericContainer.php

@@ -207,13 +207,15 @@ public function config(object $config): self
      */
     public function get(string $className, string|UnitEnum|null $tag = null, mixed ...$params): object
     {
-        $this->resolveChain();
-
-        $dependency = $this->resolve($className, $tag, ...$params);
+        $previousChain = $this->chain;
 
-        $this->stopChain();
+        $this->resolveChain();
 
-        return $dependency;
+        try {
+            return $this->resolve($className, $tag, ...$params);
+        } finally {
+            $this->stopChain($previousChain);
+        }
     }
 
     public function invoke(ClassReflector|MethodReflector|FunctionReflector|callable|string $method, mixed ...$params): mixed
@@ -250,43 +252,55 @@ public function invoke(ClassReflector|MethodReflector|FunctionReflector|callable
 
     private function invokeClosure(Closure $closure, mixed ...$params): mixed
     {
-        $this->resolveChain();
+        $previousChain = $this->chain;
 
-        $parameters = $this->autowireDependencies(
-            method: $reflector = new FunctionReflector($closure),
-            parameters: $params,
-        );
+        $this->resolveChain();
 
-        $this->stopChain();
+        try {
+            $parameters = $this->autowireDependencies(
+                method: $reflector = new FunctionReflector($closure),
+                parameters: $params,
+            );
+        } finally {
+            $this->stopChain($previousChain);
+        }
 
         return $reflector->invokeArgs($parameters);
     }
 
     private function invokeMethod(MethodReflector $method, mixed ...$params): mixed
     {
-        $this->resolveChain();
+        $previousChain = $this->chain;
 
-        $object = $this->get($method->getDeclaringClass()->getName());
+        $this->resolveChain();
 
-        $parameters = $this->autowireDependencies($method, $params);
+        try {
+            $object = $this->get($method->getDeclaringClass()->getName());
 
-        $this->stopChain();
+            $parameters = $this->autowireDependencies($method, $params);
+        } finally {
+            $this->stopChain($previousChain);
+        }
 
         return $method->invokeArgs($object, $parameters);
     }
 
     private function invokeFunction(FunctionReflector|Closure $callback, mixed ...$params): mixed
     {
+        $previousChain = $this->chain;
+
         $this->resolveChain();
 
         $reflector = match (true) {
             $callback instanceof FunctionReflector => $callback,
             default => new ReflectionFunction($callback),
         };
 
-        $parameters = $this->autowireDependencies($reflector, $params);
-
-        $this->stopChain();
+        try {
+            $parameters = $this->autowireDependencies($reflector, $params);
+        } finally {
+            $this->stopChain($previousChain);
+        }
 
         return $reflector->invokeArgs($parameters);
     }
@@ -371,20 +385,23 @@ private function resolveDependency(string $className, string|UnitEnum|null $tag
 
         $dependencyName = $this->resolveTaggedName($className, $tag);
 
-        // Check if a resolved singleton is present
+        // Check if a resolved singleton is present. A singleton is only registered once it is fully
+        // constructed, so it can never be in flight and cannot take part in a cycle.
         if ($instance = $this->resolvedSingletons[$dependencyName] ?? null) {
-            $this->resolveChain()->add($class);
-
             return $instance;
         }
 
         // Check if the class has been registered as a singleton.
         if ($singletonDefinition = $this->singletonDefinitions[$dependencyName] ?? null) {
-            $instance = $singletonDefinition instanceof Closure ? $singletonDefinition($this) : $singletonDefinition;
+            $this->resolveChain()->add($class);
 
-            $this->resolvedSingletons[$dependencyName] = $instance;
+            try {
+                $instance = $singletonDefinition instanceof Closure ? $singletonDefinition($this) : $singletonDefinition;
+            } finally {
+                $this->resolveChain()->pop();
+            }
 
-            $this->resolveChain()->add($class);
+            $this->resolvedSingletons[$dependencyName] = $instance;
 
             return $instance;
         }
@@ -393,7 +410,11 @@ private function resolveDependency(string $className, string|UnitEnum|null $tag
         if ($definition = $this->definitions[$dependencyName] ?? null) {
             $this->resolveChain()->add(new FunctionReflector($definition));
 
-            return $definition($this);
+            try {
+                return $definition($this);
+            } finally {
+                $this->resolveChain()->pop();
+            }
         }
 
         // Next we check if any of our default initializers can initialize this class.
@@ -402,10 +423,14 @@ private function resolveDependency(string $className, string|UnitEnum|null $tag
 
             $this->resolveChain()->add($initializerClass);
 
-            $object = match (true) {
-                $initializer instanceof Initializer => $initializer->initialize($this->clone()),
-                $initializer instanceof DynamicInitializer => $initializer->initialize($class, $tag, $this->clone()),
-            };
+            try {
+                $object = match (true) {
+                    $initializer instanceof Initializer => $initializer->initialize($this->clone()),
+                    $initializer instanceof DynamicInitializer => $initializer->initialize($class, $tag, $this->clone()),
+                };
+            } finally {
+                $this->resolveChain()->pop();
+            }
 
             $singleton = $initializerClass->getAttribute(Singleton::class) ?? $initializerClass->getMethod('initialize')->getAttribute(Singleton::class);
 
@@ -543,12 +568,16 @@ private function autowireDependencies(MethodReflector|FunctionReflector $method,
 
         // Build the class by iterating through its
         // dependencies and resolving them.
-        foreach ($method->getParameters() as $parameter) {
-            $dependencies[] = $this->clone()->autowireDependency(
-                parameter: $parameter,
-                tag: $parameter->getAttribute(Tag::class)?->name,
-                providedValue: $parameters[$parameter->getName()] ?? null,
-            );
+        try {
+            foreach ($method->getParameters() as $parameter) {
+                $dependencies[] = $this->clone()->autowireDependency(
+                    parameter: $parameter,
+                    tag: $parameter->getAttribute(Tag::class)?->name,
+                    providedValue: $parameters[$parameter->getName()] ?? null,
+                );
+            }
+        } finally {
+            $this->resolveChain()->pop();
         }
 
         return $dependencies;
@@ -679,9 +708,12 @@ private function resolveChain(): DependencyChain
         return $this->chain;
     }
 
-    private function stopChain(): void
+    /**
+     * Restores the parent resolution chain, or resets it to `null` at the top level.
+     */
+    private function stopChain(?DependencyChain $previous = null): void
     {
-        $this->chain = null;
+        $this->chain = $previous;
     }
 
     public function __clone(): void

packages/container/tests/Exceptions/CircularDependencyExceptionTest.php

@@ -10,6 +10,11 @@
 use Tempest\Container\GenericContainer;
 use Tempest\Container\Tests\Fixtures\CircularA;
 use Tempest\Container\Tests\Fixtures\CircularZ;
+use Tempest\Container\Tests\Fixtures\NestedResolutionA;
+use Tempest\Container\Tests\Fixtures\NestedResolutionAInitializer;
+use Tempest\Container\Tests\Fixtures\NestedResolutionBInitializer;
+use Tempest\Container\Tests\Fixtures\ResolvedTwiceParent;
+use Tempest\Container\Tests\Fixtures\ResolvedTwiceParentInitializer;
 
 /**
  * @internal
@@ -74,4 +79,27 @@ public function circular_dependency_as_a_child_test(): void
             throw $circularDependencyException;
         }
     }
+
+    #[Test]
+    public function circular_dependency_after_a_nested_resolution_test(): void
+    {
+        $this->expectException(CircularDependencyEncountered::class);
+
+        $container = new GenericContainer();
+        $container->addInitializer(NestedResolutionAInitializer::class);
+        $container->addInitializer(NestedResolutionBInitializer::class);
+
+        $container->get(NestedResolutionA::class);
+    }
+
+    #[Test]
+    public function the_same_dependency_resolved_twice_in_one_chain_is_not_circular_test(): void
+    {
+        $container = new GenericContainer();
+        $container->addInitializer(ResolvedTwiceParentInitializer::class);
+
+        $parent = $container->get(ResolvedTwiceParent::class);
+
+        $this->assertInstanceOf(ResolvedTwiceParent::class, $parent);
+    }
 }

packages/container/tests/Fixtures/NestedResolutionA.php

@@ -0,0 +1,7 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Container\Tests\Fixtures;
+
+final class NestedResolutionA {}

packages/container/tests/Fixtures/NestedResolutionAInitializer.php

@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Container\Tests\Fixtures;
+
+use Tempest\Container\Container;
+use Tempest\Container\Initializer;
+
+final readonly class NestedResolutionAInitializer implements Initializer
+{
+    public function initialize(Container $container): NestedResolutionA
+    {
+        // This nested resolution used to reset the chain, hiding the cycle below.
+        $container->get(Unrelated::class);
+        $container->get(NestedResolutionB::class);
+
+        return new NestedResolutionA();
+    }
+}

packages/container/tests/Fixtures/NestedResolutionB.php

@@ -0,0 +1,7 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Container\Tests\Fixtures;
+
+final class NestedResolutionB {}

packages/container/tests/Fixtures/NestedResolutionBInitializer.php

@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Container\Tests\Fixtures;
+
+use Tempest\Container\Container;
+use Tempest\Container\Initializer;
+
+final readonly class NestedResolutionBInitializer implements Initializer
+{
+    public function initialize(Container $container): NestedResolutionB
+    {
+        $container->get(NestedResolutionA::class);
+
+        return new NestedResolutionB();
+    }
+}

packages/container/tests/Fixtures/ResolvedTwiceLeaf.php

@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Container\Tests\Fixtures;
+
+final readonly class ResolvedTwiceLeaf
+{
+    public function __construct(
+        public Unrelated $unrelated,
+    ) {}
+}

packages/container/tests/Fixtures/ResolvedTwiceParent.php

@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Container\Tests\Fixtures;
+
+final readonly class ResolvedTwiceParent
+{
+    public function __construct(
+        public ResolvedTwiceLeaf $first,
+        public ResolvedTwiceLeaf $second,
+    ) {}
+}

packages/container/tests/Fixtures/ResolvedTwiceParentInitializer.php

@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Container\Tests\Fixtures;
+
+use Tempest\Container\Container;
+use Tempest\Container\Initializer;
+
+final readonly class ResolvedTwiceParentInitializer implements Initializer
+{
+    public function initialize(Container $container): ResolvedTwiceParent
+    {
+        // One shared chain - constructor parameters each resolve against their own clone of it.
+        return new ResolvedTwiceParent(
+            $container->get(ResolvedTwiceLeaf::class),
+            $container->get(ResolvedTwiceLeaf::class),
+        );
+    }
+}

packages/container/tests/Fixtures/Unrelated.php

@@ -0,0 +1,7 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Container\Tests\Fixtures;
+
+final class Unrelated {}