feature #22060 [DI] Add "by-id" autowiring: a side-effect free variant of it based on the class<>id convention (nicolas-grekas)

This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Add "by-id" autowiring: a side-effect free variant of it based on the class<>id convention | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This PR adds a new autowiring mode, based only on the class <> id convention. This way of autowiring is free from any conflicting behavior, which is what I was looking for to begin with. The expected DX is a bit more involving than the current way we do autowiring. But it's worth it to me, because it's plain predictable - a lot less "magic" imho. So in this mode, for each `App\Foo` type hint, a reference to an "App\Foo" service will be created. If no such service exists, an exception will be thrown. To me, this opens a nice DX: when type hinting interfaces (which is the best practice), this will tell you when you need to create the explicit interface <> id mapping that is missing - thus encourage things to be made explicit, but only when required, and gradually, in a way that will favor discoverability by devs. Of course, this is opt-in, and BC. You'd need to do eg in yaml: `autowire: by_id`. For consistency, the current mode (`autowire: true`) can be configured using `autowire: by_type`. Commits ------- c298f2a90c [DI] Add "by-id" autowiring: a side-effect free variant of it based on the class<>id convention
2017-03-25 13:18:59 -07:00 · 2017-03-25 13:18:59 -07:00 · 6cc9dc7586
commit 6cc9dc7586
parent 9896f86743 c298f2a90c
18 changed files with 135 additions and 25 deletions
--- a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php
@ -221,7 +221,7 @@ class JsonDescriptor extends Descriptor
            'lazy' => $definition->isLazy(),
            'shared' => $definition->isShared(),
            'abstract' => $definition->isAbstract(),
-            'autowire' => $definition->isAutowired(),
+            'autowire' => $definition->isAutowired() ? (Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'by-type' : 'by-id') : false,
        );

        foreach ($definition->getAutowiringTypes(false) as $autowiringType) {
--- a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/MarkdownDescriptor.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/MarkdownDescriptor.php
@ -182,7 +182,7 @@ class MarkdownDescriptor extends Descriptor
            ."\n".'- Lazy: '.($definition->isLazy() ? 'yes' : 'no')
            ."\n".'- Shared: '.($definition->isShared() ? 'yes' : 'no')
            ."\n".'- Abstract: '.($definition->isAbstract() ? 'yes' : 'no')
-            ."\n".'- Autowired: '.($definition->isAutowired() ? 'yes' : 'no')
+            ."\n".'- Autowired: '.($definition->isAutowired() ? (Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'by-type' : 'by-id') : 'no')
        ;

        foreach ($definition->getAutowiringTypes(false) as $autowiringType) {
--- a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php
@ -295,7 +295,7 @@ class TextDescriptor extends Descriptor
        $tableRows[] = array('Lazy', $definition->isLazy() ? 'yes' : 'no');
        $tableRows[] = array('Shared', $definition->isShared() ? 'yes' : 'no');
        $tableRows[] = array('Abstract', $definition->isAbstract() ? 'yes' : 'no');
-        $tableRows[] = array('Autowired', $definition->isAutowired() ? 'yes' : 'no');
+        $tableRows[] = array('Autowired', $definition->isAutowired() ? (Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'by-type' : 'by-id') : 'no');

        if ($autowiringTypes = $definition->getAutowiringTypes(false)) {
            $tableRows[] = array('Autowiring Types', implode(', ', $autowiringTypes));
--- a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php
@ -371,7 +371,7 @@ class XmlDescriptor extends Descriptor
        $serviceXML->setAttribute('lazy', $definition->isLazy() ? 'true' : 'false');
        $serviceXML->setAttribute('shared', $definition->isShared() ? 'true' : 'false');
        $serviceXML->setAttribute('abstract', $definition->isAbstract() ? 'true' : 'false');
-        $serviceXML->setAttribute('autowired', $definition->isAutowired() ? 'true' : 'false');
+        $serviceXML->setAttribute('autowired', $definition->isAutowired() ? (Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'by-type' : 'by-id') : 'false');
        $serviceXML->setAttribute('file', $definition->getFile());

        $calls = $definition->getMethodCalls();
--- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
+++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
@ -312,6 +312,10 @@ class AutowirePass extends AbstractRecursivePass
            return new Reference($type);
        }

+        if (Definition::AUTOWIRE_BY_ID === $this->currentDefinition->getAutowired()) {
+            return;
+        }
+
        if (null === $this->types) {
            $this->populateAvailableTypes();
        }
@ -462,10 +466,18 @@ class AutowirePass extends AbstractRecursivePass

    private function createTypeNotFoundMessage($type, $label)
    {
-        if (!$classOrInterface = class_exists($type, false) ? 'class' : (interface_exists($type, false) ? 'interface' : null)) {
+        $autowireById = Definition::AUTOWIRE_BY_ID === $this->currentDefinition->getAutowired();
+        if (!$classOrInterface = class_exists($type, $autowireById) ? 'class' : (interface_exists($type, false) ? 'interface' : null)) {
            return sprintf('Cannot autowire service "%s": %s has type "%s" but this class does not exist.', $this->currentId, $label, $type);
        }
-        $message = sprintf('no services were found matching the "%s" %s and it cannot be auto-registered for %s.', $type, $classOrInterface, $label);
+        if (null === $this->types) {
+            $this->populateAvailableTypes();
+        }
+        if ($autowireById) {
+            $message = sprintf('%s references %s "%s" but no such service exists.%s', $label, $classOrInterface, $type, $this->createTypeAlternatives($type));
+        } else {
+            $message = sprintf('no services were found matching the "%s" %s and it cannot be auto-registered for %s.', $type, $classOrInterface, $label);
+        }

        return sprintf('Cannot autowire service "%s": %s', $this->currentId, $message);
    }
--- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php
+++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php
@ -100,7 +100,7 @@ class ResolveDefinitionTemplatesPass extends AbstractRecursivePass
        $def->setFile($parentDef->getFile());
        $def->setPublic($parentDef->isPublic());
        $def->setLazy($parentDef->isLazy());
-        $def->setAutowired($parentDef->isAutowired());
+        $def->setAutowired($parentDef->getAutowired());

        self::mergeDefinition($def, $definition);

@ -146,7 +146,7 @@ class ResolveDefinitionTemplatesPass extends AbstractRecursivePass
            $def->setDeprecated($definition->isDeprecated(), $definition->getDeprecationMessage('%service_id%'));
        }
        if (isset($changes['autowired'])) {
-            $def->setAutowired($definition->isAutowired());
+            $def->setAutowired($definition->getAutowired());
        }
        if (isset($changes['decorated_service'])) {
            $decoratedService = $definition->getDecoratedService();
--- a/src/Symfony/Component/DependencyInjection/Definition.php
+++ b/src/Symfony/Component/DependencyInjection/Definition.php
@ -21,6 +21,9 @@ use Symfony\Component\DependencyInjection\Exception\OutOfBoundsException;
 */
 class Definition
 {
+    const AUTOWIRE_BY_TYPE = 1;
+    const AUTOWIRE_BY_ID = 2;
+
    private $class;
    private $file;
    private $factory;
@ -37,7 +40,7 @@ class Definition
    private $abstract = false;
    private $lazy = false;
    private $decoratedService;
-    private $autowired = false;
+    private $autowired = 0;
    private $autowiringTypes = array();

    protected $arguments;
@ -700,6 +703,16 @@ class Definition
     * @return bool
     */
    public function isAutowired()
+    {
+        return (bool) $this->autowired;
+    }
+
+    /**
+     * Gets the autowiring mode.
+     *
+     * @return int
+     */
+    public function getAutowired()
    {
        return $this->autowired;
    }
@ -707,13 +720,18 @@ class Definition
    /**
     * Sets autowired.
     *
-     * @param bool $autowired
+     * @param bool|int $autowired
     *
     * @return $this
     */
    public function setAutowired($autowired)
    {
-        $this->autowired = (bool) $autowired;
+        $autowired = (int) $autowired;
+
+        if ($autowired && self::AUTOWIRE_BY_TYPE !== $autowired && self::AUTOWIRE_BY_ID !== $autowired) {
+            throw new InvalidArgumentException(sprintf('Invalid argument: Definition::AUTOWIRE_BY_TYPE (%d) or Definition::AUTOWIRE_BY_ID (%d) expected, %d given.', self::AUTOWIRE_BY_TYPE, self::AUTOWIRE_BY_ID, $autowired));
+        }
+        $this->autowired = $autowired;

        return $this;
    }
--- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
+++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@ -646,10 +646,11 @@ EOF;
        }

        if ($definition->isAutowired()) {
+            $autowired = Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'types' : 'ids';
            $doc .= <<<EOF

     *
-     * This service is autowired.
+     * This service is autowired by {$autowired}.
 EOF;
        }

--- a/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php
+++ b/src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php
@ -195,7 +195,7 @@ class XmlDumper extends Dumper
        }

        if ($definition->isAutowired()) {
-            $service->setAttribute('autowire', 'true');
+            $service->setAttribute('autowire', Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'by-type' : 'by-id');
        }

        foreach ($definition->getAutowiringTypes(false) as $autowiringTypeValue) {
--- a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php
+++ b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php
@ -106,7 +106,7 @@ class YamlDumper extends Dumper
        }

        if ($definition->isAutowired()) {
-            $code .= "        autowire: true\n";
+            $code .= sprintf("        autowire: %s\n", Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'by_type' : 'by_id');
        }

        $autowiringTypesCode = '';
--- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
+++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
@ -172,7 +172,7 @@ class XmlFileLoader extends FileLoader
            }
        }
        if ($defaultsNode->hasAttribute('autowire')) {
-            $defaults['autowire'] = XmlUtils::phpize($defaultsNode->getAttribute('autowire'));
+            $defaults['autowire'] = $this->getAutowired($defaultsNode->getAttribute('autowire'), $file);
        }
        if ($defaultsNode->hasAttribute('public')) {
            $defaults['public'] = XmlUtils::phpize($defaultsNode->getAttribute('public'));
@ -238,7 +238,7 @@ class XmlFileLoader extends FileLoader
        }

        if ($value = $service->getAttribute('autowire')) {
-            $definition->setAutowired(XmlUtils::phpize($value));
+            $definition->setAutowired($this->getAutowired($value, $file));
        } elseif (isset($defaults['autowire'])) {
            $definition->setAutowired($defaults['autowire']);
        }
@ -665,6 +665,23 @@ EOF
        }
    }

+    private function getAutowired($value, $file)
+    {
+        if (is_bool($value = XmlUtils::phpize($value))) {
+            return $value;
+        }
+
+        if ('by-type' === $value) {
+            return Definition::AUTOWIRE_BY_TYPE;
+        }
+
+        if ('by-id' === $value) {
+            return Definition::AUTOWIRE_BY_ID;
+        }
+
+        throw new InvalidArgumentException(sprintf('Invalid autowire attribute: "by-type", "by-id", "true" or "false" expected, "%s" given in "%s".', $value, $file));
+    }
+
    /**
     * Converts a \DomElement object to a PHP array.
     *
--- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
+++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
@ -493,6 +493,14 @@ class YamlFileLoader extends FileLoader

        $autowire = isset($service['autowire']) ? $service['autowire'] : (isset($defaults['autowire']) ? $defaults['autowire'] : null);
        if (null !== $autowire) {
+            if ('by_type' === $autowire) {
+                $autowire = Definition::AUTOWIRE_BY_TYPE;
+            } elseif ('by_id' === $autowire) {
+                $autowire = Definition::AUTOWIRE_BY_ID;
+            } elseif (!is_bool($autowire)) {
+                throw new InvalidArgumentException(sprintf('Invalid autowire attribute: "by_type", "by_id", true or false expected, "%s" given in "%s".', is_string($autowire) ? $autowire : gettype($autowire), $file));
+            }
+
            $definition->setAutowired($autowire);
        }

--- a/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd
+++ b/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd
@ -102,7 +102,7 @@
      <xsd:element name="tag" type="tag" minOccurs="0" maxOccurs="unbounded" />
    </xsd:choice>
    <xsd:attribute name="public" type="boolean" />
-    <xsd:attribute name="autowire" type="boolean" />
+    <xsd:attribute name="autowire" type="autowire" />
    <xsd:attribute name="inherit-tags" type="boolean" />
  </xsd:complexType>

@ -130,7 +130,7 @@
    <xsd:attribute name="decorates" type="xsd:string" />
    <xsd:attribute name="decoration-inner-name" type="xsd:string" />
    <xsd:attribute name="decoration-priority" type="xsd:integer" />
-    <xsd:attribute name="autowire" type="boolean" />
+    <xsd:attribute name="autowire" type="autowire" />
    <xsd:attribute name="inherit-tags" type="boolean" />
  </xsd:complexType>

@ -149,7 +149,7 @@
    <xsd:attribute name="public" type="boolean" />
    <xsd:attribute name="lazy" type="boolean" />
    <xsd:attribute name="abstract" type="boolean" />
-    <xsd:attribute name="autowire" type="boolean" />
+    <xsd:attribute name="autowire" type="autowire" />
  </xsd:complexType>

  <xsd:complexType name="prototype">
@ -169,7 +169,7 @@
    <xsd:attribute name="lazy" type="boolean" />
    <xsd:attribute name="abstract" type="boolean" />
    <xsd:attribute name="parent" type="xsd:string" />
-    <xsd:attribute name="autowire" type="boolean" />
+    <xsd:attribute name="autowire" type="autowire" />
    <xsd:attribute name="inherit-tags" type="boolean" />
  </xsd:complexType>

@ -264,4 +264,10 @@
      <xsd:pattern value="(%.+%|true|false)" />
    </xsd:restriction>
  </xsd:simpleType>
+
+  <xsd:simpleType name="autowire">
+    <xsd:restriction base="xsd:string">
+      <xsd:pattern value="(true|false|by-type|by-id)" />
+    </xsd:restriction>
+  </xsd:simpleType>
 </xsd:schema>
--- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
@ -14,6 +14,7 @@ namespace Symfony\Component\DependencyInjection\Tests\Compiler;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\DependencyInjection\Compiler\AutowirePass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Exception\RuntimeException;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\includes\FooVariadic;
@ -709,6 +710,53 @@ class AutowirePassTest extends TestCase
        $pass = new AutowirePass();
        $pass->process($container);
    }
+
+    public function testById()
+    {
+        $container = new ContainerBuilder();
+
+        $container->register(A::class, A::class);
+        $container->register(DInterface::class, F::class);
+        $container->register('d', D::class)
+            ->setAutowired(Definition::AUTOWIRE_BY_ID);
+
+        $pass = new AutowirePass();
+        $pass->process($container);
+
+        $this->assertSame(array('service_container', A::class, DInterface::class, 'd'), array_keys($container->getDefinitions()));
+        $this->assertEquals(array(new Reference(A::class), new Reference(DInterface::class)), $container->getDefinition('d')->getArguments());
+    }
+
+    public function testByIdDoesNotAutoregister()
+    {
+        $container = new ContainerBuilder();
+
+        $container->register('f', F::class);
+        $container->register('e', E::class)
+            ->setAutowired(Definition::AUTOWIRE_BY_ID);
+
+        $pass = new AutowirePass();
+        $pass->process($container);
+
+        $this->assertSame(array('service_container', 'f', 'e'), array_keys($container->getDefinitions()));
+    }
+
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
+     * @expectedExceptionMessage Cannot autowire service "j": argument $i of method Symfony\Component\DependencyInjection\Tests\Compiler\J::__construct() references class "Symfony\Component\DependencyInjection\Tests\Compiler\I" but no such service exists. This type-hint could be aliased to the existing "i" service; or be updated to "Symfony\Component\DependencyInjection\Tests\Compiler\IInterface".
+     */
+    public function testByIdAlternative()
+    {
+        $container = new ContainerBuilder();
+
+        $container->setAlias(IInterface::class, 'i');
+        $container->register('i', I::class);
+        $container->register('j', J::class)
+            ->setAutowired(Definition::AUTOWIRE_BY_ID);
+
+        $pass = new AutowirePass();
+        $pass->process($container);
+    }
 }

 class Foo
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services24.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services24.php
@ -70,7 +70,7 @@ class ProjectServiceContainer extends Container
     * This service is shared.
     * This method always returns the same instance of the service.
     *
-     * This service is autowired.
+     * This service is autowired by types.
     *
     * @return \Foo A Foo instance
     */
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php
@ -91,7 +91,7 @@ class ProjectServiceContainer extends Container
     * This service is shared.
     * This method always returns the same instance of the service.
     *
-     * This service is autowired.
+     * This service is autowired by types.
     *
     * @return \TestServiceSubscriber A TestServiceSubscriber instance
     */
@ -118,7 +118,7 @@ class ProjectServiceContainer extends Container
     * If you want to be able to request this service from the container directly,
     * make it public, otherwise you might end up with broken code.
     *
-     * This service is autowired.
+     * This service is autowired by types.
     *
     * @return \stdClass A stdClass instance
     */
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services24.xml
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services24.xml
@ -2,7 +2,7 @@
 <container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
  <services>
    <service id="service_container" class="Symfony\Component\DependencyInjection\ContainerInterface" synthetic="true"/>
-    <service id="foo" class="Foo" autowire="true"/>
+    <service id="foo" class="Foo" autowire="by-type"/>
    <service id="Psr\Container\ContainerInterface" alias="service_container" public="false"/>
    <service id="Symfony\Component\DependencyInjection\ContainerInterface" alias="service_container" public="false"/>
  </services>
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services24.yml
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services24.yml
@ -5,7 +5,7 @@ services:
        synthetic: true
    foo:
        class: Foo
-        autowire: true
+        autowire: by_type
    Psr\Container\ContainerInterface:
        alias: service_container
        public: false