Skip to content

Allow recursive container calls from factory functions #301

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
clue merged 2 commits into from
Feb 16, 2026
Merged

Allow recursive container calls from factory functions #301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9af8af8
Allow recursive container calls from factory functions for variables
clue Dec 19, 2025
61b9193
Allow recursive container calls from factory functions for classes
clue Dec 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions src/Container.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,12 +245,17 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+)

$this->container[$name] = $value;
} elseif ($this->container[$name] instanceof \Closure) {
// build list of factory parameters based on parameter types
$closure = new \ReflectionFunction($this->container[$name]);
$params = $this->loadFunctionParams($closure, $depth, true, \explode("\0", $name)[0]);
$factory = $this->container[$name];
$closure = new \ReflectionFunction($factory);

// invoke factory with list of parameters
$value = $params === [] ? ($this->container[$name])() : ($this->container[$name])(...$params);
// temporarily unset factory reference to allow loading recursive variables from environment
try {
unset($this->container[$name]);
$value = $factory(...$this->loadFunctionParams($closure, $depth, true, \explode("\0", $name)[0]));
} finally {
Comment on lines 251 to +256
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says the factory is unset "to allow loading recursive variables from environment", but in this branch it's also enabling recursive getObject()/container lookups from within object factories. Updating the comment to reflect the broader intent would avoid confusion for future maintainers.

Copilot uses AI. Check for mistakes.
$this->container[$name] = $factory;
}

if (\is_string($value)) {
if ($depth < 1) {
Expand Down Expand Up @@ -395,17 +400,15 @@ private function loadVariable(string $name, int $depth = 64) /*: object|string|i
\assert($factory instanceof \Closure);
$closure = new \ReflectionFunction($factory);

// build list of factory parameters based on parameter types
// invoke factory with list of parameters
// temporarily unset factory reference to allow loading recursive variables from environment
try {
unset($this->container[$name]);
$params = $this->loadFunctionParams($closure, $depth - 1, true, '$' . $name);
$value = $factory(...$this->loadFunctionParams($closure, $depth - 1, true, '$' . $name));
} finally {
$this->container[$name] = $factory;
}

// invoke factory with list of parameters
$value = $factory(...$params);
if (!\is_object($value) && !\is_scalar($value) && $value !== null) {
throw new \TypeError(
'Return value of ' . self::functionName($closure) . ' for $' . $name . ' must be of type object|string|int|float|bool|null, ' . $this->gettype($value) . ' returned'
Expand Down
125 changes: 102 additions & 23 deletions tests/ContainerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1305,7 +1305,7 @@ public function __construct(\stdClass $data)
$callable = $container->callable(get_class($controller));

$this->expectException(\Error::class);
$this->expectExceptionMessage('Argument #1 ($stdClass) of {closure:' . __FILE__ . ':' . $line .'}() for $stdClass requires container config with type string, none given');
$this->expectExceptionMessage('Argument #1 ($stdClass) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass requires container config with type string, none given');
$callable($request);
}

Expand Down Expand Up @@ -1820,22 +1820,6 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUndefine
$callable($request);
}

public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresRecursiveClass(): void
{
$request = new ServerRequest('GET', 'http://example.com/');

$line = __LINE__ + 2;
$container = new Container([
\stdClass::class => function (\stdClass $data) { return $data; }
]);

$callable = $container->callable(\stdClass::class);

$this->expectException(\Error::class);
$this->expectExceptionMessage('Argument #1 ($data) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass is recursive');
$callable($request);
}

public function testCallableReturnsCallableThatThrowsWhenFactoryIsRecursive(): void
{
$request = new ServerRequest('GET', 'http://example.com/');
Expand Down Expand Up @@ -2126,6 +2110,28 @@ public function testGetEnvReturnsStringFromRecursiveFactoryWithDefaultValueIfNot
$this->assertEquals('foo', $container->getEnv('X_FOO'));
}

public function testGetEnvReturnsNullIfFactoryFunctionUsesRecursiveGetEnvForVariableNotSetInGlobalEnv(): void
{
$container = new Container([
'X_FOO' => function (Container $container) { return $container->getEnv('X_FOO'); }
]);

$this->assertNull($container->getEnv('X_FOO'));
}

public function testGetEnvReturnsStringIfFactoryFunctionUsesRecursiveGetEnvForVariableSetInGlobalEnv(): void
{
$container = new Container([
'X_FOO' => function (Container $container) { return $container->getEnv('X_FOO'); }
]);

$_ENV['X_FOO'] = 'foo';
$ret = $container->getEnv('X_FOO');
unset($_ENV['X_FOO']);

$this->assertEquals('foo', $ret);
}

public function testGetEnvReturnsStringFromPsrContainer(): void
{
$psr = $this->createMock(ContainerInterface::class);
Expand Down Expand Up @@ -2592,6 +2598,45 @@ public function testGetObjectReturnsAccessLogHandlerInstanceFromConfig(): void
$this->assertSame($accessLogHandler, $ret);
}

public function testGetObjectReturnsAccessLogHandlerInstanceFromFactoryFunction(): void
{
$accessLogHandler = new AccessLogHandler();

$container = new Container([
AccessLogHandler::class => function () use ($accessLogHandler) {
return $accessLogHandler;
}
]);

$ret = $container->getObject(AccessLogHandler::class);

$this->assertSame($accessLogHandler, $ret);
}

public function testGetObjectReturnsDefaultStdclassInstanceWhenFactoryFunctionHasRecursiveArgument(): void
{
$container = new Container([
\stdClass::class => function (\stdClass $object) { return $object; }
]);

$ret = $container->getObject(\stdClass::class);

$this->assertInstanceOf(\stdClass::class, $ret);
}

public function testGetObjectReturnsDefaultStdclassInstanceWhenFactoryFunctionUsesRecursiveGetObject(): void
{
$container = new Container([
\stdClass::class => function (Container $container) {
return $container->getObject(\stdClass::class);
}
]);

$ret = $container->getObject(\stdClass::class);

$this->assertInstanceOf(\stdClass::class, $ret);
}

public function testGetObjectReturnsSelfContainerByDefault(): void
{
$container = new Container([]);
Expand All @@ -2601,6 +2646,32 @@ public function testGetObjectReturnsSelfContainerByDefault(): void
$this->assertSame($container, $ret);
}

public function testGetObjectReturnsSelfContainerIfFactoryFunctionHasRecursiveContainerArgument(): void
{
$container = new Container([
Container::class => function (Container $container): Container {
return $container;
}
]);

$ret = $container->getObject(Container::class);

$this->assertSame($container, $ret);
}

public function testGetObjectReturnsSelfContainerIfFactoryFunctionUsesRecursiveGetObject(): void
{
$container = new Container([
Container::class => function (Container $container): Container {
return $container->getObject(Container::class);
}
]);

$ret = $container->getObject(Container::class);

$this->assertSame($container, $ret);
}

public function testGetObjectReturnsOtherContainerFromConfig(): void
{
$other = new Container();
Expand Down Expand Up @@ -2763,18 +2834,26 @@ public function testGetObjectThrowsIfFactoryFunctionIsRecursive(): void
$container->getObject(AccessLogHandler::class);
}

public function testGetObjectThrowsIfFactoryFunctionHasRecursiveContainerArgument(): void
public function testGetObjectThrowsIfConstructorRequiresContainerConfig(): void
{
$container = new Container([]);

$this->expectException(\Error::class);
$this->expectExceptionMessage('Argument #1 ($path) of ' . LogStreamHandler::class . '::__construct() requires container config with type string, none given');
$container->getObject(LogStreamHandler::class);
}

public function testGetObjectThrowsIfFactoryFunctionHasClassArgumentWithConstructorThatRequiresContainerConfig(): void
{
$line = __LINE__ + 2;
$container = new Container([
Container::class => function (Container $container): Container {
return $container;
AccessLogHandler::class => function (LogStreamHandler $log) {
return new AccessLogHandler();
}
]);

$this->expectException(\Error::class);
$this->expectExceptionMessage('Argument #1 ($container) of {closure:' . __FILE__ . ':' . $line .'}() for FrameworkX\Container is recursive');
$container->getObject(Container::class);
$this->expectExceptionMessage('Argument #1 ($path) of ' . LogStreamHandler::class . '::__construct() requires container config with type string, none given');
$container->getObject(AccessLogHandler::class);
}

public function testGetObjectThrowsIfConfigReferencesInterface(): void
Expand Down