From bec38900d8d3cc0e67a7e6517cde8e835850c3bd Mon Sep 17 00:00:00 2001 From: Daniel Iwaniec Date: Tue, 27 Aug 2019 00:47:30 +0200 Subject: [PATCH 1/2] [DI] scope singly-implemented interfaces detection by file --- .../Configurator/ContainerConfigurator.php | 2 + .../Configurator/ServicesConfigurator.php | 1 + .../DependencyInjection/Loader/FileLoader.php | 20 ++++---- .../Loader/PhpFileLoader.php | 12 +++++ .../Loader/XmlFileLoader.php | 4 ++ .../Loader/YamlFileLoader.php | 5 ++ .../Adapter/Adapter.php | 9 ++++ .../AnotherAdapter/Adapter.php | 9 ++++ .../Port/PortInterface.php | 7 +++ .../Tests/Fixtures/config/prototype.php | 2 +- .../Tests/Fixtures/config/prototype_array.php | 2 +- ...mented_interface_in_multiple_resources.xml | 16 +++++++ ...urces_with_previously_registered_alias.xml | 18 +++++++ ...rces_with_previously_registered_alias2.xml | 18 +++++++ .../Tests/Fixtures/xml/services_prototype.xml | 2 +- .../Fixtures/xml/services_prototype_array.xml | 1 + ...mented_interface_in_multiple_resources.xml | 14 ++++++ ...mented_interface_in_multiple_resources.yml | 12 +++++ ...urces_with_previously_registered_alias.yml | 15 ++++++ ...rces_with_previously_registered_alias2.yml | 15 ++++++ .../Fixtures/yaml/services_prototype.yml | 2 +- ...mented_interface_in_multiple_resources.yml | 9 ++++ .../Tests/Loader/XmlFileLoaderTest.php | 48 +++++++++++++++++++ .../Tests/Loader/YamlFileLoaderTest.php | 47 ++++++++++++++++++ 24 files changed, 278 insertions(+), 12 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/SinglyImplementedInterface/Adapter/Adapter.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/SinglyImplementedInterface/AnotherAdapter/Adapter.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/SinglyImplementedInterface/Port/PortInterface.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources.xml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias.xml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias2.xml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/singly_implemented_interface_in_multiple_resources.xml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources.yml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias.yml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias2.yml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/singly_implemented_interface_in_multiple_resources.yml diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php index 87beeaa392..3d39c3f864 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php +++ b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php @@ -73,6 +73,8 @@ class ContainerConfigurator extends AbstractConfigurator final public function services(): ServicesConfigurator { + $this->loader->resetBeforeConfiguringServices(); + return new ServicesConfigurator($this->container, $this->loader, $this->instanceof, $this->path, $this->anonymousCount); } } diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php index b693fc2d66..0688ef92b4 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php +++ b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php @@ -105,6 +105,7 @@ class ServicesConfigurator extends AbstractConfigurator $ref = static::processValue($referencedId, true); $alias = new Alias((string) $ref, $this->defaults->isPublic()); $this->container->setAlias($id, $alias); + $this->loader->removeSinglyImplementedAlias((string) $ref); return new AliasConfigurator($this, $alias); } diff --git a/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php index cb20416485..371d9862aa 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php @@ -29,6 +29,9 @@ abstract class FileLoader extends BaseFileLoader protected $container; protected $isLoadingInstanceof = false; protected $instanceof = []; + protected $interfaces = []; + protected $singlyImplemented = []; + protected $singlyImplementedAliases = []; public function __construct(ContainerBuilder $container, FileLocatorInterface $locator) { @@ -57,12 +60,10 @@ abstract class FileLoader extends BaseFileLoader $classes = $this->findClasses($namespace, $resource, (array) $exclude); // prepare for deep cloning $serializedPrototype = serialize($prototype); - $interfaces = []; - $singlyImplemented = []; foreach ($classes as $class => $errorMessage) { if (interface_exists($class, false)) { - $interfaces[] = $class; + $this->interfaces[] = $class; } else { $this->setDefinition($class, $definition = unserialize($serializedPrototype)); if (null !== $errorMessage) { @@ -71,14 +72,17 @@ abstract class FileLoader extends BaseFileLoader continue; } foreach (class_implements($class, false) as $interface) { - $singlyImplemented[$interface] = isset($singlyImplemented[$interface]) ? false : $class; + $this->singlyImplemented[$interface] = isset($this->singlyImplemented[$interface]) ? false : $class; } } } - foreach ($interfaces as $interface) { - if (!empty($singlyImplemented[$interface])) { - $this->container->setAlias($interface, $singlyImplemented[$interface]) - ->setPublic(false); + + foreach ($this->interfaces as $interface) { + if (!empty($this->singlyImplemented[$interface]) && !$this->container->hasAlias($interface)) { + $this->container->setAlias($interface, $this->singlyImplemented[$interface])->setPublic(false); + $this->singlyImplementedAliases[$interface] = true; + } elseif ($this->singlyImplementedAliases[$interface] ?? false) { + $this->container->removeAlias($interface); } } } diff --git a/src/Symfony/Component/DependencyInjection/Loader/PhpFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/PhpFileLoader.php index 033798f681..c7b55826da 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/PhpFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/PhpFileLoader.php @@ -63,6 +63,18 @@ class PhpFileLoader extends FileLoader return 'php' === $type; } + + public function resetBeforeConfiguringServices(): void + { + $this->interfaces = []; + $this->singlyImplemented = []; + $this->singlyImplementedAliases = []; + } + + public function removeSinglyImplementedAlias(string $alias): void + { + unset($this->singlyImplementedAliases[$alias]); + } } /** diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index 50b90a81aa..996d11310e 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -66,6 +66,9 @@ class XmlFileLoader extends FileLoader $this->parseDefinitions($xml, $path, $defaults); } finally { $this->instanceof = []; + $this->interfaces = []; + $this->singlyImplemented = []; + $this->singlyImplementedAliases = []; } } @@ -194,6 +197,7 @@ class XmlFileLoader extends FileLoader $this->validateAlias($service, $file); $this->container->setAlias((string) $service->getAttribute('id'), $alias = new Alias($alias)); + unset($this->singlyImplementedAliases[(string) $service->getAttribute('id')]); if ($publicAttr = $service->getAttribute('public')) { $alias->setPublic(XmlUtils::phpize($publicAttr)); } elseif (isset($defaults['public'])) { diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index bb3868bdea..942c94d989 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -150,6 +150,9 @@ class YamlFileLoader extends FileLoader $this->parseDefinitions($content, $path); } finally { $this->instanceof = []; + $this->interfaces = []; + $this->singlyImplemented = []; + $this->singlyImplementedAliases = []; } } @@ -318,6 +321,7 @@ class YamlFileLoader extends FileLoader if (\is_string($service) && 0 === strpos($service, '@')) { $this->container->setAlias($id, $alias = new Alias(substr($service, 1))); + unset($this->singlyImplementedAliases[$id]); if (isset($defaults['public'])) { $alias->setPublic($defaults['public']); } @@ -341,6 +345,7 @@ class YamlFileLoader extends FileLoader if (isset($service['alias'])) { $this->container->setAlias($id, $alias = new Alias($service['alias'])); + unset($this->singlyImplementedAliases[$id]); if (\array_key_exists('public', $service)) { $alias->setPublic($service['public']); } elseif (isset($defaults['public'])) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/SinglyImplementedInterface/Adapter/Adapter.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/SinglyImplementedInterface/Adapter/Adapter.php new file mode 100644 index 0000000000..3450f0da87 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/SinglyImplementedInterface/Adapter/Adapter.php @@ -0,0 +1,9 @@ +tag('baz'); $di->load(Prototype::class.'\\', '../Prototype') ->autoconfigure() - ->exclude('../Prototype/{OtherDir,BadClasses}') + ->exclude('../Prototype/{OtherDir,BadClasses,SinglyImplementedInterface}') ->factory('f') ->deprecate('%service_id%') ->args([0]) diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype_array.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype_array.php index 819a3fa11c..501baa3c10 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype_array.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype_array.php @@ -9,7 +9,7 @@ return function (ContainerConfigurator $c) { ->tag('baz'); $di->load(Prototype::class.'\\', '../Prototype') ->autoconfigure() - ->exclude(['../Prototype/OtherDir', '../Prototype/BadClasses']) + ->exclude(['../Prototype/OtherDir', '../Prototype/BadClasses', '../Prototype/SinglyImplementedInterface']) ->factory('f') ->deprecate('%service_id%') ->args([0]) diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources.xml new file mode 100644 index 0000000000..ed533e917b --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources.xml @@ -0,0 +1,16 @@ + + + + + + + + + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias.xml new file mode 100644 index 0000000000..e463cfc424 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias.xml @@ -0,0 +1,18 @@ + + + + + + + + + + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias2.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias2.xml new file mode 100644 index 0000000000..29486267fc --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias2.xml @@ -0,0 +1,18 @@ + + + + + + + + + + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_prototype.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_prototype.xml index 9d1d622d4a..1aa28bf341 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_prototype.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_prototype.xml @@ -1,6 +1,6 @@ - + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_prototype_array.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_prototype_array.xml index a62fd06eee..b24b3af577 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_prototype_array.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_prototype_array.xml @@ -4,6 +4,7 @@ ../Prototype/OtherDir ../Prototype/BadClasses + ../Prototype/SinglyImplementedInterface diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/singly_implemented_interface_in_multiple_resources.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/singly_implemented_interface_in_multiple_resources.xml new file mode 100644 index 0000000000..d4af42d525 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/singly_implemented_interface_in_multiple_resources.xml @@ -0,0 +1,14 @@ + + + + + + + + + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources.yml new file mode 100644 index 0000000000..ba8cf0d63f --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources.yml @@ -0,0 +1,12 @@ +services: + _defaults: + autowire: true + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\: + resource: ../Prototype/SinglyImplementedInterface/Adapter/* + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\AnotherAdapter\: + resource: ../Prototype/SinglyImplementedInterface/AnotherAdapter/* + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\: + resource: ../Prototype/SinglyImplementedInterface/Port/* diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias.yml new file mode 100644 index 0000000000..76643879a2 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias.yml @@ -0,0 +1,15 @@ +services: + _defaults: + autowire: true + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\PortInterface: + alias: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\Adapter + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\: + resource: ../Prototype/SinglyImplementedInterface/Port/* + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\: + resource: ../Prototype/SinglyImplementedInterface/Adapter/* + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\AnotherAdapter\: + resource: ../Prototype/SinglyImplementedInterface/AnotherAdapter/* diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias2.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias2.yml new file mode 100644 index 0000000000..2c9a9ccd48 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias2.yml @@ -0,0 +1,15 @@ +services: + _defaults: + autowire: true + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\: + resource: ../Prototype/SinglyImplementedInterface/Port/* + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\: + resource: ../Prototype/SinglyImplementedInterface/Adapter/* + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\PortInterface: + alias: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\Adapter + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\AnotherAdapter\: + resource: ../Prototype/SinglyImplementedInterface/AnotherAdapter/* diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_prototype.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_prototype.yml index 8c0b202aab..43f8d51e04 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_prototype.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_prototype.yml @@ -1,4 +1,4 @@ services: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\: resource: ../Prototype - exclude: '../Prototype/{OtherDir,BadClasses}' + exclude: '../Prototype/{OtherDir,BadClasses,SinglyImplementedInterface}' diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/singly_implemented_interface_in_multiple_resources.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/singly_implemented_interface_in_multiple_resources.yml new file mode 100644 index 0000000000..bc0eb0398c --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/singly_implemented_interface_in_multiple_resources.yml @@ -0,0 +1,9 @@ +services: + _defaults: + autowire: true + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\: + resource: ../Prototype/SinglyImplementedInterface/Port/* + + Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\: + resource: ../Prototype/SinglyImplementedInterface/Adapter/* diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php index dafa0464ed..3e00103aed 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php @@ -652,6 +652,7 @@ class XmlFileLoaderTest extends TestCase [ str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'OtherDir') => true, str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'BadClasses') => true, + str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'SinglyImplementedInterface') => true, ] ); $this->assertContains((string) $globResource, $resources); @@ -684,6 +685,7 @@ class XmlFileLoaderTest extends TestCase [ str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'BadClasses') => true, str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'OtherDir') => true, + str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'SinglyImplementedInterface') => true, ] ); $this->assertContains((string) $globResource, $resources); @@ -901,4 +903,50 @@ class XmlFileLoaderTest extends TestCase $this->assertSame('overridden', $container->get('bar')->quz); } + + public function testSinglyImplementedInterfacesInMultipleResources() + { + $container = new ContainerBuilder(); + + $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml')); + $loader->load('singly_implemented_interface_in_multiple_resources.xml'); + + $alias = $container->getAlias(Prototype\SinglyImplementedInterface\Port\PortInterface::class); + + $this->assertSame(Prototype\SinglyImplementedInterface\Adapter\Adapter::class, (string) $alias); + } + + public function testNotSinglyImplementedInterfacesInMultipleResources() + { + $container = new ContainerBuilder(); + + $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml')); + $loader->load('not_singly_implemented_interface_in_multiple_resources.xml'); + + $this->assertFalse($container->hasAlias(Prototype\SinglyImplementedInterface\Port\PortInterface::class)); + } + + public function testNotSinglyImplementedInterfacesInMultipleResourcesWithPreviouslyRegisteredAlias() + { + $container = new ContainerBuilder(); + + $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml')); + $loader->load('not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias.xml'); + + $alias = $container->getAlias(Prototype\SinglyImplementedInterface\Port\PortInterface::class); + + $this->assertSame(Prototype\SinglyImplementedInterface\Adapter\Adapter::class, (string) $alias); + } + + public function testNotSinglyImplementedInterfacesInMultipleResourcesWithPreviouslyRegisteredAlias2() + { + $container = new ContainerBuilder(); + + $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml')); + $loader->load('not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias2.xml'); + + $alias = $container->getAlias(Prototype\SinglyImplementedInterface\Port\PortInterface::class); + + $this->assertSame(Prototype\SinglyImplementedInterface\Adapter\Adapter::class, (string) $alias); + } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index a7e56e6842..bc34b17428 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -415,6 +415,7 @@ class YamlFileLoaderTest extends TestCase false, [ str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'BadClasses') => true, str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'OtherDir') => true, + str_replace(\DIRECTORY_SEPARATOR, '/', $prototypeRealPath.\DIRECTORY_SEPARATOR.'SinglyImplementedInterface') => true, ] ); $this->assertContains((string) $globResource, $resources); @@ -834,4 +835,50 @@ class YamlFileLoaderTest extends TestCase $this->assertInstanceOf(TaggedIteratorArgument::class, $iteratorArgument); $this->assertNull($iteratorArgument->getIndexAttribute()); } + + public function testSinglyImplementedInterfacesInMultipleResources() + { + $container = new ContainerBuilder(); + + $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml')); + $loader->load('singly_implemented_interface_in_multiple_resources.yml'); + + $alias = $container->getAlias(Prototype\SinglyImplementedInterface\Port\PortInterface::class); + + $this->assertSame(Prototype\SinglyImplementedInterface\Adapter\Adapter::class, (string) $alias); + } + + public function testNotSinglyImplementedInterfacesInMultipleResources() + { + $container = new ContainerBuilder(); + + $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml')); + $loader->load('not_singly_implemented_interface_in_multiple_resources.yml'); + + $this->assertFalse($container->hasAlias(Prototype\SinglyImplementedInterface\Port\PortInterface::class)); + } + + public function testNotSinglyImplementedInterfacesInMultipleResourcesWithPreviouslyRegisteredAlias() + { + $container = new ContainerBuilder(); + + $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml')); + $loader->load('not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias.yml'); + + $alias = $container->getAlias(Prototype\SinglyImplementedInterface\Port\PortInterface::class); + + $this->assertSame(Prototype\SinglyImplementedInterface\Adapter\Adapter::class, (string) $alias); + } + + public function testNotSinglyImplementedInterfacesInMultipleResourcesWithPreviouslyRegisteredAlias2() + { + $container = new ContainerBuilder(); + + $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml')); + $loader->load('not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias2.yml'); + + $alias = $container->getAlias(Prototype\SinglyImplementedInterface\Port\PortInterface::class); + + $this->assertSame(Prototype\SinglyImplementedInterface\Adapter\Adapter::class, (string) $alias); + } } From c1f39709ffd35843d1090fab5ba17cef414e0d7b Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 7 Sep 2019 23:20:36 +0200 Subject: [PATCH 2/2] [DI] add FileLoader::registerAliasesForSinglyImplementedInterfaces() --- UPGRADE-4.4.md | 69 +++++++++---------- .../DependencyInjection/CHANGELOG.md | 1 + .../Configurator/ContainerConfigurator.php | 2 - .../Configurator/ServicesConfigurator.php | 6 +- .../DependencyInjection/Loader/FileLoader.php | 9 +-- .../Loader/PhpFileLoader.php | 23 +++---- .../Loader/XmlFileLoader.php | 5 +- .../Loader/YamlFileLoader.php | 6 +- .../Tests/Loader/FileLoaderTest.php | 6 ++ 9 files changed, 60 insertions(+), 67 deletions(-) diff --git a/UPGRADE-4.4.md b/UPGRADE-4.4.md index 6a2eaea10d..3aeac10b29 100644 --- a/UPGRADE-4.4.md +++ b/UPGRADE-4.4.md @@ -16,41 +16,37 @@ Debug DependencyInjection ------------------- + * Made singly-implemented interfaces detection be scoped by file * Deprecated support for short factories and short configurators in Yaml Before: ```yaml services: - my_service: - factory: factory_service:method + my_service: + factory: factory_service:method ``` After: ```yaml services: - my_service: - factory: ['@factory_service', method] + my_service: + factory: ['@factory_service', method] ``` + * Deprecated `tagged` in favor of `tagged_iterator` Before: ```yaml services: - App\Handler: - tags: ['app.handler'] - App\HandlerCollection: - arguments: [!tagged app.handler] + arguments: [!tagged my_tag] ``` After: ```yaml services: - App\Handler: - tags: ['app.handler'] - - App\HandlerCollection: - arguments: [!tagged_iterator app.handler] + App\HandlerCollection: + arguments: [!tagged_iterator my_tag] ``` * Passing an instance of `Symfony\Component\DependencyInjection\Parameter` as class name to `Symfony\Component\DependencyInjection\Definition` is deprecated. @@ -143,6 +139,7 @@ HttpKernel As many bundles must be compatible with a range of Symfony versions, the current directory convention is not deprecated yet, but it will be in the future. + * Deprecated the second and third argument of `KernelInterface::locateResource` * Deprecated the second and third argument of `FileLocator::__construct` * Deprecated loading resources from `%kernel.root_dir%/Resources` and `%kernel.root_dir%` as @@ -244,13 +241,13 @@ TwigBundle Before (`templates/bundles/TwigBundle/Exception/error.jsonld.twig`): ```twig { - "@id": "https://example.com", - "@type": "error", - "@context": { - "title": "{{ status_text }}", - "code": {{ status_code }}, - "message": "{{ exception.message }}" - } + "@id": "https://example.com", + "@type": "error", + "@context": { + "title": "{{ status_text }}", + "code": {{ status_code }}, + "message": "{{ exception.message }}" + } } ``` @@ -258,23 +255,23 @@ TwigBundle ```php class JsonLdErrorRenderer implements ErrorRendererInterface { - public static function getFormat(): string - { - return 'jsonld'; - } + public static function getFormat(): string + { + return 'jsonld'; + } - public function render(FlattenException $exception): string - { - return json_encode([ - '@id' => 'https://example.com', - '@type' => 'error', - '@context' => [ - 'title' => $exception->getTitle(), - 'code' => $exception->getStatusCode(), - 'message' => $exception->getMessage(), - ], - ]); - } + public function render(FlattenException $exception): string + { + return json_encode([ + '@id' => 'https://example.com', + '@type' => 'error', + '@context' => [ + 'title' => $exception->getTitle(), + 'code' => $exception->getStatusCode(), + 'message' => $exception->getMessage(), + ], + ]); + } } ``` diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md index bbe164bacf..6f56c76103 100644 --- a/src/Symfony/Component/DependencyInjection/CHANGELOG.md +++ b/src/Symfony/Component/DependencyInjection/CHANGELOG.md @@ -8,6 +8,7 @@ CHANGELOG * deprecated support for short factories and short configurators in Yaml * deprecated `tagged` in favor of `tagged_iterator` * deprecated passing an instance of `Symfony\Component\DependencyInjection\Parameter` as class name to `Symfony\Component\DependencyInjection\Definition` + * made singly-implemented interfaces detection be scoped by file 4.3.0 ----- diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php index 3d39c3f864..87beeaa392 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php +++ b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php @@ -73,8 +73,6 @@ class ContainerConfigurator extends AbstractConfigurator final public function services(): ServicesConfigurator { - $this->loader->resetBeforeConfiguringServices(); - return new ServicesConfigurator($this->container, $this->loader, $this->instanceof, $this->path, $this->anonymousCount); } } diff --git a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php index 0688ef92b4..f0fdde81c3 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php +++ b/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php @@ -105,7 +105,6 @@ class ServicesConfigurator extends AbstractConfigurator $ref = static::processValue($referencedId, true); $alias = new Alias((string) $ref, $this->defaults->isPublic()); $this->container->setAlias($id, $alias); - $this->loader->removeSinglyImplementedAlias((string) $ref); return new AliasConfigurator($this, $alias); } @@ -140,4 +139,9 @@ class ServicesConfigurator extends AbstractConfigurator { return $this->set($id, $class); } + + public function __destruct() + { + $this->loader->registerAliasesForSinglyImplementedInterfaces(); + } } diff --git a/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php index 371d9862aa..b522a31382 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php @@ -31,7 +31,6 @@ abstract class FileLoader extends BaseFileLoader protected $instanceof = []; protected $interfaces = []; protected $singlyImplemented = []; - protected $singlyImplementedAliases = []; public function __construct(ContainerBuilder $container, FileLocatorInterface $locator) { @@ -76,15 +75,17 @@ abstract class FileLoader extends BaseFileLoader } } } + } + public function registerAliasesForSinglyImplementedInterfaces() + { foreach ($this->interfaces as $interface) { if (!empty($this->singlyImplemented[$interface]) && !$this->container->hasAlias($interface)) { $this->container->setAlias($interface, $this->singlyImplemented[$interface])->setPublic(false); - $this->singlyImplementedAliases[$interface] = true; - } elseif ($this->singlyImplementedAliases[$interface] ?? false) { - $this->container->removeAlias($interface); } } + + $this->interfaces = $this->singlyImplemented = []; } /** diff --git a/src/Symfony/Component/DependencyInjection/Loader/PhpFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/PhpFileLoader.php index c7b55826da..0efa1239f0 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/PhpFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/PhpFileLoader.php @@ -41,10 +41,15 @@ class PhpFileLoader extends FileLoader return include $path; }, $this, ProtectedPhpFileLoader::class); - $callback = $load($path); + try { + $callback = $load($path); - if (\is_object($callback) && \is_callable($callback)) { - $callback(new ContainerConfigurator($this->container, $this, $this->instanceof, $path, $resource), $this->container, $this); + if (\is_object($callback) && \is_callable($callback)) { + $callback(new ContainerConfigurator($this->container, $this, $this->instanceof, $path, $resource), $this->container, $this); + } + } finally { + $this->instanceof = []; + $this->registerAliasesForSinglyImplementedInterfaces(); } } @@ -63,18 +68,6 @@ class PhpFileLoader extends FileLoader return 'php' === $type; } - - public function resetBeforeConfiguringServices(): void - { - $this->interfaces = []; - $this->singlyImplemented = []; - $this->singlyImplementedAliases = []; - } - - public function removeSinglyImplementedAlias(string $alias): void - { - unset($this->singlyImplementedAliases[$alias]); - } } /** diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index 996d11310e..06100a46d7 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -66,9 +66,7 @@ class XmlFileLoader extends FileLoader $this->parseDefinitions($xml, $path, $defaults); } finally { $this->instanceof = []; - $this->interfaces = []; - $this->singlyImplemented = []; - $this->singlyImplementedAliases = []; + $this->registerAliasesForSinglyImplementedInterfaces(); } } @@ -197,7 +195,6 @@ class XmlFileLoader extends FileLoader $this->validateAlias($service, $file); $this->container->setAlias((string) $service->getAttribute('id'), $alias = new Alias($alias)); - unset($this->singlyImplementedAliases[(string) $service->getAttribute('id')]); if ($publicAttr = $service->getAttribute('public')) { $alias->setPublic(XmlUtils::phpize($publicAttr)); } elseif (isset($defaults['public'])) { diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 942c94d989..03daeaeff3 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -150,9 +150,7 @@ class YamlFileLoader extends FileLoader $this->parseDefinitions($content, $path); } finally { $this->instanceof = []; - $this->interfaces = []; - $this->singlyImplemented = []; - $this->singlyImplementedAliases = []; + $this->registerAliasesForSinglyImplementedInterfaces(); } } @@ -321,7 +319,6 @@ class YamlFileLoader extends FileLoader if (\is_string($service) && 0 === strpos($service, '@')) { $this->container->setAlias($id, $alias = new Alias(substr($service, 1))); - unset($this->singlyImplementedAliases[$id]); if (isset($defaults['public'])) { $alias->setPublic($defaults['public']); } @@ -345,7 +342,6 @@ class YamlFileLoader extends FileLoader if (isset($service['alias'])) { $this->container->setAlias($id, $alias = new Alias($service['alias'])); - unset($this->singlyImplementedAliases[$id]); if (\array_key_exists('public', $service)) { $alias->setPublic($service['public']); } elseif (isset($defaults['public'])) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php index d2800def2a..d4446cbe4e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php @@ -260,4 +260,10 @@ class TestFileLoader extends FileLoader { return false; } + + public function registerClasses(Definition $prototype, $namespace, $resource, $exclude = null) + { + parent::registerClasses($prototype, $namespace, $resource, $exclude); + $this->registerAliasesForSinglyImplementedInterfaces(); + } }