diff --git a/docs/best-practices/controllers.md b/docs/best-practices/controllers.md index 3744783..a4b95e5 100644 --- a/docs/best-practices/controllers.md +++ b/docs/best-practices/controllers.md @@ -190,8 +190,6 @@ covers most common use cases: * Each class may or may not have a constructor. * If the constructor has an optional argument, it will be omitted unless an explicit [container configuration](#container-configuration) is used. -* If the constructor has a nullable argument, it will be given a `null` value - unless an explicit [container configuration](#container-configuration) is used. * If the constructor references another class, it will load this class next. This covers most common use cases where the request handler class uses a @@ -325,27 +323,6 @@ some manual configuration like this: ]); - $app = new FrameworkX\App($container); - - // … - ``` - -=== "Nullable values" - - ```php title="public/index.php" - function (?string $name) { - // example UserController class uses $name, defaults to null if not set - return new Acme\Todo\UserController($name ?? 'ACME'); - }, - 'name' => 'Demo' - ]); - - $app = new FrameworkX\App($container); // … @@ -415,7 +392,7 @@ all uppercase in any factory function like this: // Framework X also uses environment variables internally. // You may explicitly configure this built-in functionality like this: // 'X_LISTEN' => '0.0.0.0:8081' - // 'X_LISTEN' => fn(?string $PORT = '8080') => '0.0.0.0:' . $PORT + // 'X_LISTEN' => fn(string $PORT = '8080') => '0.0.0.0:' . $PORT 'X_LISTEN' => '127.0.0.1:8080' ]); diff --git a/src/Container.php b/src/Container.php index 387d4d5..01c7e14 100644 --- a/src/Container.php +++ b/src/Container.php @@ -296,9 +296,6 @@ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool if ($parameter->isDefaultValueAvailable()) { return $parameter->getDefaultValue(); } - if ($type->allowsNull()) { - return null; - } throw new \Error( self::parameterError($parameter, $for) . ' expects unsupported type ' . $type @@ -321,14 +318,12 @@ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool ); } - // use default/nullable argument if not loadable as container variable or by type - if (!$type instanceof \ReflectionNamedType || $type->isBuiltin() || !\array_key_exists($type->getName(), $this->container)) { - if ($parameter->isDefaultValueAvailable()) { - return $parameter->getDefaultValue(); - } - if ($type !== null && $type->allowsNull() && $type->getName() !== 'mixed') { - return null; - } + // use default argument if not loadable as container variable or by type + if ( + $parameter->isDefaultValueAvailable() && + (!$type instanceof \ReflectionNamedType || $type->isBuiltin() || !\array_key_exists($type->getName(), $this->container)) + ) { + return $parameter->getDefaultValue(); } // abort if required container variable is not defined or for any other primitive types (array etc.) diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index c63a130..c0db268 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -68,36 +68,6 @@ public function __invoke(ServerRequestInterface $request): Response $this->assertEquals('{"name":"Alice"}', (string) $response->getBody()); } - public function testCallableReturnsCallableForNullableClassViaAutowiringWillDefaultToNullValue(): void - { - $request = new ServerRequest('GET', 'http://example.com/'); - - $controller = new class(new \stdClass()) { - /** @var ?\stdClass */ - private $data; - - public function __construct(?\stdClass $data) - { - $this->data = $data; - } - - public function __invoke(ServerRequestInterface $request): Response - { - return new Response(200, [], (string) json_encode($this->data)); - } - }; - - $container = new Container([]); - - $callable = $container->callable(get_class($controller)); - $this->assertInstanceOf(\Closure::class, $callable); - - $response = $callable($request); - $this->assertInstanceOf(ResponseInterface::class, $response); - $this->assertEquals(200, $response->getStatusCode()); - $this->assertEquals('null', (string) $response->getBody()); - } - public function testCallableReturnsCallableForNullableClassViaContainerConfiguration(): void { $request = new ServerRequest('GET', 'http://example.com/'); @@ -130,37 +100,6 @@ public function __invoke(ServerRequestInterface $request): Response $this->assertEquals('{}', (string) $response->getBody()); } - /** - * @requires PHP 8 - */ - public function testCallableReturnsCallableForUnionWithNullViaAutowiringWillDefaultToNullValue(): void - { - $request = new ServerRequest('GET', 'http://example.com/'); - - // @phpstan-ignore-next-line for PHP < 8 - $controller = new class(null) { - /** @var mixed */ - private $data = false; - - #[PHP8] public function __construct(string|int|null $data) { $this->data = $data; } // @phpstan-ignore-line - - public function __invoke(ServerRequestInterface $request): Response - { - return new Response(200, [], (string) json_encode($this->data)); - } - }; - - $container = new Container([]); - - $callable = $container->callable(get_class($controller)); - $this->assertInstanceOf(\Closure::class, $callable); - - $response = $callable($request); - $this->assertInstanceOf(ResponseInterface::class, $response); - $this->assertEquals(200, $response->getStatusCode()); - $this->assertEquals('null', (string) $response->getBody()); - } - public function testCallableReturnsCallableForClassWithNullDefaultViaAutowiringWillDefaultToNullValue(): void { $request = new ServerRequest('GET', 'http://example.com/'); @@ -843,7 +782,7 @@ public function __invoke(): ResponseInterface ResponseInterface::class => function (?\stdClass $user, ?\stdClass $data) { return new Response(200, [], (string) json_encode(['user' => $user, 'data' => $data])); }, - 'user' => (object) [] + 'user' => (object) ['name' => 'Alice'] ]); $callable = $container->callable(get_class($controller)); @@ -852,7 +791,7 @@ public function __invoke(): ResponseInterface $response = $callable($request); $this->assertInstanceOf(ResponseInterface::class, $response); $this->assertEquals(200, $response->getStatusCode()); - $this->assertEquals('{"user":{},"data":null}', (string) $response->getBody()); + $this->assertEquals('{"user":{"name":"Alice"},"data":{}}', (string) $response->getBody()); } public function testCallableReturnsCallableForClassNameWithDependencyMappedWithFactoryThatRequiresNullableContainerVariablesWithFactory(): void @@ -879,7 +818,7 @@ public function __invoke(): ResponseInterface return new Response(200, [], (string) json_encode(['user' => $user, 'data' => $data])); }, 'user' => function (): ?\stdClass { // @phpstan-ignore-line - return (object) []; + return (object) ['name' => 'Alice']; } ]); @@ -889,7 +828,7 @@ public function __invoke(): ResponseInterface $response = $callable($request); $this->assertInstanceOf(ResponseInterface::class, $response); $this->assertEquals(200, $response->getStatusCode()); - $this->assertEquals('{"user":{},"data":null}', (string) $response->getBody()); + $this->assertEquals('{"user":{"name":"Alice"},"data":{}}', (string) $response->getBody()); } public function testCallableReturnsCallableForClassNameWithDependencyMappedWithFactoryThatRequiresContainerVariablesWithDefaultValues(): void @@ -1153,40 +1092,6 @@ public function __invoke(): ResponseInterface $this->assertEquals('"bar"', (string) $response->getBody()); } - public function testCallableReturnsCallableForClassNameWithDependencyMappedWithFactoryThatRequiresNullableStringEnvironmentVariableAssignsNull(): void - { - $request = new ServerRequest('GET', 'http://example.com/'); - - $controller = new class(new Response()) { - /** @var ResponseInterface */ - private $response; - - public function __construct(ResponseInterface $response) - { - $this->response = $response; - } - - public function __invoke(): ResponseInterface - { - return $this->response; - } - }; - - $container = new Container([ - ResponseInterface::class => function (?string $FOO) { - return new Response(200, [], (string) json_encode($FOO)); - } - ]); - - $callable = $container->callable(get_class($controller)); - $this->assertInstanceOf(\Closure::class, $callable); - - $response = $callable($request); - $this->assertInstanceOf(ResponseInterface::class, $response); - $this->assertEquals(200, $response->getStatusCode()); - $this->assertEquals('null', (string) $response->getBody()); - } - public function testCallableReturnsCallableForClassNameWithDependencyMappedWithFactoryThatRequiresUntypedEnvironmentVariable(): void { $request = new ServerRequest('GET', 'http://example.com/'); @@ -1317,6 +1222,31 @@ public function __construct(\stdClass $data) $callable($request); } + public function testCallableReturnsCallableThatThrowsWhenFactoryReferencesUnknownNullableVariable(): void + { + $request = new ServerRequest('GET', 'http://example.com/'); + + $controller = new class(new \stdClass()) { + public function __construct(\stdClass $data) + { + assert($data instanceof \stdClass); + } + }; + + $line = __LINE__ + 2; + $container = new Container([ + \stdClass::class => function (?string $username) { + return (object) ['name' => $username]; + } + ]); + + $callable = $container->callable(get_class($controller)); + + $this->expectException(\Error::class); + $this->expectExceptionMessage('Argument #1 ($username) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass requires container config with type ?string, none given'); + $callable($request); + } + public function testCallableReturnsCallableThatThrowsWhenFactoryReferencesRecursiveVariable(): void { $request = new ServerRequest('GET', 'http://example.com/'); @@ -1973,6 +1903,82 @@ public function testGetEnvReturnsStringFromMapFactory(): void $this->assertEquals('bar', $container->getEnv('X_FOO')); } + /** + * @requires PHP 8 + */ + public function testGetEnvReturnsNullFromFactoryForUnsupportedUnionVariableWithNullDefaultEvenForKnownVariable(): void + { + $fn = null; + $fn = #[PHP8] function (string|int|null $bar = null) { return $bar; }; + $container = new Container([ + 'X_FOO' => $fn, + 'bar' => 'ignored' + ]); + + $this->assertNull($container->getEnv('X_FOO')); + } + + /** + * @requires PHP 8 + */ + public function testGetEnvReturnsStringFromFactoryForUnsupportedUnionVariableWithStringDefaultEvenForKnownVariable(): void + { + $fn = null; + $fn = #[PHP8] function (string|int|null $bar = 'default') { return $bar; }; + $container = new Container([ + 'X_FOO' => $fn, + 'bar' => 'ignored' + ]); + + $this->assertEquals('default', $container->getEnv('X_FOO')); + } + + public function testGetEnvReturnsNullFromFactoryForUnknownNullableVariableWithNullDefault(): void + { + $container = new Container([ + 'X_FOO' => function (?string $X_UNDEFINED = null) { return $X_UNDEFINED; } + ]); + + $this->assertNull($container->getEnv('X_FOO')); + } + + public function testGetEnvReturnsStringFromFactoryForUnknownVariableWithStringDefault(): void + { + $container = new Container([ + 'X_FOO' => function (string $X_UNDEFINED = 'foo') { return $X_UNDEFINED; } + ]); + + $this->assertEquals('foo', $container->getEnv('X_FOO')); + } + + /** + * @requires PHP 8 + */ + public function testGetEnvReturnsNullFromFactoryForUnknownAndUnsupportedUnionVariableWithNullDefault(): void + { + $fn = null; + $fn = #[PHP8] function (string|int|null $X_UNDEFINED = null) { return $X_UNDEFINED; }; + $container = new Container([ + 'X_FOO' => $fn + ]); + + $this->assertNull($container->getEnv('X_FOO')); + } + + /** + * @requires PHP 8 + */ + public function testGetEnvReturnsStringFromFactoryForUnknownAndUnsupportedUnionVariableWithStringDefault(): void + { + $fn = null; + $fn = #[PHP8] function (string|int|null $X_UNDEFINED = 'default') { return $X_UNDEFINED; }; + $container = new Container([ + 'X_FOO' => $fn + ]); + + $this->assertEquals('default', $container->getEnv('X_FOO')); + } + public function testGetEnvReturnsStringFromGlobalEnvIfNotSetInMap(): void { $container = new Container([]); @@ -2233,6 +2239,33 @@ public function testGetEnvThrowsWhenFactoryFunctionExpectsRequiredEnvVariableBut $container->getEnv('X_FOO'); } + public function testGetEnvThrowsWhenFactoryFunctionExpectsRequiredNullableEnvVariableButNoneGiven(): void + { + $line = __LINE__ + 2; + $container = new Container([ + 'X_FOO' => function (?string $X_UNDEFINED) { return $X_UNDEFINED; } + ]); + + $this->expectException(\Error::class); + $this->expectExceptionMessage('Argument #1 ($X_UNDEFINED) of {closure:' . __FILE__ . ':' . $line . '}() for $X_FOO requires container config with type ?string, none given'); + $container->getEnv('X_FOO'); + } + + /** + * @requires PHP 8 + */ + public function testGetEnvThrowsWhenFactoryFunctionExpectsRequiredMixedEnvVariableButNoneGiven(): void + { + $line = __LINE__ + 2; + $container = new Container([ + 'X_FOO' => function (mixed $X_UNDEFINED) { return $X_UNDEFINED; } + ]); + + $this->expectException(\Error::class); + $this->expectExceptionMessage('Argument #1 ($X_UNDEFINED) of {closure:' . __FILE__ . ':' . $line . '}() for $X_FOO requires container config with type mixed, none given'); + $container->getEnv('X_FOO'); + } + public function testGetEnvThrowsWhenFactoryFunctionExpectsNullableIntArgumentButGivenString(): void { $line = __LINE__ + 2; @@ -2264,6 +2297,23 @@ public function testGetEnvThrowsWhenFactoryFunctionExpectsUnsupportedUnionType() $container->getEnv('X_FOO'); } + /** + * @requires PHP 8 + */ + public function testGetEnvThrowsWhenFactoryFunctionExpectsNullableUnionType(): void + { + $line = __LINE__ + 2; + $fn = null; + $fn = #[PHP8] function (string|int|null $X_UNDEFINED) { return $X_UNDEFINED; }; + $container = new Container([ + 'X_FOO' => $fn + ]); + + $this->expectException(\Error::class); + $this->expectExceptionMessage('Argument #1 ($X_UNDEFINED) of {closure:' . __FILE__ . ':' . $line . '}() for $X_FOO expects unsupported type string|int|null'); + $container->getEnv('X_FOO'); + } + /** @link https://3v4l.org/VaFMd */ public function testGetEnvThrowsWhenFactoryFunctionExpectsIntArgumentButGivenAnonymousClass(): void { diff --git a/tests/Io/RouteHandlerTest.php b/tests/Io/RouteHandlerTest.php index a89fc79..69a3893 100644 --- a/tests/Io/RouteHandlerTest.php +++ b/tests/Io/RouteHandlerTest.php @@ -234,33 +234,6 @@ public function __invoke(): ?Response $this->assertSame($response, $ret); } - public function testHandleRequestWithGetRequestReturnsResponseFromMatchingHandlerClassNameWithNullableConstructor(): void - { - $request = new ServerRequest('GET', 'http://example.com/'); - $response = new Response(200, [], ''); - - $controller = new class(null) { - /** @var ?Response */ - public static $response; - public function __construct(?int $value) - { - assert($value === null); - } - public function __invoke(): ?Response - { - return self::$response; - } - }; - $controller::$response = $response; - - $handler = new RouteHandler(); - $handler->map(['GET'], '/', get_class($controller)); - - $ret = $handler($request); - - $this->assertSame($response, $ret); - } - public function testHandleRequestWithGetRequestReturnsResponseFromMatchingHandlerClassNameWithRequiredResponseInConstructor(): void { $request = new ServerRequest('GET', 'http://example.com/');