feature #33137 [DI] deprecate support for non-object services (nicola…

…s-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[DI] deprecate support for non-object services

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32411
| License       | MIT
| Doc PR        | -

Follows #32432 /cc @derrabus
Prepares for adding the `?object` return-type on master.

Commits
-------

7c01c4c [DI] deprecate support for non-object services

src/Symfony/Component/DependencyInjection/Container.php

@@ -141,8 +141,8 @@ public function setParameter($name, $value)
      * 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 @@ public function has($id)
      * @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 @@ public function has($id)
      */
     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;
     }
 
     /**

src/Symfony/Component/DependencyInjection/ContainerBuilder.php

@@ -487,13 +487,17 @@ public function getCompiler()
     /**
      * 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 @@ public function has($id)
      * @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 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV
             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)

src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php

@@ -322,8 +322,8 @@ public function testGetAliases()
         $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 @@ public function testDecoratedSelfReferenceInvolvingPrivateServices()
 
     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()

src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php

@@ -288,6 +288,9 @@ public function testHas()
         $this->assertTrue($sc->has('foo.baz'), '->has() returns true if a get*Method() is defined');
     }
 
+    /**
+     * @group legacy
+     */
     public function testScalarService()
     {
         $c = new Container();

src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php

@@ -1282,19 +1282,20 @@ public function testScalarService()
     {
         $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);
         eval('?>'.$dumper->dump(['class' => 'Symfony_DI_PhpDumper_Test_Scalar_Service']));
 
         $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()

src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterControllerArgumentLocatorsPassTest.php

@@ -306,7 +306,7 @@ public function provideBindings()
     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 @@ public function testBindScalarValueToControllerArgument($bindingKey)
         $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()