From 7c01c4c80c69159b2b39ea8bc53431196d7b29fb Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 13 Aug 2019 14:49:41 +0200 Subject: [PATCH] [DI] deprecate support for non-object services --- .../DependencyInjection/Container.php | 14 ++++++++++---- .../DependencyInjection/ContainerBuilder.php | 18 ++++++++++++++---- .../Tests/ContainerBuilderTest.php | 19 ++++++++++--------- .../Tests/ContainerTest.php | 3 +++ .../Tests/Dumper/PhpDumperTest.php | 9 +++++---- ...sterControllerArgumentLocatorsPassTest.php | 18 ++++++------------ 6 files changed, 48 insertions(+), 33 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Container.php b/src/Symfony/Component/DependencyInjection/Container.php index 740150bf2a..c46f4f2e8c 100644 --- a/src/Symfony/Component/DependencyInjection/Container.php +++ b/src/Symfony/Component/DependencyInjection/Container.php @@ -141,8 +141,8 @@ class Container implements ResettableContainerInterface * Setting a synthetic service to null resets it: has() returns false and get() * behaves in the same way as if the service was never created. * - * @param string $id The service identifier - * @param object $service The service instance + * @param string $id The service identifier + * @param object|null $service The service instance */ public function set($id, $service) { @@ -210,7 +210,7 @@ class Container implements ResettableContainerInterface * @param string $id The service identifier * @param int $invalidBehavior The behavior when the service does not exist * - * @return object The associated service + * @return object|null The associated service * * @throws ServiceCircularReferenceException When a circular reference is detected * @throws ServiceNotFoundException When the service is not defined @@ -220,9 +220,15 @@ class Container implements ResettableContainerInterface */ public function get($id, $invalidBehavior = /* self::EXCEPTION_ON_INVALID_REFERENCE */ 1) { - return $this->services[$id] + $service = $this->services[$id] ?? $this->services[$id = $this->aliases[$id] ?? $id] ?? ('service_container' === $id ? $this : ($this->factories[$id] ?? [$this, 'make'])($id, $invalidBehavior)); + + if (!\is_object($service) && null !== $service) { + @trigger_error(sprintf('Non-object services are deprecated since Symfony 4.4, please fix the "%s" service which is of type "%s" right now.', $id, \gettype($service)), E_USER_DEPRECATED); + } + + return $service; } /** diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 2523362450..f07a141851 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -487,13 +487,17 @@ class ContainerBuilder extends Container implements TaggedContainerInterface /** * Sets a service. * - * @param string $id The service identifier - * @param object $service The service instance + * @param string $id The service identifier + * @param object|null $service The service instance * * @throws BadMethodCallException When this ContainerBuilder is compiled */ public function set($id, $service) { + if (!\is_object($service) && null !== $service) { + @trigger_error(sprintf('Non-object services are deprecated since Symfony 4.4, setting the "%s" service to a value of type "%s" should be avoided.', $id, \gettype($service)), E_USER_DEPRECATED); + } + $id = (string) $id; if ($this->isCompiled() && (isset($this->definitions[$id]) && !$this->definitions[$id]->isSynthetic())) { @@ -539,7 +543,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface * @param string $id The service identifier * @param int $invalidBehavior The behavior when the service does not exist * - * @return object The associated service + * @return object|null The associated service * * @throws InvalidArgumentException when no definitions are available * @throws ServiceCircularReferenceException When a circular reference is detected @@ -554,7 +558,13 @@ class ContainerBuilder extends Container implements TaggedContainerInterface return parent::get($id); } - return $this->doGet($id, $invalidBehavior); + $service = $this->doGet($id, $invalidBehavior); + + if (!\is_object($service) && null !== $service) { + @trigger_error(sprintf('Non-object services are deprecated since Symfony 4.4, please fix the "%s" service which is of type "%s" right now.', $id, \gettype($service)), E_USER_DEPRECATED); + } + + return $service; } private function doGet(string $id, int $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, array &$inlineServices = null, bool $isConstructorArgument = false) diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php index 3fede26b80..632dff8e98 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php @@ -322,8 +322,8 @@ class ContainerBuilderTest extends TestCase $builder->register('bar', 'stdClass'); $this->assertFalse($builder->hasAlias('bar')); - $builder->set('foobar', 'stdClass'); - $builder->set('moo', 'stdClass'); + $builder->set('foobar', new \stdClass()); + $builder->set('moo', new \stdClass()); $this->assertCount(2, $builder->getAliases(), '->getAliases() does not return aliased services that have been overridden'); } @@ -1573,16 +1573,17 @@ class ContainerBuilderTest extends TestCase public function testScalarService() { - $c = new ContainerBuilder(); - $c->register('foo', 'string') - ->setPublic(true) + $container = new ContainerBuilder(); + $container->register('foo', 'string') ->setFactory([ScalarFactory::class, 'getSomeValue']) ; + $container->register('bar', 'stdClass') + ->setProperty('foo', new Reference('foo')) + ->setPublic(true) + ; + $container->compile(); - $c->compile(); - - $this->assertTrue($c->has('foo')); - $this->assertSame('some value', $c->get('foo')); + $this->assertSame('some value', $container->get('bar')->foo); } public function testWither() diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php index fcbe600895..d9d2ff9eca 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php @@ -288,6 +288,9 @@ class ContainerTest extends TestCase $this->assertTrue($sc->has('foo.baz'), '->has() returns true if a get*Method() is defined'); } + /** + * @group legacy + */ public function testScalarService() { $c = new Container(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php index a54192551b..d6b20071be 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php @@ -1282,10 +1282,12 @@ class PhpDumperTest extends TestCase { $container = new ContainerBuilder(); $container->register('foo', 'string') - ->setPublic(true) ->setFactory([ScalarFactory::class, 'getSomeValue']) ; - + $container->register('bar', 'stdClass') + ->setProperty('foo', new Reference('foo')) + ->setPublic(true) + ; $container->compile(); $dumper = new PhpDumper($container); @@ -1293,8 +1295,7 @@ class PhpDumperTest extends TestCase $container = new \Symfony_DI_PhpDumper_Test_Scalar_Service(); - $this->assertTrue($container->has('foo')); - $this->assertSame('some value', $container->get('foo')); + $this->assertSame('some value', $container->get('bar')->foo); } public function testWither() diff --git a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php index 3c5b197837..021ababdbd 100644 --- a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php @@ -306,7 +306,7 @@ class RegisterControllerArgumentLocatorsPassTest extends TestCase public function testBindScalarValueToControllerArgument($bindingKey) { $container = new ContainerBuilder(); - $resolver = $container->register('argument_resolver.service')->addArgument([]); + $resolver = $container->register('argument_resolver.service', 'stdClass')->addArgument([]); $container->register('foo', ArgumentWithoutTypeController::class) ->setBindings([$bindingKey => '%foo%']) @@ -317,19 +317,13 @@ class RegisterControllerArgumentLocatorsPassTest extends TestCase $pass = new RegisterControllerArgumentLocatorsPass(); $pass->process($container); - $locator = $container->getDefinition((string) $resolver->getArgument(0))->getArgument(0); + $locatorId = (string) $resolver->getArgument(0); + $container->getDefinition($locatorId)->setPublic(true); - $locator = $container->getDefinition((string) $locator['foo::fooAction']->getValues()[0]); + $container->compile(); - // assert the locator has a someArg key - $arguments = $locator->getArgument(0); - $this->assertArrayHasKey('someArg', $arguments); - $this->assertInstanceOf(ServiceClosureArgument::class, $arguments['someArg']); - // get the Reference that someArg points to - $reference = $arguments['someArg']->getValues()[0]; - // make sure this service *does* exist and returns the correct value - $this->assertTrue($container->has((string) $reference)); - $this->assertSame('foo_val', $container->get((string) $reference)); + $locator = $container->get($locatorId); + $this->assertSame('foo_val', $locator->get('foo::fooAction')->get('someArg')); } public function provideBindScalarValueToControllerArgument()