Skip to content

Commit 41006f1

Browse files
cassifyitcassifyit
authored
Merge pull request clue#274 from clue-labs/container-messages
Improve exception messages for `Container` to always match PHP 8+ format
2 parents aedab13 + 5f1cba2 commit 41006f1

File tree

3 files changed

+120
-43
lines changed

3 files changed

+120
-43
lines changed

‎src/Container.php‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public function __construct($loader = [])
2222
/** @var mixed $loader explicit type check for mixed if user ignores parameter type */
2323
if (!\is_array($loader) && !$loader instanceof ContainerInterface) {
2424
throw new \TypeError(
25-
'Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, ' . (\is_object($loader) ? get_class($loader) : gettype($loader)) . ' given'
25+
'Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, ' . $this->gettype($loader) . ' given'
2626
);
2727
}
2828

@@ -31,7 +31,7 @@ public function __construct($loader = [])
3131
(!\is_object($value) && !\is_scalar($value) && $value !== null) ||
3232
(!$value instanceof $name && !$value instanceof \Closure && !\is_string($value) && \strpos($name, '\\') !== false)
3333
) {
34-
throw new \BadMethodCallException('Map for ' . $name . ' contains unexpected ' . (is_object($value) ? get_class($value) : gettype($value)));
34+
throw new \BadMethodCallException('Map for ' . $name . ' contains unexpected ' . $this->gettype($value));
3535
}
3636
}
3737
$this->container = $loader;
@@ -113,7 +113,7 @@ public function getEnv(string $name): ?string
113113
}
114114

115115
if (!\is_string($value) && $value !== null) {
116-
throw new \TypeError('Environment variable $' . $name . ' expected type string|null, but got ' . (\is_object($value) ? \get_class($value) : \gettype($value)));
116+
throw new \TypeError('Environment variable $' . $name . ' expected type string|null, but got ' . $this->gettype($value));
117117
}
118118

119119
return $value;
@@ -166,7 +166,7 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+)
166166
// @phpstan-ignore-next-line because type of container value is explicitly checked after getting here
167167
$value = $this->loadObject($this->container[$name], $depth - 1);
168168
if (!$value instanceof $name) {
169-
throw new \BadMethodCallException('Factory for ' . $name . ' returned unexpected ' . \get_class($value));
169+
throw new \BadMethodCallException('Factory for ' . $name . ' returned unexpected ' . $this->gettype($value));
170170
}
171171

172172
$this->container[$name] = $value;
@@ -187,12 +187,12 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+)
187187
$value = $this->loadObject($value, $depth - 1);
188188
}
189189
if (!$value instanceof $name) {
190-
throw new \BadMethodCallException('Factory for ' . $name . ' returned unexpected ' . (is_object($value) ? get_class($value) : gettype($value)));
190+
throw new \BadMethodCallException('Factory for ' . $name . ' returned unexpected ' . $this->gettype($value));
191191
}
192192

193193
$this->container[$name] = $value;
194194
} elseif (!$this->container[$name] instanceof $name) {
195-
throw new \BadMethodCallException('Map for ' . $name . ' contains unexpected ' . (\is_object($this->container[$name]) ? \get_class($this->container[$name]) : \gettype($this->container[$name])));
195+
throw new \BadMethodCallException('Map for ' . $name . ' contains unexpected ' . $this->gettype($this->container[$name]));
196196
}
197197

198198
assert($this->container[$name] instanceof $name);
@@ -329,7 +329,7 @@ private function loadVariable(string $name, string $type, bool $nullable, int $d
329329
$value = $params === [] ? $factory() : $factory(...$params);
330330

331331
if (!\is_object($value) && !\is_scalar($value) && $value !== null) {
332-
throw new \BadMethodCallException('Container variable $' . $name . ' expected type object|scalar|null from factory, but got ' . \gettype($value));
332+
throw new \BadMethodCallException('Container variable $' . $name . ' expected type object|scalar|null from factory, but got ' . $this->gettype($value));
333333
}
334334

335335
$this->container[$name] = $value;
@@ -363,7 +363,7 @@ private function loadVariable(string $name, string $type, bool $nullable, int $d
363363
(!\is_object($value) && !\in_array($type, ['string', 'int', 'float', 'bool'])) ||
364364
($type === 'string' && !\is_string($value)) || ($type === 'int' && !\is_int($value)) || ($type === 'float' && !\is_float($value)) || ($type === 'bool' && !\is_bool($value))
365365
) {
366-
throw new \BadMethodCallException('Container variable $' . $name . ' expected type ' . $type . ', but got ' . (\is_object($value) ? \get_class($value) : \gettype($value)));
366+
throw new \BadMethodCallException('Container variable $' . $name . ' expected type ' . $type . ', but got ' . $this->gettype($value));
367367
}
368368

369369
return $value;
@@ -372,11 +372,35 @@ private function loadVariable(string $name, string $type, bool $nullable, int $d
372372
/** @throws void */
373373
private static function parameterError(\ReflectionParameter $parameter): string
374374
{
375-
$name = $parameter->getDeclaringFunction()->getShortName();
376-
if (!$parameter->getDeclaringFunction()->isClosure() && ($class = $parameter->getDeclaringClass()) !== null) {
375+
$function = $parameter->getDeclaringFunction();
376+
$name = $function->getShortName();
377+
if ($name[0] === '{') { // $function->isAnonymous() (PHP 8.2+)
378+
// use PHP 8.4+ format including closure file and line on all PHP versions: https://3v4l.org/tAs7s
379+
$name = '{closure:' . $function->getFileName() . ':' . $function->getStartLine() . '}';
380+
} elseif (($class = $parameter->getDeclaringClass()) !== null) {
377381
$name = explode("\0", $class->getName())[0] . '::' . $name;
378382
}
379383

380-
return 'Argument ' . ($parameter->getPosition() + 1) . ' ($' . $parameter->getName() . ') of ' . $name . '()';
384+
return 'Argument #' . ($parameter->getPosition() + 1) . ' ($' . $parameter->getName() . ') of ' . $name . '()';
385+
}
386+
387+
/**
388+
* @param mixed $value
389+
* @return string
390+
* @throws void
391+
* @see https://www.php.net/manual/en/function.get-debug-type.php (PHP 8+)
392+
*/
393+
private function gettype($value): string
394+
{
395+
if (\is_int($value)) {
396+
return 'int';
397+
} elseif (\is_float($value)) {
398+
return 'float';
399+
} elseif (\is_bool($value)) {
400+
return \var_export($value, true);
401+
} elseif ($value === null) {
402+
return 'null';
403+
}
404+
return \is_object($value) ? \get_class($value) : \gettype($value);
381405
}
382406
}

‎tests/AppTest.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1529,25 +1529,25 @@ public function provideInvalidClasses(): \Generator
15291529

15301530
yield [
15311531
InvalidConstructorUntyped::class,
1532-
'Argument 1 ($value) of %s::__construct() has no type'
1532+
'Argument #1 ($value) of %s::__construct() has no type'
15331533
];
15341534

15351535
yield [
15361536
InvalidConstructorInt::class,
1537-
'Argument 1 ($value) of %s::__construct() expects unsupported type int'
1537+
'Argument #1 ($value) of %s::__construct() expects unsupported type int'
15381538
];
15391539

15401540
if (PHP_VERSION_ID >= 80000) {
15411541
yield [
15421542
InvalidConstructorUnion::class,
1543-
'Argument 1 ($value) of %s::__construct() expects unsupported type int|float'
1543+
'Argument #1 ($value) of %s::__construct() expects unsupported type int|float'
15441544
];
15451545
}
15461546

15471547
if (PHP_VERSION_ID >= 80100) {
15481548
yield [
15491549
InvalidConstructorIntersection::class,
1550-
'Argument 1 ($value) of %s::__construct() expects unsupported type Traversable&ArrayAccess'
1550+
'Argument #1 ($value) of %s::__construct() expects unsupported type Traversable&ArrayAccess'
15511551
];
15521552
}
15531553

@@ -1558,7 +1558,7 @@ public function provideInvalidClasses(): \Generator
15581558

15591559
yield [
15601560
InvalidConstructorSelf::class,
1561-
'Argument 1 ($value) of %s::__construct() is recursive'
1561+
'Argument #1 ($value) of %s::__construct() is recursive'
15621562
];
15631563
}
15641564

‎tests/ContainerTest.php‎

Lines changed: 80 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1275,6 +1275,7 @@ public function __construct(\stdClass $data)
12751275
}
12761276
};
12771277

1278+
$line = __LINE__ + 2;
12781279
$container = new Container([
12791280
\stdClass::class => function (string $username) {
12801281
return (object) ['name' => $username];
@@ -1284,7 +1285,7 @@ public function __construct(\stdClass $data)
12841285
$callable = $container->callable(get_class($controller));
12851286

12861287
$this->expectException(\BadMethodCallException::class);
1287-
$this->expectExceptionMessageMatches('/Argument 1 \(\$username\) of {closure(:[^{}]+)?}\(\) is not defined$/');
1288+
$this->expectExceptionMessage('Argument #1 ($username) of {closure:' . __FILE__ . ':' . $line .'}() is not defined');
12881289
$callable($request);
12891290
}
12901291

@@ -1386,7 +1387,7 @@ public function __construct(\stdClass $data)
13861387
$callable = $container->callable(get_class($controller));
13871388

13881389
$this->expectException(\BadMethodCallException::class);
1389-
$this->expectExceptionMessage('Container variable $http expected type stdClass, but got integer');
1390+
$this->expectExceptionMessage('Container variable $http expected type stdClass, but got int');
13901391
$callable($request);
13911392
}
13921393

@@ -1411,7 +1412,7 @@ public function __construct(\stdClass $data)
14111412
$callable = $container->callable(get_class($controller));
14121413

14131414
$this->expectException(\BadMethodCallException::class);
1414-
$this->expectExceptionMessage('Container variable $http expected type string, but got integer');
1415+
$this->expectExceptionMessage('Container variable $http expected type string, but got int');
14151416
$callable($request);
14161417
}
14171418

@@ -1552,7 +1553,7 @@ public function __construct(\stdClass $data)
15521553
$callable = $container->callable(get_class($controller));
15531554

15541555
$this->expectException(\BadMethodCallException::class);
1555-
$this->expectExceptionMessage('Map for stdClass contains unexpected integer');
1556+
$this->expectExceptionMessage('Map for stdClass contains unexpected int');
15561557
$callable($request);
15571558
}
15581559

@@ -1574,7 +1575,7 @@ public function __construct(\stdClass $data)
15741575
$callable = $container->callable(get_class($controller));
15751576

15761577
$this->expectException(\BadMethodCallException::class);
1577-
$this->expectExceptionMessage('Map for stdClass contains unexpected NULL');
1578+
$this->expectExceptionMessage('Map for stdClass contains unexpected null');
15781579
$callable($request);
15791580
}
15801581

@@ -1596,7 +1597,7 @@ public function __construct(?\stdClass $data)
15961597
$callable = $container->callable(get_class($controller));
15971598

15981599
$this->expectException(\BadMethodCallException::class);
1599-
$this->expectExceptionMessage('Map for stdClass contains unexpected NULL');
1600+
$this->expectExceptionMessage('Map for stdClass contains unexpected null');
16001601
$callable($request);
16011602
}
16021603

@@ -1640,7 +1641,7 @@ public function __construct(string $name)
16401641
$callable = $container->callable(get_class($controller));
16411642

16421643
$this->expectException(\BadMethodCallException::class);
1643-
$this->expectExceptionMessage('Argument 1 ($name) of class@anonymous::__construct() expects unsupported type string');
1644+
$this->expectExceptionMessage('Argument #1 ($name) of class@anonymous::__construct() expects unsupported type string');
16441645
$callable($request);
16451646
}
16461647

@@ -1677,7 +1678,7 @@ public function testCtorThrowsWhenMapForClassContainsInvalidObject(): void
16771678
public function testCtorThrowsWhenMapForClassContainsInvalidNull(): void
16781679
{
16791680
$this->expectException(\BadMethodCallException::class);
1680-
$this->expectExceptionMessage('Map for Psr\Http\Message\ResponseInterface contains unexpected NULL');
1681+
$this->expectExceptionMessage('Map for Psr\Http\Message\ResponseInterface contains unexpected null');
16811682

16821683
new Container([
16831684
ResponseInterface::class => null
@@ -1710,7 +1711,7 @@ public function testCallableReturnsCallableThatThrowsWhenFactoryReturnsInvalidIn
17101711
$callable = $container->callable(\stdClass::class);
17111712

17121713
$this->expectException(\BadMethodCallException::class);
1713-
$this->expectExceptionMessage('Factory for stdClass returned unexpected integer');
1714+
$this->expectExceptionMessage('Factory for stdClass returned unexpected int');
17141715
$callable($request);
17151716
}
17161717

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

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

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

17721774
$this->expectException(\BadMethodCallException::class);
1773-
$this->expectExceptionMessageMatches('/Argument 1 \(\$undefined\) of {closure(:[^{}]+)?}\(\) has no type$/');
1775+
$this->expectExceptionMessage('Argument #1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() has no type');
17741776
$callable($request);
17751777
}
17761778

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

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

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

17901793
$this->expectException(\BadMethodCallException::class);
1791-
$this->expectExceptionMessageMatches('/Argument 1 \(\$undefined\) of {closure(:[^{}]+)?}\(\) is not defined$/');
1794+
$this->expectExceptionMessage('Argument #1 ($undefined) of {closure:' . __FILE__ . ':' . $line .'}() is not defined');
17921795
$callable($request);
17931796
}
17941797

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

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

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

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

@@ -2083,7 +2087,39 @@ public function testGetEnvThrowsIfMapContainsInvalidType(): void
20832087
]);
20842088

20852089
$this->expectException(\TypeError::class);
2086-
$this->expectExceptionMessage('Environment variable $X_FOO expected type string|null, but got boolean');
2090+
$this->expectExceptionMessage('Environment variable $X_FOO expected type string|null, but got false');
2091+
$container->getEnv('X_FOO');
2092+
}
2093+
2094+
/**
2095+
* @requires PHP 8
2096+
*/
2097+
public function testGetEnvThrowsWhenFactoryUsesBuiltInFunctionThatReferencesUnknownVariable(): void
2098+
{
2099+
$container = new Container([
2100+
'X_FOO' => \Closure::fromCallable('extension_loaded')
2101+
]);
2102+
2103+
$this->expectException(\BadMethodCallException::class);
2104+
$this->expectExceptionMessage('Argument #1 ($extension) of extension_loaded() is not defined');
2105+
$container->getEnv('X_FOO');
2106+
}
2107+
2108+
public function testGetEnvThrowsWhenFactoryUsesClassMethodThatReferencesUnknownVariable(): void
2109+
{
2110+
$class = new class {
2111+
public function foo(string $bar): string
2112+
{
2113+
return $bar;
2114+
}
2115+
};
2116+
2117+
$container = new Container([
2118+
'X_FOO' => \Closure::fromCallable([$class, 'foo'])
2119+
]);
2120+
2121+
$this->expectException(\BadMethodCallException::class);
2122+
$this->expectExceptionMessage('Argument #1 ($bar) of class@anonymous::foo() is not defined');
20872123
$container->getEnv('X_FOO');
20882124
}
20892125

@@ -2097,7 +2133,7 @@ public function testGetEnvThrowsIfMapPsrContainerReturnsInvalidType(): void
20972133
$container = new Container($psr);
20982134

20992135
$this->expectException(\TypeError::class);
2100-
$this->expectExceptionMessage('Environment variable $X_FOO expected type string|null, but got integer');
2136+
$this->expectExceptionMessage('Environment variable $X_FOO expected type string|null, but got int');
21012137
$container->getEnv('X_FOO');
21022138
}
21032139

@@ -2227,22 +2263,39 @@ public function testInvokeContainerAsFinalRequestHandlerThrows(): void
22272263
$container($request);
22282264
}
22292265

2230-
public function testCtorWithInvalidValueThrows(): void
2266+
public static function provideInvalidContainerConfigValues(): \Generator
22312267
{
2232-
$this->expectException(\TypeError::class);
2233-
$this->expectExceptionMessage('Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, stdClass given');
2234-
new Container((object) []); // @phpstan-ignore-line
2268+
yield [
2269+
(object) [],
2270+
\stdClass::class
2271+
];
2272+
yield [
2273+
new Container([]),
2274+
Container::class
2275+
];
2276+
yield [
2277+
true,
2278+
'true'
2279+
];
2280+
yield [
2281+
false,
2282+
'false'
2283+
];
2284+
yield [
2285+
1.0,
2286+
'float'
2287+
];
22352288
}
22362289

2237-
public function expectExceptionMessageMatches(string $regularExpression): void
2290+
/**
2291+
* @dataProvider provideInvalidContainerConfigValues
2292+
* @param mixed $value
2293+
* @param string $type
2294+
*/
2295+
public function testCtorWithInvalidValueThrows($value, string $type): void
22382296
{
2239-
if (method_exists(parent::class, 'expectExceptionMessageMatches')) {
2240-
// @phpstan-ignore-next-line PHPUnit 8.4+
2241-
parent::expectExceptionMessageMatches($regularExpression);
2242-
} else {
2243-
// legacy PHPUnit
2244-
assert(method_exists($this, 'expectExceptionMessageRegExp'));
2245-
$this->expectExceptionMessageRegExp($regularExpression);
2246-
}
2297+
$this->expectException(\TypeError::class);
2298+
$this->expectExceptionMessage('Argument #1 ($loader) must be of type array|Psr\Container\ContainerInterface, ' . $type . ' given');
2299+
new Container($value); // @phpstan-ignore-line
22472300
}
22482301
}

0 commit comments

Comments
 (0)