From 1c8beb03c17a0182955b22c592c7b36f5a8325b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Tue, 28 Jan 2025 21:04:57 +0100 Subject: [PATCH 1/3] Improve exception messages for closures to always match PHP 8.4+ format --- src/Container.php | 8 ++++-- tests/ContainerTest.php | 56 +++++++++++++++++++++++++++++------------ 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/src/Container.php b/src/Container.php index 42dd1d3..ecc17a2 100644 --- a/src/Container.php +++ b/src/Container.php @@ -372,8 +372,12 @@ private function loadVariable(string $name, string $type, bool $nullable, int $d /** @throws void */ private static function parameterError(\ReflectionParameter $parameter): string { - $name = $parameter->getDeclaringFunction()->getShortName(); - if (!$parameter->getDeclaringFunction()->isClosure() && ($class = $parameter->getDeclaringClass()) !== null) { + $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) { $name = explode("\0", $class->getName())[0] . '::' . $name; } diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index a3f7899..e181475 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -1275,6 +1275,7 @@ public function __construct(\stdClass $data) } }; + $line = __LINE__ + 2; $container = new Container([ \stdClass::class => function (string $username) { return (object) ['name' => $username]; @@ -1284,7 +1285,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessageMatches('/Argument 1 \(\$username\) of {closure(:[^{}]+)?}\(\) is not defined$/'); + $this->expectExceptionMessage('Argument 1 ($username) of {closure:' . __FILE__ . ':' . $line .'}() is not defined'); $callable($request); } @@ -1763,6 +1764,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUntypedA { $request = new ServerRequest('GET', 'http://example.com/'); + $line = __LINE__ + 2; $container = new Container([ \stdClass::class => function ($undefined) { return $undefined; } ]); @@ -1770,7 +1772,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUntypedA $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessageMatches('/Argument 1 \(\$undefined\) of {closure(:[^{}]+)?}\(\) has no type$/'); + $this->expectExceptionMessage('Argument 1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() has no type'); $callable($request); } @@ -1781,6 +1783,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUndefine { $request = new ServerRequest('GET', 'http://example.com/'); + $line = __LINE__ + 2; $container = new Container([ \stdClass::class => function (mixed $undefined) { return $undefined; } ]); @@ -1788,7 +1791,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUndefine $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessageMatches('/Argument 1 \(\$undefined\) of {closure(:[^{}]+)?}\(\) is not defined$/'); + $this->expectExceptionMessage('Argument 1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() is not defined'); $callable($request); } @@ -1796,6 +1799,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresRecursiv { $request = new ServerRequest('GET', 'http://example.com/'); + $line = __LINE__ + 2; $container = new Container([ \stdClass::class => function (\stdClass $data) { return $data; } ]); @@ -1803,7 +1807,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresRecursiv $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessageMatches('/Argument 1 \(\$data\) of {closure(:[^{}]+)?}\(\) is recursive$/'); + $this->expectExceptionMessage('Argument 1 ($data) of {closure:' . __FILE__ . ':' . $line .'}() is recursive'); $callable($request); } @@ -2087,6 +2091,38 @@ public function testGetEnvThrowsIfMapContainsInvalidType(): void $container->getEnv('X_FOO'); } + /** + * @requires PHP 8 + */ + public function testGetEnvThrowsWhenFactoryUsesBuiltInFunctionThatReferencesUnknownVariable(): void + { + $container = new Container([ + 'X_FOO' => \Closure::fromCallable('extension_loaded') + ]); + + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('Argument 1 ($extension) of extension_loaded() is not defined'); + $container->getEnv('X_FOO'); + } + + public function testGetEnvThrowsWhenFactoryUsesClassMethodThatReferencesUnknownVariable(): void + { + $class = new class { + public function foo(string $bar): string + { + return $bar; + } + }; + + $container = new Container([ + 'X_FOO' => \Closure::fromCallable([$class, 'foo']) + ]); + + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('Argument 1 ($bar) of class@anonymous::foo() is not defined'); + $container->getEnv('X_FOO'); + } + public function testGetEnvThrowsIfMapPsrContainerReturnsInvalidType(): void { $psr = $this->createMock(ContainerInterface::class); @@ -2233,16 +2269,4 @@ public function testCtorWithInvalidValueThrows(): void $this->expectExceptionMessage('Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, stdClass given'); new Container((object) []); // @phpstan-ignore-line } - - public function expectExceptionMessageMatches(string $regularExpression): void - { - if (method_exists(parent::class, 'expectExceptionMessageMatches')) { - // @phpstan-ignore-next-line PHPUnit 8.4+ - parent::expectExceptionMessageMatches($regularExpression); - } else { - // legacy PHPUnit - assert(method_exists($this, 'expectExceptionMessageRegExp')); - $this->expectExceptionMessageRegExp($regularExpression); - } - } } From eedc82f432f0c0cdc2fab713634c4b456907f2bf 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 2/3] Improve exception messages for invalid types to match PHP 8.3+ format --- src/Container.php | 36 +++++++++++++++++++++------- tests/ContainerTest.php | 53 +++++++++++++++++++++++++++++++---------- 2 files changed, 69 insertions(+), 20 deletions(-) diff --git a/src/Container.php b/src/Container.php index ecc17a2..b25b800 100644 --- a/src/Container.php +++ b/src/Container.php @@ -22,7 +22,7 @@ public function __construct($loader = []) /** @var mixed $loader explicit type check for mixed if user ignores parameter type */ if (!\is_array($loader) && !$loader instanceof ContainerInterface) { throw new \TypeError( - 'Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, ' . (\is_object($loader) ? get_class($loader) : gettype($loader)) . ' given' + 'Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, ' . $this->gettype($loader) . ' given' ); } @@ -31,7 +31,7 @@ public function __construct($loader = []) (!\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 ' . (is_object($value) ? get_class($value) : gettype($value))); + throw new \BadMethodCallException('Map for ' . $name . ' contains unexpected ' . $this->gettype($value)); } } $this->container = $loader; @@ -113,7 +113,7 @@ public function getEnv(string $name): ?string } if (!\is_string($value) && $value !== null) { - throw new \TypeError('Environment variable $' . $name . ' expected type string|null, but got ' . (\is_object($value) ? \get_class($value) : \gettype($value))); + throw new \TypeError('Environment variable $' . $name . ' expected type string|null, but got ' . $this->gettype($value)); } return $value; @@ -166,7 +166,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) // @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 ' . \get_class($value)); + throw new \BadMethodCallException('Factory for ' . $name . ' returned unexpected ' . $this->gettype($value)); } $this->container[$name] = $value; @@ -187,12 +187,12 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) $value = $this->loadObject($value, $depth - 1); } if (!$value instanceof $name) { - throw new \BadMethodCallException('Factory for ' . $name . ' returned unexpected ' . (is_object($value) ? get_class($value) : gettype($value))); + throw new \BadMethodCallException('Factory for ' . $name . ' returned unexpected ' . $this->gettype($value)); } $this->container[$name] = $value; } elseif (!$this->container[$name] instanceof $name) { - throw new \BadMethodCallException('Map for ' . $name . ' contains unexpected ' . (\is_object($this->container[$name]) ? \get_class($this->container[$name]) : \gettype($this->container[$name]))); + throw new \BadMethodCallException('Map for ' . $name . ' contains unexpected ' . $this->gettype($this->container[$name])); } assert($this->container[$name] instanceof $name); @@ -329,7 +329,7 @@ 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 ' . \gettype($value)); + throw new \BadMethodCallException('Container variable $' . $name . ' expected type object|scalar|null from factory, but got ' . $this->gettype($value)); } $this->container[$name] = $value; @@ -363,7 +363,7 @@ 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 ' . (\is_object($value) ? \get_class($value) : \gettype($value))); + throw new \BadMethodCallException('Container variable $' . $name . ' expected type ' . $type . ', but got ' . $this->gettype($value)); } return $value; @@ -383,4 +383,24 @@ private static function parameterError(\ReflectionParameter $parameter): string return 'Argument ' . ($parameter->getPosition() + 1) . ' ($' . $parameter->getName() . ') of ' . $name . '()'; } + + /** + * @param mixed $value + * @return string + * @throws void + * @see https://www.php.net/manual/en/function.get-debug-type.php (PHP 8+) + */ + private function gettype($value): string + { + if (\is_int($value)) { + return 'int'; + } elseif (\is_float($value)) { + return 'float'; + } elseif (\is_bool($value)) { + return \var_export($value, true); + } elseif ($value === null) { + return 'null'; + } + return \is_object($value) ? \get_class($value) : \gettype($value); + } } diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index e181475..20f0cd0 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -1387,7 +1387,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 integer'); + $this->expectExceptionMessage('Container variable $http expected type stdClass, but got int'); $callable($request); } @@ -1412,7 +1412,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 integer'); + $this->expectExceptionMessage('Container variable $http expected type string, but got int'); $callable($request); } @@ -1553,7 +1553,7 @@ public function __construct(\stdClass $data) $callable = $container->callable(get_class($controller)); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Map for stdClass contains unexpected integer'); + $this->expectExceptionMessage('Map for stdClass contains unexpected int'); $callable($request); } @@ -1575,7 +1575,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('Map for stdClass contains unexpected null'); $callable($request); } @@ -1597,7 +1597,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('Map for stdClass contains unexpected null'); $callable($request); } @@ -1678,7 +1678,7 @@ public function testCtorThrowsWhenMapForClassContainsInvalidObject(): void public function testCtorThrowsWhenMapForClassContainsInvalidNull(): void { $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Map for Psr\Http\Message\ResponseInterface contains unexpected NULL'); + $this->expectExceptionMessage('Map for Psr\Http\Message\ResponseInterface contains unexpected null'); new Container([ ResponseInterface::class => null @@ -1711,7 +1711,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryReturnsInvalidIn $callable = $container->callable(\stdClass::class); $this->expectException(\BadMethodCallException::class); - $this->expectExceptionMessage('Factory for stdClass returned unexpected integer'); + $this->expectExceptionMessage('Factory for stdClass returned unexpected int'); $callable($request); } @@ -2087,7 +2087,7 @@ public function testGetEnvThrowsIfMapContainsInvalidType(): void ]); $this->expectException(\TypeError::class); - $this->expectExceptionMessage('Environment variable $X_FOO expected type string|null, but got boolean'); + $this->expectExceptionMessage('Environment variable $X_FOO expected type string|null, but got false'); $container->getEnv('X_FOO'); } @@ -2133,7 +2133,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 integer'); + $this->expectExceptionMessage('Environment variable $X_FOO expected type string|null, but got int'); $container->getEnv('X_FOO'); } @@ -2263,10 +2263,39 @@ public function testInvokeContainerAsFinalRequestHandlerThrows(): void $container($request); } - public function testCtorWithInvalidValueThrows(): void + public static function provideInvalidContainerConfigValues(): \Generator + { + yield [ + (object) [], + \stdClass::class + ]; + yield [ + new Container([]), + Container::class + ]; + yield [ + true, + 'true' + ]; + yield [ + false, + 'false' + ]; + yield [ + 1.0, + 'float' + ]; + } + + /** + * @dataProvider provideInvalidContainerConfigValues + * @param mixed $value + * @param string $type + */ + public function testCtorWithInvalidValueThrows($value, string $type): void { $this->expectException(\TypeError::class); - $this->expectExceptionMessage('Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, stdClass given'); - new Container((object) []); // @phpstan-ignore-line + $this->expectExceptionMessage('Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, ' . $type . ' given'); + new Container($value); // @phpstan-ignore-line } } From 5f1cba28469e992e126719782776417df3447aa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sat, 21 Dec 2024 18:41:53 +0100 Subject: [PATCH 3/3] Improve exception messages for argument positions to match PHP 8+ format --- src/Container.php | 2 +- tests/AppTest.php | 10 +++++----- tests/ContainerTest.php | 14 +++++++------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Container.php b/src/Container.php index b25b800..3ea15fd 100644 --- a/src/Container.php +++ b/src/Container.php @@ -381,7 +381,7 @@ private static function parameterError(\ReflectionParameter $parameter): string $name = explode("\0", $class->getName())[0] . '::' . $name; } - return 'Argument ' . ($parameter->getPosition() + 1) . ' ($' . $parameter->getName() . ') of ' . $name . '()'; + return 'Argument #' . ($parameter->getPosition() + 1) . ' ($' . $parameter->getName() . ') of ' . $name . '()'; } /** diff --git a/tests/AppTest.php b/tests/AppTest.php index ab7a4ae..b27919f 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -1529,25 +1529,25 @@ public function provideInvalidClasses(): \Generator yield [ InvalidConstructorUntyped::class, - 'Argument 1 ($value) of %s::__construct() has no type' + 'Argument #1 ($value) of %s::__construct() has no type' ]; yield [ InvalidConstructorInt::class, - 'Argument 1 ($value) of %s::__construct() expects unsupported type int' + 'Argument #1 ($value) of %s::__construct() expects unsupported type int' ]; if (PHP_VERSION_ID >= 80000) { yield [ InvalidConstructorUnion::class, - 'Argument 1 ($value) of %s::__construct() expects unsupported type int|float' + 'Argument #1 ($value) of %s::__construct() expects unsupported type int|float' ]; } if (PHP_VERSION_ID >= 80100) { yield [ InvalidConstructorIntersection::class, - 'Argument 1 ($value) of %s::__construct() expects unsupported type Traversable&ArrayAccess' + 'Argument #1 ($value) of %s::__construct() expects unsupported type Traversable&ArrayAccess' ]; } @@ -1558,7 +1558,7 @@ public function provideInvalidClasses(): \Generator yield [ InvalidConstructorSelf::class, - 'Argument 1 ($value) of %s::__construct() is recursive' + 'Argument #1 ($value) of %s::__construct() is recursive' ]; } diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index 20f0cd0..31aabb4 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 .'}() is not defined'); $callable($request); } @@ -1641,7 +1641,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() expects unsupported type string'); $callable($request); } @@ -1772,7 +1772,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 .'}() has no type'); $callable($request); } @@ -1791,7 +1791,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 .'}() is not defined'); $callable($request); } @@ -1807,7 +1807,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 .'}() is recursive'); $callable($request); } @@ -2101,7 +2101,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() is not defined'); $container->getEnv('X_FOO'); } @@ -2119,7 +2119,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() is not defined'); $container->getEnv('X_FOO'); }