Skip to content

Commit

Permalink
Merge pull request #274 from clue-labs/container-messages
Browse files Browse the repository at this point in the history 
Improve exception messages for `Container` to always match PHP 8+ format
  • Loading branch information
@cassifyit
cassifyit authored Jan 30, 2025
2 parents aedab13 + 5f1cba2 commit 41006f1
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 43 deletions.
46 changes: 35 additions & 11 deletions src/Container.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
}

Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -372,11 +372,35 @@ 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;
}

return 'Argument ' . ($parameter->getPosition() + 1) . ' ($' . $parameter->getName() . ') of ' . $name . '()';
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);
}
}
10 changes: 5 additions & 5 deletions tests/AppTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
];
}

Expand All @@ -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'
];
}

Expand Down
107 changes: 80 additions & 27 deletions tests/ContainerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1275,6 +1275,7 @@ public function __construct(\stdClass $data)
}
};

$line = __LINE__ + 2;
$container = new Container([
\stdClass::class => function (string $username) {
return (object) ['name' => $username];
Expand All @@ -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);
}

Expand Down Expand Up @@ -1386,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);
}

Expand All @@ -1411,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);
}

Expand Down Expand Up @@ -1552,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);
}

Expand All @@ -1574,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);
}

Expand All @@ -1596,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);
}

Expand Down Expand Up @@ -1640,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);
}

Expand Down Expand Up @@ -1677,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
Expand Down Expand Up @@ -1710,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);
}

Expand Down Expand Up @@ -1763,14 +1764,15 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUntypedA
{
$request = new ServerRequest('GET', 'http://example.com/');

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

$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);
}

Expand All @@ -1781,29 +1783,31 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryRequiresUndefine
{
$request = new ServerRequest('GET', 'http://example.com/');

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

$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);
}

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

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

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

$this->expectException(\BadMethodCallException::class);
$this->expectExceptionMessageMatches('/Argument 1 \(\$data\) of {closure(:[^{}]+)?}\(\) is recursive$/');
$this->expectExceptionMessage('Argument #1 ($data) of {closure:' . __FILE__ . ':' . $line .'}() is recursive');
$callable($request);
}

Expand Down Expand Up @@ -2083,7 +2087,39 @@ 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');
}

/**
* @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');
}

Expand All @@ -2097,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');
}

Expand Down Expand Up @@ -2227,22 +2263,39 @@ public function testInvokeContainerAsFinalRequestHandlerThrows(): void
$container($request);
}

public function testCtorWithInvalidValueThrows(): void
public static function provideInvalidContainerConfigValues(): \Generator
{
$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
yield [
(object) [],
\stdClass::class
];
yield [
new Container([]),
Container::class
];
yield [
true,
'true'
];
yield [
false,
'false'
];
yield [
1.0,
'float'
];
}

public function expectExceptionMessageMatches(string $regularExpression): void
/**
* @dataProvider provideInvalidContainerConfigValues
* @param mixed $value
* @param string $type
*/
public function testCtorWithInvalidValueThrows($value, string $type): 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);
}
$this->expectException(\TypeError::class);
$this->expectExceptionMessage('Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, ' . $type . ' given');
new Container($value); // @phpstan-ignore-line
}
}

0 comments on commit 41006f1

Please sign in to comment.