diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e50cefe..7ff0001c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 1.2.1 under development +- Chg #316: Fix exception messages (@xepozz) - Bug #317: Fix delegated container (@xepozz) ## 1.2.0 November 05, 2022 diff --git a/src/BuildingException.php b/src/BuildingException.php new file mode 100644 index 00000000..d838fdab --- /dev/null +++ b/src/BuildingException.php @@ -0,0 +1,35 @@ +getMessage() === '' ? $error::class : $error->getMessage(), + implode('" -> "', $buildStack === [] ? [$id] : $buildStack) + ); + + parent::__construct($message, 0, $previous); + } +} diff --git a/src/Container.php b/src/Container.php index 984f0e6c..43540927 100644 --- a/src/Container.php +++ b/src/Container.php @@ -5,13 +5,16 @@ namespace Yiisoft\Di; use Closure; +use Psr\Container\ContainerExceptionInterface; use Psr\Container\ContainerInterface; +use Psr\Container\NotFoundExceptionInterface; +use Throwable; use Yiisoft\Definitions\ArrayDefinition; +use Yiisoft\Definitions\DefinitionStorage; use Yiisoft\Definitions\Exception\CircularReferenceException; use Yiisoft\Definitions\Exception\InvalidConfigException; use Yiisoft\Definitions\Exception\NotInstantiableException; use Yiisoft\Definitions\Helpers\DefinitionValidator; -use Yiisoft\Definitions\DefinitionStorage; use Yiisoft\Di\Helpers\DefinitionNormalizer; use Yiisoft\Di\Helpers\DefinitionParser; use Yiisoft\Di\Helpers\TagHelper; @@ -119,8 +122,9 @@ public function has(string $id): bool * * @throws CircularReferenceException * @throws InvalidConfigException - * @throws NotFoundException + * @throws NotFoundExceptionInterface * @throws NotInstantiableException + * @throws BuildingException * * @return mixed|object An instance of the requested interface. * @@ -132,14 +136,21 @@ public function get(string $id) { if (!array_key_exists($id, $this->instances)) { try { - $this->instances[$id] = $this->build($id); - } catch (NotFoundException $e) { - if (!$this->delegates->has($id)) { + try { + $this->instances[$id] = $this->build($id); + } catch (NotFoundExceptionInterface $e) { + if (!$this->delegates->has($id)) { + throw $e; + } + + /** @psalm-suppress MixedReturnStatement */ + return $this->delegates->get($id); + } + } catch (Throwable $e) { + if ($e instanceof ContainerExceptionInterface && !$e instanceof InvalidConfigException) { throw $e; } - - /** @psalm-suppress MixedReturnStatement */ - return $this->delegates->get($id); + throw new BuildingException($id, $e, $this->definitions->getBuildStack(), $e); } } @@ -295,7 +306,8 @@ private function validateDefinition(mixed $definition, ?string $id = null): void $definition = array_merge( $class === null ? [] : [ArrayDefinition::CLASS_NAME => $class], [ArrayDefinition::CONSTRUCTOR => $constructorArguments], - $methodsAndProperties, + // extract only value from parsed definition method + array_map(fn (array $data): mixed => $data[2], $methodsAndProperties), ); } @@ -433,10 +445,10 @@ private function setDefinitionResetter(string $id, Closure $resetter): void /** * Add definition to storage. * - * @see $definitions - * * @param string $id ID to set definition for. * @param mixed|object $definition Definition to set. + * + * @see $definitions */ private function addDefinitionToStorage(string $id, $definition): void { @@ -452,9 +464,9 @@ private function addDefinitionToStorage(string $id, $definition): void * * @param string $id The interface or an alias name that was previously registered. * - * @throws CircularReferenceException * @throws InvalidConfigException - * @throws NotFoundException + * @throws NotFoundExceptionInterface + * @throws CircularReferenceException * * @return mixed|object New built instance of the specified class. * @@ -470,11 +482,13 @@ private function build(string $id) if ($id === ContainerInterface::class) { return $this; } - throw new CircularReferenceException(sprintf( - 'Circular reference to "%s" detected while building: %s.', - $id, - implode(', ', array_keys($this->building)) - )); + throw new CircularReferenceException( + sprintf( + 'Circular reference to "%s" detected while building: %s.', + $id, + implode(', ', array_keys($this->building)) + ) + ); } $this->building[$id] = 1; @@ -503,8 +517,8 @@ private function getTaggedServices(string $tagAlias): array } /** + * @throws NotFoundExceptionInterface * @throws InvalidConfigException - * @throws NotFoundException * * @return mixed|object */ diff --git a/tests/Unit/BuildingExceptionTest.php b/tests/Unit/BuildingExceptionTest.php new file mode 100644 index 00000000..5d3e8b27 --- /dev/null +++ b/tests/Unit/BuildingExceptionTest.php @@ -0,0 +1,40 @@ +assertSame('Caught unhandled error "i am angry" while building "test".', $exception->getMessage()); + } + + public function testEmptyMessage(): void + { + $exception = new BuildingException('test', new RuntimeException()); + + $this->assertSame('Caught unhandled error "RuntimeException" while building "test".', $exception->getMessage()); + } + + public function testBuildStack(): void + { + $exception = new BuildingException('test', new RuntimeException('i am angry'), ['a', 'b', 'test']); + + $this->assertSame('Caught unhandled error "i am angry" while building "a" -> "b" -> "test".', $exception->getMessage()); + } + + public function testCode(): void + { + $exception = new BuildingException('test', new RuntimeException()); + + $this->assertSame(0, $exception->getCode()); + } +} diff --git a/tests/Unit/CompositeContainerTest.php b/tests/Unit/CompositeContainerTest.php index 21f1948b..0d104d7e 100644 --- a/tests/Unit/CompositeContainerTest.php +++ b/tests/Unit/CompositeContainerTest.php @@ -11,6 +11,7 @@ use Yiisoft\Di\Container; use Yiisoft\Di\ContainerConfig; use Yiisoft\Di\Tests\Support\EngineMarkOne; +use Yiisoft\Di\Tests\Support\EngineMarkTwo; use Yiisoft\Di\Tests\Support\NonPsrContainer; final class CompositeContainerTest extends TestCase @@ -36,6 +37,10 @@ public function testTagsWithYiiAndNotYiiContainers(): void 'class' => EngineMarkOne::class, 'tags' => ['engine'], ], + EngineMarkTwo::class => [ + 'class' => EngineMarkTwo::class, + 'tags' => ['engine'], + ], ]); $firstContainer = new Container($config); @@ -47,8 +52,9 @@ public function testTagsWithYiiAndNotYiiContainers(): void $engines = $compositeContainer->get('tag@engine'); $this->assertIsArray($engines); - $this->assertCount(1, $engines); + $this->assertCount(2, $engines); $this->assertInstanceOf(EngineMarkOne::class, $engines[0]); + $this->assertInstanceOf(EngineMarkTwo::class, $engines[1]); } public function testNonPsrContainer(): void diff --git a/tests/Unit/ContainerTest.php b/tests/Unit/ContainerTest.php index 901f5b73..7637fbfb 100644 --- a/tests/Unit/ContainerTest.php +++ b/tests/Unit/ContainerTest.php @@ -10,6 +10,7 @@ use Psr\Container\NotFoundExceptionInterface; use RuntimeException; use stdClass; +use Yiisoft\Di\BuildingException; use Yiisoft\Di\CompositeContainer; use Yiisoft\Di\Container; use Yiisoft\Di\ContainerConfig; @@ -378,7 +379,10 @@ public function testMixedIndexedConstructorParametersAreNotAllowed(): void ]); $container = new Container($config); - $this->expectException(InvalidConfigException::class); + $this->expectException(BuildingException::class); + $this->expectExceptionMessage( + 'Caught unhandled error "Arguments indexed both by name and by position are not allowed in the same array." while building "test".' + ); $container->get('test'); } @@ -1608,7 +1612,10 @@ public function getExtensions(): array } }; - $this->expectException(RuntimeException::class); + $this->expectException(BuildingException::class); + $this->expectExceptionMessage( + 'Caught unhandled error "RuntimeException" while building "Yiisoft\Di\Tests\Support\B".' + ); $config = ContainerConfig::create() ->withDefinitions([ @@ -1894,8 +1901,8 @@ public function testNonArrayReset(): void ]); $this->expectException(InvalidConfigException::class); - $this->expectExceptionMessageMatches( - '/^Invalid definition: "reset" should be closure, (integer|int) given\.$/' + $this->expectExceptionMessage( + 'Invalid definition: "reset" should be closure, int given.' ); new Container($config); } @@ -1918,6 +1925,23 @@ public function testNonArrayTags(): void new Container($config); } + public function testNonArrayArguments(): void + { + $config = ContainerConfig::create() + ->withDefinitions([ + EngineMarkOne::class => [ + 'class' => EngineMarkOne::class, + 'setNumber()' => 42, + ], + ]); + + $this->expectException(InvalidConfigException::class); + $this->expectExceptionMessage( + 'Invalid definition: incorrect method "setNumber()" arguments. Expected array, got "int". Probably you should wrap them into square brackets.', + ); + $container = new Container($config); + } + public function dataInvalidTags(): array { return [ diff --git a/tests/Unit/NotFoundExceptionTest.php b/tests/Unit/NotFoundExceptionTest.php index 732b58d8..0cf7266b 100644 --- a/tests/Unit/NotFoundExceptionTest.php +++ b/tests/Unit/NotFoundExceptionTest.php @@ -32,4 +32,11 @@ public function testBuildStack(): void $exception->getMessage() ); } + + public function testCode(): void + { + $exception = new NotFoundException('test'); + + $this->assertSame(0, $exception->getCode()); + } }