From c298f2a90c9b6691df378fe819a4c54481c9f48c Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sun, 19 Mar 2017 12:22:02 +0100 Subject: [PATCH] [DI] Add "by-id" autowiring: a side-effect free variant of it based on the class<>id convention --- .../Console/Descriptor/JsonDescriptor.php | 2 +- .../Console/Descriptor/MarkdownDescriptor.php | 2 +- .../Console/Descriptor/TextDescriptor.php | 2 +- .../Console/Descriptor/XmlDescriptor.php | 2 +- .../Compiler/AutowirePass.php | 16 ++++++- .../ResolveDefinitionTemplatesPass.php | 4 +- .../DependencyInjection/Definition.php | 24 ++++++++-- .../DependencyInjection/Dumper/PhpDumper.php | 3 +- .../DependencyInjection/Dumper/XmlDumper.php | 2 +- .../DependencyInjection/Dumper/YamlDumper.php | 2 +- .../Loader/XmlFileLoader.php | 21 +++++++- .../Loader/YamlFileLoader.php | 8 ++++ .../schema/dic/services/services-1.0.xsd | 14 ++++-- .../Tests/Compiler/AutowirePassTest.php | 48 +++++++++++++++++++ .../Tests/Fixtures/php/services24.php | 2 +- .../Fixtures/php/services_subscriber.php | 4 +- .../Tests/Fixtures/xml/services24.xml | 2 +- .../Tests/Fixtures/yaml/services24.yml | 2 +- 18 files changed, 135 insertions(+), 25 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php index 81d7233811f1b..bee019a0badef 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php +++ b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php @@ -221,7 +221,7 @@ private function getContainerDefinitionData(Definition $definition, $omitTags = '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) { diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/MarkdownDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/MarkdownDescriptor.php index c0319aad6bb19..6b1d7a9921b87 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/MarkdownDescriptor.php +++ b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/MarkdownDescriptor.php @@ -182,7 +182,7 @@ protected function describeContainerDefinition(Definition $definition, array $op ."\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) { diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php index 2481b15375801..7b2e01f85171a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php +++ b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php @@ -295,7 +295,7 @@ protected function describeContainerDefinition(Definition $definition, array $op $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)); diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php index 40e749aa6a3e7..d930c4a64615c 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php +++ b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php @@ -371,7 +371,7 @@ private function getContainerDefinitionDocument(Definition $definition, $id = nu $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(); diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php index 6930d632de707..4d818adaa9dea 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php @@ -355,6 +355,10 @@ private function getAutowiredReference($type, $autoRegister = true) return new Reference($type); } + if (Definition::AUTOWIRE_BY_ID === $this->currentDefinition->getAutowired()) { + return; + } + if (null === $this->types) { $this->populateAvailableTypes(); } @@ -505,10 +509,18 @@ private function createAutowiredDefinition(\ReflectionClass $typeHint) 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); } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php index 7cccb96b17232..92fbdf8f1b3a1 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php @@ -101,7 +101,7 @@ private function doResolveDefinition(ChildDefinition $definition) $def->setFile($parentDef->getFile()); $def->setPublic($parentDef->isPublic()); $def->setLazy($parentDef->isLazy()); - $def->setAutowired($parentDef->isAutowired()); + $def->setAutowired($parentDef->getAutowired()); self::mergeDefinition($def, $definition); @@ -147,7 +147,7 @@ public static function mergeDefinition(Definition $def, ChildDefinition $definit $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(); diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php index 651f3339eb284..09251bbcba393 100644 --- a/src/Symfony/Component/DependencyInjection/Definition.php +++ b/src/Symfony/Component/DependencyInjection/Definition.php @@ -21,6 +21,9 @@ */ class Definition { + const AUTOWIRE_BY_TYPE = 1; + const AUTOWIRE_BY_ID = 2; + private $class; private $file; private $factory; @@ -38,7 +41,7 @@ class Definition private $abstract = false; private $lazy = false; private $decoratedService; - private $autowired = false; + private $autowired = 0; private $autowiringTypes = array(); protected $arguments; @@ -736,6 +739,16 @@ public function setAutowiringTypes(array $types) * @return bool */ public function isAutowired() + { + return (bool) $this->autowired; + } + + /** + * Gets the autowiring mode. + * + * @return int + */ + public function getAutowired() { return $this->autowired; } @@ -743,13 +756,18 @@ public function isAutowired() /** * 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; } diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index d4721ccaaef2b..b9cea03cc3970 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -734,10 +734,11 @@ private function addService($id, Definition $definition) } if ($definition->isAutowired()) { + $autowired = Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'types' : 'ids'; $doc .= <<isAutowired()) { - $service->setAttribute('autowire', 'true'); + $service->setAttribute('autowire', Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'by-type' : 'by-id'); } foreach ($definition->getAutowiringTypes(false) as $autowiringTypeValue) { diff --git a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php index ee0bd3d619218..2fce474b42ed9 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php @@ -106,7 +106,7 @@ private function addService($id, $definition) } if ($definition->isAutowired()) { - $code .= " autowire: true\n"; + $code .= sprintf(" autowire: %s\n", Definition::AUTOWIRE_BY_TYPE === $definition->getAutowired() ? 'by_type' : 'by_id'); } $autowiringTypesCode = ''; diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index 171cc9a4664e5..b87f30096b9b9 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -172,7 +172,7 @@ private function getServiceDefaults(\DOMDocument $xml, $file) } } 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 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults = } if ($value = $service->getAttribute('autowire')) { - $definition->setAutowired(XmlUtils::phpize($value)); + $definition->setAutowired($this->getAutowired($value, $file)); } elseif (isset($defaults['autowire'])) { $definition->setAutowired($defaults['autowire']); } @@ -666,6 +666,23 @@ private function loadFromExtensions(\DOMDocument $xml) } } + 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. * diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 95f409b42e135..47d65ce1d7988 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -500,6 +500,14 @@ private function parseDefinition($id, $service, $file, array $defaults) $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); } diff --git 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 index ce3b04b57ad4d..8619c6ee6eb57 100644 --- 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 @@ - + @@ -131,7 +131,7 @@ - + @@ -151,7 +151,7 @@ - + @@ -172,7 +172,7 @@ - + @@ -279,4 +279,10 @@ + + + + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index e3740c48b4854..cf748702e295c 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -14,6 +14,7 @@ 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; @@ -755,6 +756,53 @@ public function testAlternatives() $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 diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services24.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services24.php index be8cb0678c19a..8deafa468c0d3 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services24.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services24.php @@ -70,7 +70,7 @@ public function isFrozen() * 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 */ diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php index 1c4e389c8b4cb..1e58eeb9e0d21 100644 --- 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 @@ protected function getTestServiceSubscriberService() * 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 @@ protected function getFooServiceService() * 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 */ diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services24.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services24.xml index c4e32cb634e0c..c0815cd51229d 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services24.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services24.xml @@ -2,7 +2,7 @@ - + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services24.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services24.yml index afed157017f4d..977e30ffcca23 100644 --- 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