From c7a3b7917e5689248948bd5fa76d92eb7bd7ddb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sat, 8 Mar 2025 18:54:48 +0100 Subject: [PATCH 1/8] Describe all potential `Container` exceptions --- src/Container.php | 34 ++++++++++++--- tests/ContainerTest.php | 91 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 5 deletions(-) diff --git a/src/Container.php b/src/Container.php index 3ea15fd..7b60f07 100644 --- a/src/Container.php +++ b/src/Container.php @@ -16,7 +16,10 @@ class Container /** @var bool */ private $useProcessEnv; - /** @param array|ContainerInterface $loader */ + /** + * @param array|ContainerInterface $loader + * @throws \TypeError|\BadMethodCallException if given $loader is invalid + */ public function __construct($loader = []) { /** @var mixed $loader explicit type check for mixed if user ignores parameter type */ @@ -40,7 +43,11 @@ public function __construct($loader = []) $this->useProcessEnv = \ZEND_THREAD_SAFE === false || \in_array(\PHP_SAPI, ['cli', 'cli-server', 'cgi-fcgi', 'fpm-fcgi'], true); } - /** @return mixed */ + /** + * @return mixed returns whatever the $next handler returns + * @throws \BadMethodCallException if used as a final request handler + * @throws \Throwable if $next handler throws unexpected exception + */ public function __invoke(ServerRequestInterface $request, ?callable $next = null) { if ($next === null) { @@ -60,6 +67,7 @@ public function __invoke(ServerRequestInterface $request, ?callable $next = null /** * @param class-string $class * @return callable(ServerRequestInterface,?callable=null) + * @throws void * @internal */ public function callable(string $class): callable @@ -99,7 +107,11 @@ public function callable(string $class): callable }; } - /** @internal */ + /** + * @throws \BadMethodCallException|\TypeError if container config or factory returns an unexpected type + * @throws \Throwable if container factory function throws unexpected exception + * @internal + */ public function getEnv(string $name): ?string { assert(\preg_match('/^[A-Z][A-Z0-9_]+$/', $name) === 1); @@ -119,7 +131,11 @@ public function getEnv(string $name): ?string return $value; } - /** @internal */ + /** + * @throws \BadMethodCallException|\TypeError if container config or factory returns an unexpected type + * @throws \Throwable if container factory function throws unexpected exception + * @internal + */ public function getAccessLogHandler(): AccessLogHandler { if ($this->container instanceof ContainerInterface) { @@ -133,7 +149,11 @@ public function getAccessLogHandler(): AccessLogHandler return $this->loadObject(AccessLogHandler::class); } - /** @internal */ + /** + * @throws \BadMethodCallException|\TypeError if container config or factory returns an unexpected type + * @throws \Throwable if container factory function throws unexpected exception + * @internal + */ public function getErrorHandler(): ErrorHandler { if ($this->container instanceof ContainerInterface) { @@ -152,6 +172,7 @@ public function getErrorHandler(): ErrorHandler * @param class-string $name * @return T * @throws \BadMethodCallException if object of type $name can not be loaded + * @throws \Throwable if container factory function throws unexpected exception */ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) */ { @@ -230,6 +251,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) /** * @return list * @throws \BadMethodCallException if either parameter can not be loaded + * @throws \Throwable if container factory function throws unexpected exception */ private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $depth, bool $allowVariables): array { @@ -244,6 +266,7 @@ private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $ /** * @return mixed * @throws \BadMethodCallException if $parameter can not be loaded + * @throws \Throwable if container factory function throws unexpected exception */ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool $allowVariables) /*: mixed (PHP 8.0+) */ { @@ -308,6 +331,7 @@ private function hasVariable(string $name): bool /** * @return object|string|int|float|bool|null * @throws \BadMethodCallException if $name is not a valid container variable + * @throws \Throwable if container factory function throws unexpected exception */ private function loadVariable(string $name, string $type, bool $nullable, int $depth) /*: object|string|int|float|bool|null (PHP 8.0+) */ { diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index 31aabb4..8f1f0d0 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -2080,6 +2080,45 @@ public function testGetEnvReturnsStringFromGlobalEnvBeforeProcessEnvIfPsrContain $this->assertEquals('foo', $ret); } + public function testGetEnvThrowsIfFactoryFunctionThrows(): void + { + $container = new Container([ + 'X_FOO' => function () { + throw new \RuntimeException('Demo'); + } + ]); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Demo'); + $container->getEnv('X_FOO'); + } + + public function testGetEnvThrowsIfFactoryFunctionReturnsInvalidResource(): void + { + $container = new Container([ + 'X_FOO' => function () { + return tmpfile(); + } + ]); + + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('Container variable $X_FOO expected type object|scalar|null from factory, but got resource'); + $container->getEnv('X_FOO'); + } + + public function testGetEnvThrowsIfFactoryFunctionReturnsInvalidInt(): void + { + $container = new Container([ + 'X_FOO' => function () { + return 0; + } + ]); + + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Environment variable $X_FOO expected type string|null, but got int'); + $container->getEnv('X_FOO'); + } + public function testGetEnvThrowsIfMapContainsInvalidType(): void { $container = new Container([ @@ -2189,6 +2228,32 @@ public function testGetAccessLogHandlerReturnsDefaultAccessLogHandlerInstanceIfP $this->assertInstanceOf(AccessLogHandler::class, $accessLogHandler); } + public function testGetAccessLogHandlerThrowsIfFactoryFunctionThrows(): void + { + $container = new Container([ + AccessLogHandler::class => function () { + throw new \RuntimeException('Demo'); + } + ]); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Demo'); + $container->getAccessLogHandler(); + } + + public function testGetAccessLogHandlerThrowsIfFactoryFunctionReturnsInvalidValue(): void + { + $container = new Container([ + AccessLogHandler::class => function () { + return null; + } + ]); + + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('Factory for FrameworkX\AccessLogHandler returned unexpected null'); + $container->getAccessLogHandler(); + } + public function testGetErrorHandlerReturnsDefaultErrorHandlerInstance(): void { $container = new Container([]); @@ -2241,6 +2306,32 @@ public function testGetErrorHandlerReturnsDefaultErrorHandlerInstanceIfPsrContai $this->assertInstanceOf(ErrorHandler::class, $errorHandler); } + public function testGetErrorHandlerThrowsIfFactoryFunctionThrows(): void + { + $container = new Container([ + ErrorHandler::class => function () { + throw new \RuntimeException('Demo'); + } + ]); + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('Demo'); + $container->getErrorHandler(); + } + + public function testGetErrorHandlerThrowsIfFactoryFunctionReturnsInvalidValue(): void + { + $container = new Container([ + ErrorHandler::class => function () { + return null; + } + ]); + + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('Factory for FrameworkX\ErrorHandler returned unexpected null'); + $container->getErrorHandler(); + } + public function testInvokeContainerAsMiddlewareReturnsFromNextRequestHandler(): void { $request = new ServerRequest('GET', 'http://example.com/'); From 76b37773d077f737650ff7e78879da81e203bd4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Thu, 19 Dec 2024 20:44:53 +0100 Subject: [PATCH 2/8] Throw `TypeError` more consistently for any type errors --- src/Container.php | 84 +++++++++++++++++++++++------------- tests/ContainerTest.php | 94 ++++++++++++++++++++++++----------------- 2 files changed, 109 insertions(+), 69 deletions(-) diff --git a/src/Container.php b/src/Container.php index 7b60f07..a62cc5b 100644 --- a/src/Container.php +++ b/src/Container.php @@ -17,27 +17,31 @@ class Container private $useProcessEnv; /** - * @param array|ContainerInterface $loader - * @throws \TypeError|\BadMethodCallException if given $loader is invalid + * @param array|ContainerInterface $config + * @throws \TypeError if given $config is invalid */ - public function __construct($loader = []) + public function __construct($config = []) { - /** @var mixed $loader explicit type check for mixed if user ignores parameter type */ - if (!\is_array($loader) && !$loader instanceof ContainerInterface) { + /** @var mixed $config explicit type check for mixed if user ignores parameter type */ + if (!\is_array($config) && !$config instanceof ContainerInterface) { throw new \TypeError( - 'Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, ' . $this->gettype($loader) . ' given' + 'Argument #1 ($config) must be of type array|Psr\Container\ContainerInterface, ' . $this->gettype($config) . ' given' ); } - foreach (($loader instanceof ContainerInterface ? [] : $loader) as $name => $value) { - if ( - (!\is_object($value) && !\is_scalar($value) && $value !== null) || - (!$value instanceof $name && !$value instanceof \Closure && !\is_string($value) && \strpos($name, '\\') !== false) - ) { - throw new \BadMethodCallException('Map for ' . $name . ' contains unexpected ' . $this->gettype($value)); + foreach (($config instanceof ContainerInterface ? [] : $config) as $name => $value) { + if (!$value instanceof $name && !$value instanceof \Closure && !\is_string($value) && \strpos($name, '\\') !== false) { + throw new \TypeError( + 'Argument #1 ($config) for key "' . $name . '" must be of type ' . $name . '|Closure|string, ' . $this->gettype($value) . ' given' + ); + } + if (!\is_object($value) && !\is_scalar($value) && $value !== null) { + throw new \TypeError( + 'Argument #1 ($config) for key "' . $name . '" must be of type object|string|int|float|bool|null|Closure, ' . $this->gettype($value) . ' given' + ); } } - $this->container = $loader; + $this->container = $config; // prefer reading environment from `$_ENV` and `$_SERVER`, only fall back to `getenv()` in thread-safe environments $this->useProcessEnv = \ZEND_THREAD_SAFE === false || \in_array(\PHP_SAPI, ['cli', 'cli-server', 'cgi-fcgi', 'fpm-fcgi'], true); @@ -108,7 +112,7 @@ public function callable(string $class): callable } /** - * @throws \BadMethodCallException|\TypeError if container config or factory returns an unexpected type + * @throws \TypeError if container config or factory returns an unexpected type * @throws \Throwable if container factory function throws unexpected exception * @internal */ @@ -125,14 +129,16 @@ public function getEnv(string $name): ?string } if (!\is_string($value) && $value !== null) { - throw new \TypeError('Environment variable $' . $name . ' expected type string|null, but got ' . $this->gettype($value)); + throw new \TypeError( + 'Return value of ' . __METHOD__ . '() for $' . $name . ' must be of type string|null, ' . $this->gettype($value) . ' returned' + ); } return $value; } /** - * @throws \BadMethodCallException|\TypeError if container config or factory returns an unexpected type + * @throws \TypeError if container config or factory returns an unexpected type * @throws \Throwable if container factory function throws unexpected exception * @internal */ @@ -150,7 +156,7 @@ public function getAccessLogHandler(): AccessLogHandler } /** - * @throws \BadMethodCallException|\TypeError if container config or factory returns an unexpected type + * @throws \TypeError if container config or factory returns an unexpected type * @throws \Throwable if container factory function throws unexpected exception * @internal */ @@ -171,6 +177,7 @@ public function getErrorHandler(): ErrorHandler * @template T of object * @param class-string $name * @return T + * @throws \TypeError if container config or factory returns an unexpected type * @throws \BadMethodCallException if object of type $name can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ @@ -181,13 +188,15 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) if (\array_key_exists($name, $this->container)) { if (\is_string($this->container[$name])) { if ($depth < 1) { - throw new \BadMethodCallException('Factory for ' . $name . ' is recursive'); + throw new \BadMethodCallException('Container config for ' . $name . ' is recursive'); } // @phpstan-ignore-next-line because type of container value is explicitly checked after getting here $value = $this->loadObject($this->container[$name], $depth - 1); if (!$value instanceof $name) { - throw new \BadMethodCallException('Factory for ' . $name . ' returned unexpected ' . $this->gettype($value)); + throw new \TypeError( + 'Return value of ' . __METHOD__ . '() for ' . $name . ' must be of type ' . $name . ', ' . $this->gettype($value) . ' returned' + ); } $this->container[$name] = $value; @@ -201,19 +210,23 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) if (\is_string($value)) { if ($depth < 1) { - throw new \BadMethodCallException('Factory for ' . $name . ' is recursive'); + throw new \BadMethodCallException('Container config for ' . $name . ' is recursive'); } // @phpstan-ignore-next-line because type of container value is explicitly checked after getting here $value = $this->loadObject($value, $depth - 1); } if (!$value instanceof $name) { - throw new \BadMethodCallException('Factory for ' . $name . ' returned unexpected ' . $this->gettype($value)); + throw new \TypeError( + 'Return value of ' . self::functionName($closure) . ' for ' . $name . ' must be of type ' . $name . ', ' . $this->gettype($value) . ' returned' + ); } $this->container[$name] = $value; } elseif (!$this->container[$name] instanceof $name) { - throw new \BadMethodCallException('Map for ' . $name . ' contains unexpected ' . $this->gettype($this->container[$name])); + throw new \TypeError( + 'Return value of ' . __METHOD__ . '() for ' . $name . ' must be of type ' . $name . ', ' . $this->gettype($this->container[$name]) . ' returned' + ); } assert($this->container[$name] instanceof $name); @@ -250,6 +263,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) /** * @return list + * @throws \TypeError if container config or factory returns an unexpected type * @throws \BadMethodCallException if either parameter can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ @@ -265,6 +279,7 @@ private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $ /** * @return mixed + * @throws \TypeError if container config or factory returns an unexpected type * @throws \BadMethodCallException if $parameter can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ @@ -330,7 +345,8 @@ private function hasVariable(string $name): bool /** * @return object|string|int|float|bool|null - * @throws \BadMethodCallException if $name is not a valid container variable + * @throws \TypeError if container factory returns an unexpected type + * @throws \BadMethodCallException if $name can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ private function loadVariable(string $name, string $type, bool $nullable, int $depth) /*: object|string|int|float|bool|null (PHP 8.0+) */ @@ -340,7 +356,7 @@ private function loadVariable(string $name, string $type, bool $nullable, int $d if (\is_array($this->container) && ($this->container[$name] ?? null) instanceof \Closure) { if ($depth < 1) { - throw new \BadMethodCallException('Container variable $' . $name . ' is recursive'); + throw new \BadMethodCallException('Container config for $' . $name . ' is recursive'); } // build list of factory parameters based on parameter types @@ -353,7 +369,9 @@ private function loadVariable(string $name, string $type, bool $nullable, int $d $value = $params === [] ? $factory() : $factory(...$params); if (!\is_object($value) && !\is_scalar($value) && $value !== null) { - throw new \BadMethodCallException('Container variable $' . $name . ' expected type object|scalar|null from factory, but got ' . $this->gettype($value)); + 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' + ); } $this->container[$name] = $value; @@ -387,25 +405,31 @@ private function loadVariable(string $name, string $type, bool $nullable, int $d (!\is_object($value) && !\in_array($type, ['string', 'int', 'float', 'bool'])) || ($type === 'string' && !\is_string($value)) || ($type === 'int' && !\is_int($value)) || ($type === 'float' && !\is_float($value)) || ($type === 'bool' && !\is_bool($value)) ) { - throw new \BadMethodCallException('Container variable $' . $name . ' expected type ' . $type . ', but got ' . $this->gettype($value)); + throw new \TypeError( + 'Return value of ' . __METHOD__ . '() for $' . $name . ' must be of type ' . $type . ', ' . $this->gettype($value) . ' returned' + ); } return $value; } /** @throws void */ - private static function parameterError(\ReflectionParameter $parameter): string + private static function functionName(\ReflectionFunctionAbstract $function): string { - $function = $parameter->getDeclaringFunction(); $name = $function->getShortName(); if ($name[0] === '{') { // $function->isAnonymous() (PHP 8.2+) // use PHP 8.4+ format including closure file and line on all PHP versions: https://3v4l.org/tAs7s $name = '{closure:' . $function->getFileName() . ':' . $function->getStartLine() . '}'; - } elseif (($class = $parameter->getDeclaringClass()) !== null) { + } elseif ($function instanceof \ReflectionMethod && ($class = $function->getDeclaringClass()) !== null) { $name = explode("\0", $class->getName())[0] . '::' . $name; } + return $name . '()'; + } - return 'Argument #' . ($parameter->getPosition() + 1) . ' ($' . $parameter->getName() . ') of ' . $name . '()'; + /** @throws void */ + private static function parameterError(\ReflectionParameter $parameter): string + { + return 'Argument #' . ($parameter->getPosition() + 1) . ' ($' . $parameter->getName() . ') of ' . self::functionName($parameter->getDeclaringFunction()); } /** diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index 8f1f0d0..91e38d6 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -1309,7 +1309,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Container variable $stdClass is recursive'); + $this->expectExceptionMessage('Container config for $stdClass is recursive'); $callable($request); } @@ -1335,7 +1335,7 @@ public function __construct(string $stdClass) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Container variable $stdClass expected type string, but got stdClass'); + $this->expectExceptionMessage('Return value of ' . Container::class . '::loadVariable() for $stdClass must be of type string, stdClass returned'); $callable($request); } @@ -1350,6 +1350,7 @@ public function __construct(\stdClass $data) } }; + $line = __LINE__ + 5; $container = new Container([ \stdClass::class => function (string $http) { return (object) ['name' => $http]; @@ -1362,7 +1363,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Container variable $http expected type object|scalar|null from factory, but got resource'); + $this->expectExceptionMessage('Return value of {closure:' . __FILE__ . ':' . $line . '}() for $http must be of type object|string|int|float|bool|null, resource returned'); $callable($request); } @@ -1387,7 +1388,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Container variable $http expected type stdClass, but got int'); + $this->expectExceptionMessage('Return value of ' . Container::class . '::loadVariable() for $http must be of type stdClass, int returned'); $callable($request); } @@ -1412,7 +1413,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Container variable $http expected type string, but got int'); + $this->expectExceptionMessage('Return value of ' . Container::class . '::loadVariable() for $http must be of type string, int returned'); $callable($request); } @@ -1437,7 +1438,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Container variable $http expected type int, but got string'); + $this->expectExceptionMessage('Return value of ' . Container::class . '::loadVariable() for $http must be of type int, string returned'); $callable($request); } @@ -1462,7 +1463,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Container variable $percent expected type float, but got string'); + $this->expectExceptionMessage('Return value of ' . Container::class . '::loadVariable() for $percent must be of type float, string returned'); $callable($request); } @@ -1487,7 +1488,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Container variable $admin expected type bool, but got string'); + $this->expectExceptionMessage('Return value of ' . Container::class . '::loadVariable() for $admin must be of type bool, string returned'); $callable($request); } @@ -1553,7 +1554,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Map for stdClass contains unexpected int'); + $this->expectExceptionMessage('Return value of ' . Container::class . '::loadObject() for stdClass must be of type stdClass, int returned'); $callable($request); } @@ -1575,7 +1576,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Map for stdClass contains unexpected null'); + $this->expectExceptionMessage('Return value of ' . Container::class . '::loadObject() for stdClass must be of type stdClass, null returned'); $callable($request); } @@ -1597,7 +1598,7 @@ public function __construct(?\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Map for stdClass contains unexpected null'); + $this->expectExceptionMessage('Return value of ' . Container::class . '::loadObject() for stdClass must be of type stdClass, null returned'); $callable($request); } @@ -1619,7 +1620,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Map for stdClass contains unexpected React\Http\Message\Response'); + $this->expectExceptionMessage('Return value of ' . Container::class . '::loadObject() for stdClass must be of type stdClass, React\Http\Message\Response returned'); $callable($request); } @@ -1645,46 +1646,56 @@ public function __construct(string $name) $callable($request); } - public function testCtorThrowsWhenMapContainsInvalidArray(): void + public function testCtorThrowsWhenConfigContainsInvalidArray(): void { - $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Map for all contains unexpected array'); + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Argument #1 ($config) for key "all" must be of type object|string|int|float|bool|null|Closure, array given'); new Container([ // @phpstan-ignore-line 'all' => [] ]); } - public function testCtorThrowsWhenMapContainsInvalidResource(): void + public function testCtorThrowsWhenConfigContainsInvalidResource(): void { - $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Map for file contains unexpected resource'); + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Argument #1 ($config) for key "file" must be of type object|string|int|float|bool|null|Closure, resource given'); new Container([ // @phpstan-ignore-line 'file' => tmpfile() ]); } - public function testCtorThrowsWhenMapForClassContainsInvalidObject(): void + public function testCtorThrowsWhenConfigForClassContainsInvalidObject(): void { - $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Map for Psr\Http\Message\ResponseInterface contains unexpected stdClass'); + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Argument #1 ($config) for key "Psr\Http\Message\ResponseInterface" must be of type Psr\Http\Message\ResponseInterface|Closure|string, stdClass given'); new Container([ ResponseInterface::class => new \stdClass() ]); } - public function testCtorThrowsWhenMapForClassContainsInvalidNull(): void + public function testCtorThrowsWhenConfigForClassContainsInvalidNull(): void { - $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Map for Psr\Http\Message\ResponseInterface contains unexpected null'); + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Argument #1 ($config) for key "Psr\Http\Message\ResponseInterface" must be of type Psr\Http\Message\ResponseInterface|Closure|string, null given'); new Container([ ResponseInterface::class => null ]); } + public function testCtorThrowsWhenConfigForClassContainsInvalidResource(): void + { + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Argument #1 ($config) for key "Psr\Http\Message\ResponseInterface" must be of type Psr\Http\Message\ResponseInterface|Closure|string, resource given'); + + new Container([ // @phpstan-ignore-line + ResponseInterface::class => tmpfile() + ]); + } + public function testCallableReturnsCallableThatThrowsWhenFactoryReturnsInvalidClassName(): void { $request = new ServerRequest('GET', 'http://example.com/'); @@ -1704,6 +1715,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryReturnsInvalidIn { $request = new ServerRequest('GET', 'http://example.com/'); + $line = __LINE__ + 2; $container = new Container([ \stdClass::class => function () { return 42; } ]); @@ -1711,7 +1723,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryReturnsInvalidIn $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Factory for stdClass returned unexpected int'); + $this->expectExceptionMessage('Return value of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type stdClass, int returned'); $callable($request); } @@ -1726,7 +1738,7 @@ public function testCallableReturnsCallableThatThrowsWhenMapReferencesClassNameT $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Factory for stdClass returned unexpected React\Http\Message\Response'); + $this->expectExceptionMessage('Return value of ' . Container::class . '::loadObject() for stdClass must be of type stdClass, React\Http\Message\Response returned'); $callable($request); } @@ -1734,6 +1746,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryReturnsClassName { $request = new ServerRequest('GET', 'http://example.com/'); + $line = __LINE__ + 2; $container = new Container([ \stdClass::class => function () { return Response::class; } ]); @@ -1741,7 +1754,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryReturnsClassName $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Factory for stdClass returned unexpected React\Http\Message\Response'); + $this->expectExceptionMessage('Return value of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type stdClass, React\Http\Message\Response returned'); $callable($request); } @@ -1822,7 +1835,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryIsRecursive(): v $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Factory for stdClass is recursive'); + $this->expectExceptionMessage('Container config for stdClass is recursive'); $callable($request); } @@ -1839,7 +1852,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryIsRecursiveClass $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Factory for stdClass is recursive'); + $this->expectExceptionMessage('Container config for stdClass is recursive'); $callable($request); } @@ -2095,14 +2108,15 @@ public function testGetEnvThrowsIfFactoryFunctionThrows(): void public function testGetEnvThrowsIfFactoryFunctionReturnsInvalidResource(): void { + $line = __LINE__ + 2; $container = new Container([ 'X_FOO' => function () { return tmpfile(); } ]); - $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Container variable $X_FOO expected type object|scalar|null from factory, but got resource'); + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Return value of {closure:' . __FILE__ . ':' . $line . '}() for $X_FOO must be of type object|string|int|float|bool|null, resource returned'); $container->getEnv('X_FOO'); } @@ -2115,7 +2129,7 @@ public function testGetEnvThrowsIfFactoryFunctionReturnsInvalidInt(): void ]); $this->expectException(\TypeError::class); - $this->expectExceptionMessage('Environment variable $X_FOO expected type string|null, but got int'); + $this->expectExceptionMessage('Return value of FrameworkX\Container::getEnv() for $X_FOO must be of type string|null, int returned'); $container->getEnv('X_FOO'); } @@ -2126,7 +2140,7 @@ public function testGetEnvThrowsIfMapContainsInvalidType(): void ]); $this->expectException(\TypeError::class); - $this->expectExceptionMessage('Environment variable $X_FOO expected type string|null, but got false'); + $this->expectExceptionMessage('Return value of ' . Container::class . '::getEnv() for $X_FOO must be of type string|null, false returned'); $container->getEnv('X_FOO'); } @@ -2172,7 +2186,7 @@ public function testGetEnvThrowsIfMapPsrContainerReturnsInvalidType(): void $container = new Container($psr); $this->expectException(\TypeError::class); - $this->expectExceptionMessage('Environment variable $X_FOO expected type string|null, but got int'); + $this->expectExceptionMessage('Return value of ' . Container::class .'::getEnv() for $X_FOO must be of type string|null, int returned'); $container->getEnv('X_FOO'); } @@ -2243,14 +2257,15 @@ public function testGetAccessLogHandlerThrowsIfFactoryFunctionThrows(): void public function testGetAccessLogHandlerThrowsIfFactoryFunctionReturnsInvalidValue(): void { + $line = __LINE__ + 2; $container = new Container([ AccessLogHandler::class => function () { return null; } ]); - $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Factory for FrameworkX\AccessLogHandler returned unexpected null'); + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Return value of {closure:' . __FILE__ . ':' . $line . '}() for FrameworkX\AccessLogHandler must be of type FrameworkX\AccessLogHandler, null returned'); $container->getAccessLogHandler(); } @@ -2321,14 +2336,15 @@ public function testGetErrorHandlerThrowsIfFactoryFunctionThrows(): void public function testGetErrorHandlerThrowsIfFactoryFunctionReturnsInvalidValue(): void { + $line = __LINE__ + 2; $container = new Container([ ErrorHandler::class => function () { return null; } ]); - $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Factory for FrameworkX\ErrorHandler returned unexpected null'); + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Return value of {closure:' . __FILE__ . ':' . $line . '}() for FrameworkX\ErrorHandler must be of type FrameworkX\ErrorHandler, null returned'); $container->getErrorHandler(); } @@ -2386,7 +2402,7 @@ public static function provideInvalidContainerConfigValues(): \Generator public function testCtorWithInvalidValueThrows($value, string $type): void { $this->expectException(\TypeError::class); - $this->expectExceptionMessage('Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, ' . $type . ' given'); + $this->expectExceptionMessage('Argument #1 ($config) must be of type array|Psr\Container\ContainerInterface, ' . $type . ' given'); new Container($value); // @phpstan-ignore-line } } From d5e09a2301ae702d6bd8bea5bd7a4ccca6e2f0cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Thu, 21 Sep 2023 18:57:42 +0200 Subject: [PATCH 3/8] Improve exception messages for invalid arguments to factory functions --- src/Container.php | 54 +++++++++++++++++++++++------------------ tests/ContainerTest.php | 18 +++++++++----- 2 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/Container.php b/src/Container.php index a62cc5b..697d87d 100644 --- a/src/Container.php +++ b/src/Container.php @@ -123,7 +123,7 @@ public function getEnv(string $name): ?string if ($this->container instanceof ContainerInterface && $this->container->has($name)) { $value = $this->container->get($name); } elseif ($this->hasVariable($name)) { - $value = $this->loadVariable($name, 'mixed', true, 64); + $value = $this->loadVariable($name); } else { return null; } @@ -302,7 +302,17 @@ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool // load container variables if parameter name is known assert($type === null || $type instanceof \ReflectionNamedType); if ($allowVariables && $this->hasVariable($parameter->getName())) { - return $this->loadVariable($parameter->getName(), $type === null ? 'mixed' : $type->getName(), $parameter->allowsNull(), $depth); + $value = $this->loadVariable($parameter->getName(), $depth); + + // skip type checks and allow all values if expected type is undefined or mixed (PHP 8+) + // allow null values if parameter is marked nullable or untyped or mixed + if ($type === null || ($value === null && $parameter->allowsNull()) || $type->getName() === 'mixed' || $this->validateType($value, $type)) { + return $value; + } + + throw new \TypeError( + self::parameterError($parameter) . ' must be of type ' . ($type->allowsNull() ? '?' : '') . $type->getName() . ', ' . $this->gettype($value) . ' given' + ); } // abort if parameter is untyped and not explicitly defined by container variable @@ -349,7 +359,7 @@ private function hasVariable(string $name): bool * @throws \BadMethodCallException if $name can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ - private function loadVariable(string $name, string $type, bool $nullable, int $depth) /*: object|string|int|float|bool|null (PHP 8.0+) */ + private function loadVariable(string $name, int $depth = 64) /*: object|string|int|float|bool|null (PHP 8.0+) */ { assert($this->hasVariable($name)); assert(\is_array($this->container) || !$this->container->has($name)); @@ -389,30 +399,26 @@ private function loadVariable(string $name, string $type, bool $nullable, int $d } assert(\is_object($value) || \is_scalar($value) || $value === null); - - // allow null values if parameter is marked nullable or untyped or mixed - if ($nullable && $value === null) { - return null; - } - - // skip type checks and allow all values if expected type is undefined or mixed (PHP 8+) - if ($type === 'mixed') { - return $value; - } - - if ( - (\is_object($value) && !$value instanceof $type) || - (!\is_object($value) && !\in_array($type, ['string', 'int', 'float', 'bool'])) || - ($type === 'string' && !\is_string($value)) || ($type === 'int' && !\is_int($value)) || ($type === 'float' && !\is_float($value)) || ($type === 'bool' && !\is_bool($value)) - ) { - throw new \TypeError( - 'Return value of ' . __METHOD__ . '() for $' . $name . ' must be of type ' . $type . ', ' . $this->gettype($value) . ' returned' - ); - } - return $value; } + /** + * @param object|string|int|float|bool|null $value + * @param \ReflectionNamedType $type + * @throws void + */ + private function validateType($value, \ReflectionNamedType $type): bool + { + $type = $type->getName(); + return ( + (\is_object($value) && $value instanceof $type) || + (\is_string($value) && $type === 'string') || + (\is_int($value) && $type === 'int') || + (\is_float($value) && $type === 'float') || + (\is_bool($value) && $type === 'bool') + ); + } + /** @throws void */ private static function functionName(\ReflectionFunctionAbstract $function): string { diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index 91e38d6..4c9af12 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -1324,6 +1324,7 @@ public function __construct(string $stdClass) } }; + $line = __LINE__ + 2; $container = new Container([ get_class($controller) => function (string $stdClass) use ($controller) { $class = get_class($controller); @@ -1335,7 +1336,7 @@ public function __construct(string $stdClass) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Return value of ' . Container::class . '::loadVariable() for $stdClass must be of type string, stdClass returned'); + $this->expectExceptionMessage('Argument #1 ($stdClass) of {closure:' . __FILE__ . ':' . $line . '}() must be of type string, stdClass given'); $callable($request); } @@ -1378,6 +1379,7 @@ public function __construct(\stdClass $data) } }; + $line = __LINE__ + 2; $container = new Container([ \stdClass::class => function (\stdClass $http) { return (object) ['name' => $http]; @@ -1388,7 +1390,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Return value of ' . Container::class . '::loadVariable() for $http must be of type stdClass, int returned'); + $this->expectExceptionMessage('Argument #1 ($http) of {closure:' . __FILE__ . ':' . $line . '}() must be of type stdClass, int given'); $callable($request); } @@ -1403,6 +1405,7 @@ public function __construct(\stdClass $data) } }; + $line = __LINE__ + 2; $container = new Container([ \stdClass::class => function (string $http) { return (object) ['name' => $http]; @@ -1413,7 +1416,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Return value of ' . Container::class . '::loadVariable() for $http must be of type string, int returned'); + $this->expectExceptionMessage('Argument #1 ($http) of {closure:' . __FILE__ . ':' . $line . '}() must be of type string, int given'); $callable($request); } @@ -1428,6 +1431,7 @@ public function __construct(\stdClass $data) } }; + $line = __LINE__ + 2; $container = new Container([ \stdClass::class => function (int $http) { return (object) ['name' => $http]; @@ -1438,7 +1442,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Return value of ' . Container::class . '::loadVariable() for $http must be of type int, string returned'); + $this->expectExceptionMessage('Argument #1 ($http) of {closure:' . __FILE__ . ':' . $line . '}() must be of type int, string given'); $callable($request); } @@ -1453,6 +1457,7 @@ public function __construct(\stdClass $data) } }; + $line = __LINE__ + 2; $container = new Container([ \stdClass::class => function (float $percent) { return (object) ['percent' => $percent]; @@ -1463,7 +1468,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Return value of ' . Container::class . '::loadVariable() for $percent must be of type float, string returned'); + $this->expectExceptionMessage('Argument #1 ($percent) of {closure:' . __FILE__ . ':' . $line . '}() must be of type float, string given'); $callable($request); } @@ -1478,6 +1483,7 @@ public function __construct(\stdClass $data) } }; + $line = __LINE__ + 2; $container = new Container([ \stdClass::class => function (bool $admin) { return (object) ['admin' => $admin]; @@ -1488,7 +1494,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Return value of ' . Container::class . '::loadVariable() for $admin must be of type bool, string returned'); + $this->expectExceptionMessage('Argument #1 ($admin) of {closure:' . __FILE__ . ':' . $line . '}() must be of type bool, string given'); $callable($request); } From d940bc83e1a52f7819380b88c71dd5b271642eb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sat, 21 Dec 2024 19:49:02 +0100 Subject: [PATCH 4/8] Improve exception messages to include reference to failing entry --- src/Container.php | 28 ++++++++++++++-------------- tests/ContainerTest.php | 24 ++++++++++++------------ 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/Container.php b/src/Container.php index 697d87d..9144154 100644 --- a/src/Container.php +++ b/src/Container.php @@ -203,7 +203,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) } 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); + $params = $this->loadFunctionParams($closure, $depth, true, $name); // invoke factory with list of parameters $value = $params === [] ? ($this->container[$name])() : ($this->container[$name])(...$params); @@ -254,7 +254,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) // build list of constructor parameters based on parameter types $ctor = $class->getConstructor(); - $params = $ctor === null ? [] : $this->loadFunctionParams($ctor, $depth, false); + $params = $ctor === null ? [] : $this->loadFunctionParams($ctor, $depth, false, ''); // instantiate with list of parameters // @phpstan-ignore-next-line because `$class->newInstance()` is known to return `T` @@ -267,11 +267,11 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) * @throws \BadMethodCallException if either parameter can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ - private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $depth, bool $allowVariables): array + private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $depth, bool $allowVariables, string $for): array { $params = []; foreach ($function->getParameters() as $parameter) { - $params[] = $this->loadParameter($parameter, $depth, $allowVariables); + $params[] = $this->loadParameter($parameter, $depth, $allowVariables, $for); } return $params; @@ -283,7 +283,7 @@ private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $ * @throws \BadMethodCallException if $parameter can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ - private function loadParameter(\ReflectionParameter $parameter, int $depth, bool $allowVariables) /*: mixed (PHP 8.0+) */ + private function loadParameter(\ReflectionParameter $parameter, int $depth, bool $allowVariables, string $for) /*: mixed (PHP 8.0+) */ { assert(\is_array($this->container)); @@ -296,7 +296,7 @@ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool if ($hasDefault) { return $parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null; } - throw new \BadMethodCallException(self::parameterError($parameter) . ' expects unsupported type ' . $type); + throw new \BadMethodCallException(self::parameterError($parameter, $for) . ' expects unsupported type ' . $type); } // @codeCoverageIgnoreEnd // load container variables if parameter name is known @@ -311,7 +311,7 @@ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool } throw new \TypeError( - self::parameterError($parameter) . ' must be of type ' . ($type->allowsNull() ? '?' : '') . $type->getName() . ', ' . $this->gettype($value) . ' given' + self::parameterError($parameter, $for) . ' must be of type ' . ($type->allowsNull() ? '?' : '') . $type->getName() . ', ' . $this->gettype($value) . ' given' ); } @@ -321,7 +321,7 @@ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool if ($parameter->isDefaultValueAvailable()) { return $parameter->getDefaultValue(); } - throw new \BadMethodCallException(self::parameterError($parameter) . ' has no type'); + throw new \BadMethodCallException(self::parameterError($parameter, $for) . ' has no type'); } // use default/nullable argument if not loadable as container variable or by type @@ -333,15 +333,15 @@ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool // abort if required container variable is not defined or for any other primitive types (array etc.) if ($type->isBuiltin()) { if ($allowVariables) { - throw new \BadMethodCallException(self::parameterError($parameter) . ' is not defined'); + throw new \BadMethodCallException(self::parameterError($parameter, $for) . ' is not defined'); } else { - throw new \BadMethodCallException(self::parameterError($parameter) . ' expects unsupported type ' . $type->getName()); + throw new \BadMethodCallException(self::parameterError($parameter, $for) . ' expects unsupported type ' . $type->getName()); } } // abort for unreasonably deep nesting or recursive types if ($depth < 1) { - throw new \BadMethodCallException(self::parameterError($parameter) . ' is recursive'); + throw new \BadMethodCallException(self::parameterError($parameter, $for) . ' is recursive'); } // @phpstan-ignore-next-line because `$type->getName()` is a `class-string` by definition @@ -373,7 +373,7 @@ private function loadVariable(string $name, int $depth = 64) /*: object|string|i $factory = $this->container[$name]; assert($factory instanceof \Closure); $closure = new \ReflectionFunction($factory); - $params = $this->loadFunctionParams($closure, $depth - 1, true); + $params = $this->loadFunctionParams($closure, $depth - 1, true, '$' . $name); // invoke factory with list of parameters $value = $params === [] ? $factory() : $factory(...$params); @@ -433,9 +433,9 @@ private static function functionName(\ReflectionFunctionAbstract $function): str } /** @throws void */ - private static function parameterError(\ReflectionParameter $parameter): string + private static function parameterError(\ReflectionParameter $parameter, string $for): string { - return 'Argument #' . ($parameter->getPosition() + 1) . ' ($' . $parameter->getName() . ') of ' . self::functionName($parameter->getDeclaringFunction()); + return 'Argument #' . ($parameter->getPosition() + 1) . ' ($' . $parameter->getName() . ') of ' . self::functionName($parameter->getDeclaringFunction()) . ($for !== '' ? ' for ' . $for : ''); } /** diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index 4c9af12..90e6818 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -1285,7 +1285,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($username) of {closure:' . __FILE__ . ':' . $line .'}() is not defined'); + $this->expectExceptionMessage('Argument #1 ($username) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass is not defined'); $callable($request); } @@ -1336,7 +1336,7 @@ public function __construct(string $stdClass) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($stdClass) of {closure:' . __FILE__ . ':' . $line . '}() must be of type string, stdClass given'); + $this->expectExceptionMessage('Argument #1 ($stdClass) of {closure:' . __FILE__ . ':' . $line . '}() for ' . get_class($controller) . ' must be of type string, stdClass given'); $callable($request); } @@ -1390,7 +1390,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($http) of {closure:' . __FILE__ . ':' . $line . '}() must be of type stdClass, int given'); + $this->expectExceptionMessage('Argument #1 ($http) of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type stdClass, int given'); $callable($request); } @@ -1416,7 +1416,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($http) of {closure:' . __FILE__ . ':' . $line . '}() must be of type string, int given'); + $this->expectExceptionMessage('Argument #1 ($http) of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type string, int given'); $callable($request); } @@ -1442,7 +1442,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($http) of {closure:' . __FILE__ . ':' . $line . '}() must be of type int, string given'); + $this->expectExceptionMessage('Argument #1 ($http) of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type int, string given'); $callable($request); } @@ -1468,7 +1468,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($percent) of {closure:' . __FILE__ . ':' . $line . '}() must be of type float, string given'); + $this->expectExceptionMessage('Argument #1 ($percent) of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type float, string given'); $callable($request); } @@ -1494,7 +1494,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($admin) of {closure:' . __FILE__ . ':' . $line . '}() must be of type bool, string given'); + $this->expectExceptionMessage('Argument #1 ($admin) of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type bool, string given'); $callable($request); } @@ -1791,7 +1791,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUntypedA $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() has no type'); + $this->expectExceptionMessage('Argument #1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass has no type'); $callable($request); } @@ -1810,7 +1810,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUndefine $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() is not defined'); + $this->expectExceptionMessage('Argument #1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass is not defined'); $callable($request); } @@ -1826,7 +1826,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresRecursiv $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($data) of {closure:' . __FILE__ . ':' . $line .'}() is recursive'); + $this->expectExceptionMessage('Argument #1 ($data) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass is recursive'); $callable($request); } @@ -2160,7 +2160,7 @@ public function testGetEnvThrowsWhenFactoryUsesBuiltInFunctionThatReferencesUnkn ]); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($extension) of extension_loaded() is not defined'); + $this->expectExceptionMessage('Argument #1 ($extension) of extension_loaded() for $X_FOO is not defined'); $container->getEnv('X_FOO'); } @@ -2178,7 +2178,7 @@ public function foo(string $bar): string ]); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($bar) of class@anonymous::foo() is not defined'); + $this->expectExceptionMessage('Argument #1 ($bar) of class@anonymous::foo() for $X_FOO is not defined'); $container->getEnv('X_FOO'); } From fc007e050af352b17f53a62ff077701830606849 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sun, 16 Mar 2025 11:46:25 +0100 Subject: [PATCH 5/8] Improve exception messages to report anonymous classes consistently --- src/Container.php | 10 ++++++---- tests/AppTest.php | 2 +- tests/ContainerTest.php | 41 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/Container.php b/src/Container.php index 9144154..0d25306 100644 --- a/src/Container.php +++ b/src/Container.php @@ -100,7 +100,9 @@ public function callable(string $class): callable // This initial version is intentionally limited to checking the method name only. // A follow-up version will likely use reflection to check request handler argument types. if (!is_callable($handler)) { - throw new \BadMethodCallException('Request handler class "' . $class . '" has no public __invoke() method'); + throw new \BadMethodCallException( + 'Request handler class ' . \explode("\0", $class)[0] . ' has no public __invoke() method' + ); } // invoke request handler as middleware handler or final controller @@ -203,7 +205,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) } 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, $name); + $params = $this->loadFunctionParams($closure, $depth, true, \explode("\0", $name)[0]); // invoke factory with list of parameters $value = $params === [] ? ($this->container[$name])() : ($this->container[$name])(...$params); @@ -427,7 +429,7 @@ private static function functionName(\ReflectionFunctionAbstract $function): str // use PHP 8.4+ format including closure file and line on all PHP versions: https://3v4l.org/tAs7s $name = '{closure:' . $function->getFileName() . ':' . $function->getStartLine() . '}'; } elseif ($function instanceof \ReflectionMethod && ($class = $function->getDeclaringClass()) !== null) { - $name = explode("\0", $class->getName())[0] . '::' . $name; + $name = \explode("\0", $class->getName())[0] . '::' . $name; } return $name . '()'; } @@ -455,6 +457,6 @@ private function gettype($value): string } elseif ($value === null) { return 'null'; } - return \is_object($value) ? \get_class($value) : \gettype($value); + return \is_object($value) ? \explode("\0", \get_class($value))[0] : \gettype($value); } } diff --git a/tests/AppTest.php b/tests/AppTest.php index bb421e6..d9f4c85 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -1739,7 +1739,7 @@ public function testInvokeWithMatchingRouteReturnsInternalServerErrorResponseWhe $this->assertStringContainsString("Error 500: Internal Server Error\n", (string) $response->getBody()); $this->assertStringContainsString("

The requested page failed to load, please try again later.

\n", (string) $response->getBody()); - $this->assertStringMatchesFormat("%a

Expected request handler to return Psr\Http\Message\ResponseInterface but got uncaught BadMethodCallException with message Request handler class %s has no public __invoke() method in Container.php:%d.

\n%a", (string) $response->getBody()); + $this->assertStringMatchesFormat("%a

Expected request handler to return Psr\Http\Message\ResponseInterface but got uncaught BadMethodCallException with message Request handler class class@anonymous has no public __invoke() method in Container.php:%d.

\n%a", (string) $response->getBody()); } public function testInvokeWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerReturnsPromiseWhichFulfillsWithWrongValue(): void diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index 90e6818..5aaf1c3 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -1264,6 +1264,21 @@ public function __invoke(): ResponseInterface $this->assertEquals('"bar"', (string) $response->getBody()); } + public function testCallableReturnsCallableThatThrowsForNonCallableClass(): void + { + $request = new ServerRequest('GET', 'http://example.com/'); + + $controller = new class() { }; + + $container = new Container([]); + + $callable = $container->callable(get_class($controller)); + + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('Request handler class class@anonymous has no public __invoke() method'); + $callable($request); + } + public function testCallableReturnsCallableThatThrowsWhenFactoryReferencesUnknownVariable(): void { $request = new ServerRequest('GET', 'http://example.com/'); @@ -1336,7 +1351,7 @@ public function __construct(string $stdClass) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($stdClass) of {closure:' . __FILE__ . ':' . $line . '}() for ' . get_class($controller) . ' must be of type string, stdClass given'); + $this->expectExceptionMessage('Argument #1 ($stdClass) of {closure:' . __FILE__ . ':' . $line . '}() for class@anonymous must be of type string, stdClass given'); $callable($request); } @@ -1682,6 +1697,16 @@ public function testCtorThrowsWhenConfigForClassContainsInvalidObject(): void ]); } + public function testCtorThrowsWhenConfigForClassContainsInvalidAnonymousClass(): void + { + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Argument #1 ($config) for key "Psr\Http\Message\ResponseInterface" must be of type Psr\Http\Message\ResponseInterface|Closure|string, class@anonymous given'); + + new Container([ + ResponseInterface::class => new class() { } + ]); + } + public function testCtorThrowsWhenConfigForClassContainsInvalidNull(): void { $this->expectException(\TypeError::class); @@ -2182,6 +2207,20 @@ public function foo(string $bar): string $container->getEnv('X_FOO'); } + /** @link https://3v4l.org/VaFMd */ + public function testGetEnvThrowsWhenFactoryFunctionExpectsIntArgumentButGivenAnonymousClass(): void + { + $line = __LINE__ + 2; + $container = new Container([ + 'X_FOO' => function (int $bar) { return (string) $bar; }, + 'bar' => new class { } + ]); + + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Argument #1 ($bar) of {closure:' . __FILE__ . ':' . $line . '}() for $X_FOO must be of type int, class@anonymous given'); + $container->getEnv('X_FOO'); + } + public function testGetEnvThrowsIfMapPsrContainerReturnsInvalidType(): void { $psr = $this->createMock(ContainerInterface::class); From 0fbddea49f60fc1722db75626a4160989172eebf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sun, 16 Mar 2025 23:24:20 +0100 Subject: [PATCH 6/8] Improve exception messages for undefined variables and unknown types --- src/Container.php | 60 +++++++++++++++++++++++------------------ tests/AppTest.php | 4 +-- tests/ContainerTest.php | 59 ++++++++++++++++++++++++++++++++++------ 3 files changed, 87 insertions(+), 36 deletions(-) diff --git a/src/Container.php b/src/Container.php index 0d25306..93f970a 100644 --- a/src/Container.php +++ b/src/Container.php @@ -180,7 +180,7 @@ public function getErrorHandler(): ErrorHandler * @param class-string $name * @return T * @throws \TypeError if container config or factory returns an unexpected type - * @throws \BadMethodCallException if object of type $name can not be loaded + * @throws \Error|\BadMethodCallException if object of type $name can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) */ @@ -266,7 +266,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) /** * @return list * @throws \TypeError if container config or factory returns an unexpected type - * @throws \BadMethodCallException if either parameter can not be loaded + * @throws \Error|\BadMethodCallException if either parameter can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $depth, bool $allowVariables, string $for): array @@ -282,23 +282,27 @@ private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $ /** * @return mixed * @throws \TypeError if container config or factory returns an unexpected type - * @throws \BadMethodCallException if $parameter can not be loaded + * @throws \Error|\BadMethodCallException if $parameter can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool $allowVariables, string $for) /*: mixed (PHP 8.0+) */ { assert(\is_array($this->container)); - $type = $parameter->getType(); - $hasDefault = $parameter->isDefaultValueAvailable() || ((!$type instanceof \ReflectionNamedType || $type->getName() !== 'mixed') && $parameter->allowsNull()); // abort for union types (PHP 8.0+) and intersection types (PHP 8.1+) // @phpstan-ignore-next-line for PHP < 8 if ($type instanceof \ReflectionUnionType || $type instanceof \ReflectionIntersectionType) { // @codeCoverageIgnoreStart - if ($hasDefault) { - return $parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null; + if ($parameter->isDefaultValueAvailable()) { + return $parameter->getDefaultValue(); } - throw new \BadMethodCallException(self::parameterError($parameter, $for) . ' expects unsupported type ' . $type); + if ($type->allowsNull()) { + return null; + } + + throw new \Error( + self::parameterError($parameter, $for) . ' expects unsupported type ' . $type + ); } // @codeCoverageIgnoreEnd // load container variables if parameter name is known @@ -313,32 +317,25 @@ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool } throw new \TypeError( - self::parameterError($parameter, $for) . ' must be of type ' . ($type->allowsNull() ? '?' : '') . $type->getName() . ', ' . $this->gettype($value) . ' given' + self::parameterError($parameter, $for) . ' must be of type ' . self::typeName($type) . ', ' . $this->gettype($value) . ' given' ); } - // abort if parameter is untyped and not explicitly defined by container variable - if ($type === null) { - assert($parameter->allowsNull()); + // 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(); } - throw new \BadMethodCallException(self::parameterError($parameter, $for) . ' has no type'); - } - - // use default/nullable argument if not loadable as container variable or by type - assert($type instanceof \ReflectionNamedType); - if ($hasDefault && ($type->isBuiltin() || !\array_key_exists($type->getName(), $this->container))) { - return $parameter->isDefaultValueAvailable() ? $parameter->getDefaultValue() : null; + if ($type !== null && $type->allowsNull() && $type->getName() !== 'mixed') { + return null; + } } // abort if required container variable is not defined or for any other primitive types (array etc.) - if ($type->isBuiltin()) { - if ($allowVariables) { - throw new \BadMethodCallException(self::parameterError($parameter, $for) . ' is not defined'); - } else { - throw new \BadMethodCallException(self::parameterError($parameter, $for) . ' expects unsupported type ' . $type->getName()); - } + if (!$type instanceof \ReflectionNamedType || $type->isBuiltin()) { + throw new \Error( + self::parameterError($parameter, $for) . ' requires container config' . ($type !== null ? ' with type ' . self::typeName($type) : '') . ', none given' + ); } // abort for unreasonably deep nesting or recursive types @@ -358,7 +355,7 @@ private function hasVariable(string $name): bool /** * @return object|string|int|float|bool|null * @throws \TypeError if container factory returns an unexpected type - * @throws \BadMethodCallException if $name can not be loaded + * @throws \Error|\BadMethodCallException if $name can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ private function loadVariable(string $name, int $depth = 64) /*: object|string|int|float|bool|null (PHP 8.0+) */ @@ -440,6 +437,17 @@ private static function parameterError(\ReflectionParameter $parameter, string $ return 'Argument #' . ($parameter->getPosition() + 1) . ' ($' . $parameter->getName() . ') of ' . self::functionName($parameter->getDeclaringFunction()) . ($for !== '' ? ' for ' . $for : ''); } + /** + * @param \ReflectionNamedType $type + * @return string + * @throws void + * @see https://www.php.net/manual/en/reflectiontype.tostring.php (PHP 8+) + */ + private static function typeName(\ReflectionNamedType $type): string + { + return ($type->allowsNull() && $type->getName() !== 'mixed' ? '?' : '') . $type->getName(); + } + /** * @param mixed $value * @return string diff --git a/tests/AppTest.php b/tests/AppTest.php index d9f4c85..eca1107 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -1637,12 +1637,12 @@ public function provideInvalidClasses(): \Generator yield [ InvalidConstructorUntyped::class, - 'Argument #1 ($value) of %s::__construct() has no type' + 'Argument #1 ($value) of %s::__construct() requires container config, none given' ]; yield [ InvalidConstructorInt::class, - 'Argument #1 ($value) of %s::__construct() expects unsupported type int' + 'Argument #1 ($value) of %s::__construct() requires container config with type int, none given' ]; if (PHP_VERSION_ID >= 80000) { diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index 5aaf1c3..5c66734 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -1300,7 +1300,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($username) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass is not defined'); + $this->expectExceptionMessage('Argument #1 ($username) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass requires container config with type string, none given'); $callable($request); } @@ -1663,7 +1663,7 @@ public function __construct(string $name) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($name) of class@anonymous::__construct() expects unsupported type string'); + $this->expectExceptionMessage('Argument #1 ($name) of class@anonymous::__construct() requires container config with type string, none given'); $callable($request); } @@ -1816,7 +1816,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUntypedA $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass has no type'); + $this->expectExceptionMessage('Argument #1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass requires container config, none given'); $callable($request); } @@ -1835,7 +1835,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUndefine $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass is not defined'); + $this->expectExceptionMessage('Argument #1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass requires container config with type mixed, none given'); $callable($request); } @@ -2184,8 +2184,8 @@ public function testGetEnvThrowsWhenFactoryUsesBuiltInFunctionThatReferencesUnkn 'X_FOO' => \Closure::fromCallable('extension_loaded') ]); - $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($extension) of extension_loaded() for $X_FOO is not defined'); + $this->expectException(\Error::class); + $this->expectExceptionMessage('Argument #1 ($extension) of extension_loaded() for $X_FOO requires container config with type string, none given'); $container->getEnv('X_FOO'); } @@ -2202,8 +2202,51 @@ public function foo(string $bar): string 'X_FOO' => \Closure::fromCallable([$class, 'foo']) ]); - $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Argument #1 ($bar) of class@anonymous::foo() for $X_FOO is not defined'); + $this->expectException(\Error::class); + $this->expectExceptionMessage('Argument #1 ($bar) of class@anonymous::foo() for $X_FOO requires container config with type string, none given'); + $container->getEnv('X_FOO'); + } + + public function testGetEnvThrowsWhenFactoryFunctionExpectsRequiredEnvVariableButNoneGiven(): void + { + $line = __LINE__ + 2; + $container = new Container([ + 'X_FOO' => function (string $X_UNDEFINED) { return (string) $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'); + } + + public function testGetEnvThrowsWhenFactoryFunctionExpectsNullableIntArgumentButGivenString(): void + { + $line = __LINE__ + 2; + $container = new Container([ + 'X_FOO' => function (?int $bar) { return (string) $bar; }, + 'bar' => 'bar' + ]); + + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Argument #1 ($bar) of {closure:' . __FILE__ . ':' . $line . '}() for $X_FOO must be of type ?int, string given'); + $container->getEnv('X_FOO'); + } + + /** + * @requires PHP 8 + */ + public function testGetEnvThrowsWhenFactoryFunctionExpectsUnsupportedUnionType(): void + { + $line = __LINE__ + 2; + $fn = null; + $fn = #[PHP8] function (string|int $X_UNION) { return (string) $X_UNION; }; + $container = new Container([ + 'X_FOO' => $fn, + 'X_UNION' => 42 + ]); + + $this->expectException(\Error::class); + $this->expectExceptionMessage('Argument #1 ($X_UNION) of {closure:' . __FILE__ . ':' . $line . '}() for $X_FOO expects unsupported type string|int'); $container->getEnv('X_FOO'); } From 1d19f898b91c4a0aa3216cda557f5fdb4556c926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sun, 16 Mar 2025 23:36:33 +0100 Subject: [PATCH 7/8] Improve exception messages for recursive container config --- src/Container.php | 18 +++++++----------- tests/ContainerTest.php | 27 ++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/Container.php b/src/Container.php index 93f970a..876cef8 100644 --- a/src/Container.php +++ b/src/Container.php @@ -190,7 +190,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) if (\array_key_exists($name, $this->container)) { if (\is_string($this->container[$name])) { if ($depth < 1) { - throw new \BadMethodCallException('Container config for ' . $name . ' is recursive'); + throw new \Error('Container config for ' . $name . ' is recursive'); } // @phpstan-ignore-next-line because type of container value is explicitly checked after getting here @@ -212,7 +212,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) if (\is_string($value)) { if ($depth < 1) { - throw new \BadMethodCallException('Container config for ' . $name . ' is recursive'); + throw new \Error('Container config for ' . $name . ' is recursive'); } // @phpstan-ignore-next-line because type of container value is explicitly checked after getting here @@ -287,6 +287,11 @@ private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $ */ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool $allowVariables, string $for) /*: mixed (PHP 8.0+) */ { + // abort for unreasonably deep nesting or recursive types + if ($depth < 1) { + throw new \Error(self::parameterError($parameter, $for) . ' is recursive'); + } + assert(\is_array($this->container)); $type = $parameter->getType(); @@ -338,11 +343,6 @@ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool ); } - // abort for unreasonably deep nesting or recursive types - if ($depth < 1) { - throw new \BadMethodCallException(self::parameterError($parameter, $for) . ' is recursive'); - } - // @phpstan-ignore-next-line because `$type->getName()` is a `class-string` by definition return $this->loadObject($type->getName(), $depth - 1); } @@ -364,10 +364,6 @@ private function loadVariable(string $name, int $depth = 64) /*: object|string|i assert(\is_array($this->container) || !$this->container->has($name)); if (\is_array($this->container) && ($this->container[$name] ?? null) instanceof \Closure) { - if ($depth < 1) { - throw new \BadMethodCallException('Container config for $' . $name . ' is recursive'); - } - // build list of factory parameters based on parameter types $factory = $this->container[$name]; assert($factory instanceof \Closure); diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index 5c66734..fb21bb3 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -1315,6 +1315,7 @@ public function __construct(\stdClass $data) } }; + $line = __LINE__ + 2; $container = new Container([ \stdClass::class => function (string $stdClass) { return (object) ['name' => $stdClass]; @@ -1324,7 +1325,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Container config for $stdClass is recursive'); + $this->expectExceptionMessage('Argument #1 ($stdClass) of {closure:' . __FILE__ . ':' . $line .'}() for $stdClass is recursive'); $callable($request); } @@ -2357,6 +2358,30 @@ public function testGetAccessLogHandlerThrowsIfFactoryFunctionReturnsInvalidValu $container->getAccessLogHandler(); } + public function testGetAccessLogHandlerThrowsIfConfigIsRecursive(): void + { + $container = new Container([ + AccessLogHandler::class => AccessLogHandler::class + ]); + + $this->expectException(\Error::class); + $this->expectExceptionMessage('Container config for FrameworkX\AccessLogHandler is recursive'); + $container->getAccessLogHandler(); + } + + public function testGetAccessLogHandlerThrowsIfFactoryFunctionIsRecursive(): void + { + $container = new Container([ + AccessLogHandler::class => function () { + return AccessLogHandler::class; + } + ]); + + $this->expectException(\Error::class); + $this->expectExceptionMessage('Container config for FrameworkX\AccessLogHandler is recursive'); + $container->getAccessLogHandler(); + } + public function testGetErrorHandlerReturnsDefaultErrorHandlerInstance(): void { $container = new Container([]); From 5dd0883e36f36d4ff47c6b1a68136f0b535a22e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Mon, 8 Dec 2025 17:40:45 +0100 Subject: [PATCH 8/8] Improve exception messages for invalid class references --- src/Container.php | 23 +++++------- tests/AppTest.php | 6 +-- tests/ContainerTest.php | 82 ++++++++++++++++++++++++++--------------- 3 files changed, 65 insertions(+), 46 deletions(-) diff --git a/src/Container.php b/src/Container.php index 876cef8..111fe63 100644 --- a/src/Container.php +++ b/src/Container.php @@ -77,11 +77,6 @@ public function __invoke(ServerRequestInterface $request, ?callable $next = null public function callable(string $class): callable { return function (ServerRequestInterface $request, ?callable $next = null) use ($class) { - // Check `$class` references a valid class name that can be autoloaded - if (\is_array($this->container) && !\class_exists($class, true) && !interface_exists($class, false) && !trait_exists($class, false)) { - throw new \BadMethodCallException('Request handler class ' . $class . ' not found'); - } - try { if ($this->container instanceof ContainerInterface) { $handler = $this->container->get($class); @@ -89,7 +84,7 @@ public function callable(string $class): callable $handler = $this->loadObject($class); } } catch (\Throwable $e) { - throw new \BadMethodCallException( + throw new \Error( 'Request handler class ' . $class . ' failed to load: ' . $e->getMessage(), 0, $e @@ -100,8 +95,8 @@ public function callable(string $class): callable // This initial version is intentionally limited to checking the method name only. // A follow-up version will likely use reflection to check request handler argument types. if (!is_callable($handler)) { - throw new \BadMethodCallException( - 'Request handler class ' . \explode("\0", $class)[0] . ' has no public __invoke() method' + throw new \Error( + 'Request handler ' . \explode("\0", $class)[0] . ' has no public __invoke() method' ); } @@ -180,7 +175,7 @@ public function getErrorHandler(): ErrorHandler * @param class-string $name * @return T * @throws \TypeError if container config or factory returns an unexpected type - * @throws \Error|\BadMethodCallException if object of type $name can not be loaded + * @throws \Error if object of type $name can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) */ @@ -238,7 +233,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) // Check `$name` references a valid class name that can be autoloaded if (!\class_exists($name, true) && !interface_exists($name, false) && !trait_exists($name, false)) { - throw new \BadMethodCallException('Class ' . $name . ' not found'); + throw new \Error('Class ' . $name . ' not found'); } $class = new \ReflectionClass($name); @@ -251,7 +246,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) } elseif ($class->isTrait()) { $modifier = 'trait'; } - throw new \BadMethodCallException('Cannot instantiate ' . $modifier . ' '. $name); + throw new \Error('Cannot instantiate ' . $modifier . ' '. $name); } // build list of constructor parameters based on parameter types @@ -266,7 +261,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) /** * @return list * @throws \TypeError if container config or factory returns an unexpected type - * @throws \Error|\BadMethodCallException if either parameter can not be loaded + * @throws \Error if either parameter can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $depth, bool $allowVariables, string $for): array @@ -282,7 +277,7 @@ private function loadFunctionParams(\ReflectionFunctionAbstract $function, int $ /** * @return mixed * @throws \TypeError if container config or factory returns an unexpected type - * @throws \Error|\BadMethodCallException if $parameter can not be loaded + * @throws \Error if $parameter can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ private function loadParameter(\ReflectionParameter $parameter, int $depth, bool $allowVariables, string $for) /*: mixed (PHP 8.0+) */ @@ -355,7 +350,7 @@ private function hasVariable(string $name): bool /** * @return object|string|int|float|bool|null * @throws \TypeError if container factory returns an unexpected type - * @throws \Error|\BadMethodCallException if $name can not be loaded + * @throws \Error if $name can not be loaded * @throws \Throwable if container factory function throws unexpected exception */ private function loadVariable(string $name, int $depth = 64) /*: object|string|int|float|bool|null (PHP 8.0+) */ diff --git a/tests/AppTest.php b/tests/AppTest.php index eca1107..f16189b 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -1605,7 +1605,7 @@ public function testInvokeWithMatchingRouteReturnsInternalServerErrorResponseWhe $this->assertStringContainsString("Error 500: Internal Server Error\n", (string) $response->getBody()); $this->assertStringContainsString("

The requested page failed to load, please try again later.

\n", (string) $response->getBody()); - $this->assertStringMatchesFormat("%a

Expected request handler to return Psr\Http\Message\ResponseInterface but got uncaught BadMethodCallException with message Request handler class UnknownClass not found in Container.php:%d.

\n%a", (string) $response->getBody()); + $this->assertStringMatchesFormat("%a

Expected request handler to return Psr\Http\Message\ResponseInterface but got uncaught Error with message Request handler class UnknownClass failed to load: Class UnknownClass not found in Container.php:%d.

\n%a", (string) $response->getBody()); } public function provideInvalidClasses(): \Generator @@ -1692,7 +1692,7 @@ public function testInvokeWithMatchingRouteReturnsInternalServerErrorResponseWhe $this->assertStringContainsString("Error 500: Internal Server Error\n", (string) $response->getBody()); $this->assertStringContainsString("

The requested page failed to load, please try again later.

\n", (string) $response->getBody()); - $this->assertStringMatchesFormat("%a

Expected request handler to return Psr\Http\Message\ResponseInterface but got uncaught BadMethodCallException with message Request handler class " . $class . " failed to load: $error in Container.php:%d.

\n%a", (string) $response->getBody()); + $this->assertStringMatchesFormat("%a

Expected request handler to return Psr\Http\Message\ResponseInterface but got uncaught Error with message Request handler class $class failed to load: $error in Container.php:%d.

\n%a", (string) $response->getBody()); } public function testInvokeWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerClassRequiresUnexpectedCallableParameter(): void @@ -1739,7 +1739,7 @@ public function testInvokeWithMatchingRouteReturnsInternalServerErrorResponseWhe $this->assertStringContainsString("Error 500: Internal Server Error\n", (string) $response->getBody()); $this->assertStringContainsString("

The requested page failed to load, please try again later.

\n", (string) $response->getBody()); - $this->assertStringMatchesFormat("%a

Expected request handler to return Psr\Http\Message\ResponseInterface but got uncaught BadMethodCallException with message Request handler class class@anonymous has no public __invoke() method in Container.php:%d.

\n%a", (string) $response->getBody()); + $this->assertStringMatchesFormat("%a

Expected request handler to return Psr\Http\Message\ResponseInterface but got uncaught Error with message Request handler class@anonymous has no public __invoke() method in Container.php:%d.

\n%a", (string) $response->getBody()); } public function testInvokeWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerReturnsPromiseWhichFulfillsWithWrongValue(): void diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index fb21bb3..c63a130 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -1264,6 +1264,19 @@ public function __invoke(): ResponseInterface $this->assertEquals('"bar"', (string) $response->getBody()); } + public function testCallableReturnsCallableThatThrowsForUnknownClass(): void + { + $request = new ServerRequest('GET', 'http://example.com/'); + + $container = new Container([]); + + $callable = $container->callable('UnknownClass'); // @phpstan-ignore-line + + $this->expectException(\Error::class); + $this->expectExceptionMessage('Request handler class UnknownClass failed to load: Class UnknownClass not found'); + $callable($request); + } + public function testCallableReturnsCallableThatThrowsForNonCallableClass(): void { $request = new ServerRequest('GET', 'http://example.com/'); @@ -1274,8 +1287,8 @@ public function testCallableReturnsCallableThatThrowsForNonCallableClass(): void $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Request handler class class@anonymous has no public __invoke() method'); + $this->expectException(\Error::class); + $this->expectExceptionMessage('Request handler class@anonymous has no public __invoke() method'); $callable($request); } @@ -1299,7 +1312,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $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); } @@ -1324,7 +1337,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Argument #1 ($stdClass) of {closure:' . __FILE__ . ':' . $line .'}() for $stdClass is recursive'); $callable($request); } @@ -1351,7 +1364,7 @@ public function __construct(string $stdClass) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Argument #1 ($stdClass) of {closure:' . __FILE__ . ':' . $line . '}() for class@anonymous must be of type string, stdClass given'); $callable($request); } @@ -1379,7 +1392,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Return value of {closure:' . __FILE__ . ':' . $line . '}() for $http must be of type object|string|int|float|bool|null, resource returned'); $callable($request); } @@ -1405,7 +1418,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Argument #1 ($http) of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type stdClass, int given'); $callable($request); } @@ -1431,7 +1444,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Argument #1 ($http) of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type string, int given'); $callable($request); } @@ -1457,7 +1470,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Argument #1 ($http) of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type int, string given'); $callable($request); } @@ -1483,7 +1496,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Argument #1 ($percent) of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type float, string given'); $callable($request); } @@ -1509,7 +1522,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Argument #1 ($admin) of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type bool, string given'); $callable($request); } @@ -1531,7 +1544,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Class Yes not found'); $callable($request); } @@ -1553,7 +1566,7 @@ public function __construct(?\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Class Yes not found'); $callable($request); } @@ -1575,7 +1588,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Return value of ' . Container::class . '::loadObject() for stdClass must be of type stdClass, int returned'); $callable($request); } @@ -1597,7 +1610,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Return value of ' . Container::class . '::loadObject() for stdClass must be of type stdClass, null returned'); $callable($request); } @@ -1619,7 +1632,7 @@ public function __construct(?\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Return value of ' . Container::class . '::loadObject() for stdClass must be of type stdClass, null returned'); $callable($request); } @@ -1641,7 +1654,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Return value of ' . Container::class . '::loadObject() for stdClass must be of type stdClass, React\Http\Message\Response returned'); $callable($request); } @@ -1663,7 +1676,7 @@ public function __construct(string $name) $callable = $container->callable(get_class($controller)); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Argument #1 ($name) of class@anonymous::__construct() requires container config with type string, none given'); $callable($request); } @@ -1738,7 +1751,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryReturnsInvalidCl $callable = $container->callable(\stdClass::class); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Class invalid not found'); $callable($request); } @@ -1754,7 +1767,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryReturnsInvalidIn $callable = $container->callable(\stdClass::class); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Return value of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type stdClass, int returned'); $callable($request); } @@ -1769,7 +1782,7 @@ public function testCallableReturnsCallableThatThrowsWhenMapReferencesClassNameT $callable = $container->callable(\stdClass::class); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Return value of ' . Container::class . '::loadObject() for stdClass must be of type stdClass, React\Http\Message\Response returned'); $callable($request); } @@ -1785,7 +1798,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryReturnsClassName $callable = $container->callable(\stdClass::class); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Return value of {closure:' . __FILE__ . ':' . $line . '}() for stdClass must be of type stdClass, React\Http\Message\Response returned'); $callable($request); } @@ -1800,7 +1813,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresInvalidC $callable = $container->callable(\stdClass::class); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Class self not found'); $callable($request); } @@ -1816,7 +1829,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUntypedA $callable = $container->callable(\stdClass::class); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Argument #1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass requires container config, none given'); $callable($request); } @@ -1835,7 +1848,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUndefine $callable = $container->callable(\stdClass::class); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Argument #1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass requires container config with type mixed, none given'); $callable($request); } @@ -1851,7 +1864,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresRecursiv $callable = $container->callable(\stdClass::class); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Argument #1 ($data) of {closure:' . __FILE__ . ':' . $line .'}() for stdClass is recursive'); $callable($request); } @@ -1866,7 +1879,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryIsRecursive(): v $callable = $container->callable(\stdClass::class); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Container config for stdClass is recursive'); $callable($request); } @@ -1883,7 +1896,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryIsRecursiveClass $callable = $container->callable(\stdClass::class); - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Container config for stdClass is recursive'); $callable($request); } @@ -1929,7 +1942,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryReturnsInvalidCl $callable = $container->callable('FooBar'); // @phpstan-ignore-line - $this->expectException(\BadMethodCallException::class); + $this->expectException(\Error::class); $this->expectExceptionMessage('Request handler class FooBar failed to load: Unable to load class'); $callable($request); } @@ -2382,6 +2395,17 @@ public function testGetAccessLogHandlerThrowsIfFactoryFunctionIsRecursive(): voi $container->getAccessLogHandler(); } + public function testGetAccessLogHandlerThrowsIfConfigReferencesInterface(): void + { + $container = new Container([ + AccessLogHandler::class => \Iterator::class + ]); + + $this->expectException(\Error::class); + $this->expectExceptionMessage('Cannot instantiate interface Iterator'); + $container->getAccessLogHandler(); + } + public function testGetErrorHandlerReturnsDefaultErrorHandlerInstance(): void { $container = new Container([]);