From e1e74404a02c841bd2b0d33b11fcf01998541e81 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Sun, 31 May 2015 10:14:49 +0200 Subject: [PATCH] [DependencyInjection] provide better error message when using deprecated configuration options --- .../Loader/XmlFileLoader.php | 22 ++++++++++++++----- .../Loader/YamlFileLoader.php | 4 ++++ .../Tests/Fixtures/xml/legacy-services6.xml | 1 + .../Tests/Fixtures/xml/services6.xml | 1 - .../Tests/Fixtures/yaml/legacy-services6.yml | 5 +++++ .../Tests/Fixtures/yaml/services6.yml | 5 ----- .../Tests/Loader/XmlFileLoaderTest.php | 9 ++++---- .../Tests/Loader/YamlFileLoaderTest.php | 9 ++++---- 8 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index f81d8d6931..50ad328812 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -126,10 +126,11 @@ class XmlFileLoader extends FileLoader * Parses an individual Definition. * * @param \DOMElement $service + * @param string $file * * @return Definition|null */ - private function parseDefinition(\DOMElement $service) + private function parseDefinition(\DOMElement $service, $file) { if ($alias = $service->getAttribute('alias')) { $public = true; @@ -149,13 +150,22 @@ class XmlFileLoader extends FileLoader foreach (array('class', 'scope', 'public', 'factory-class', 'factory-method', 'factory-service', 'synthetic', 'lazy', 'abstract') as $key) { if ($value = $service->getAttribute($key)) { + if (in_array($key, array('factory-class', 'factory-method', 'factory-service'))) { + trigger_error(sprintf('The "%s" attribute in file "%s" is deprecated since version 2.6 and will be removed in 3.0. Use the "factory" element instead.', $key, $file), E_USER_DEPRECATED); + } $method = 'set'.str_replace('-', '', $key); $definition->$method(XmlUtils::phpize($value)); } } if ($value = $service->getAttribute('synchronized')) { - $definition->setSynchronized(XmlUtils::phpize($value), 'request' !== (string) $service->getAttribute('id')); + $triggerDeprecation = 'request' !== (string) $service->getAttribute('id'); + + if ($triggerDeprecation) { + trigger_error(sprintf('The "synchronized" attribute in file "%s" is deprecated since version 2.7 and will be removed in 3.0.', $file), E_USER_DEPRECATED); + } + + $definition->setSynchronized(XmlUtils::phpize($value), $triggerDeprecation); } if ($files = $this->getChildren($service, 'file')) { @@ -173,7 +183,7 @@ class XmlFileLoader extends FileLoader $factoryService = $this->getChildren($factory, 'service'); if (isset($factoryService[0])) { - $class = $this->parseDefinition($factoryService[0]); + $class = $this->parseDefinition($factoryService[0], $file); } elseif ($childService = $factory->getAttribute('service')) { $class = new Reference($childService, ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, false); } else { @@ -192,7 +202,7 @@ class XmlFileLoader extends FileLoader $configuratorService = $this->getChildren($configurator, 'service'); if (isset($configuratorService[0])) { - $class = $this->parseDefinition($configuratorService[0]); + $class = $this->parseDefinition($configuratorService[0], $file); } elseif ($childService = $configurator->getAttribute('service')) { $class = new Reference($childService, ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, false); } else { @@ -233,7 +243,7 @@ class XmlFileLoader extends FileLoader } /** - * Parses a XML file to a \DOMDocument + * Parses a XML file to a \DOMDocument. * * @param string $file Path to a file * @@ -392,7 +402,7 @@ class XmlFileLoader extends FileLoader } /** - * Get child elements by name + * Get child elements by name. * * @param \DOMNode $node * @param mixed $name diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index ae9613a612..455e06a985 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -172,6 +172,7 @@ class YamlFileLoader extends FileLoader } if (isset($service['synchronized'])) { + trigger_error(sprintf('The "synchronized" key in file "%s" is deprecated since version 2.7 and will be removed in 3.0.', $file), E_USER_DEPRECATED); $definition->setSynchronized($service['synchronized'], 'request' !== $id); } @@ -201,14 +202,17 @@ class YamlFileLoader extends FileLoader } if (isset($service['factory_class'])) { + trigger_error(sprintf('The "factory_class" key in file "%s" is deprecated since version 2.6 and will be removed in 3.0. Use "factory" instead.', $file), E_USER_DEPRECATED); $definition->setFactoryClass($service['factory_class']); } if (isset($service['factory_method'])) { + trigger_error(sprintf('The "factory_method" key in file "%s" is deprecated since version 2.6 and will be removed in 3.0. Use "factory" instead.', $file), E_USER_DEPRECATED); $definition->setFactoryMethod($service['factory_method']); } if (isset($service['factory_service'])) { + trigger_error(sprintf('The "factory_service" key in file "%s" is deprecated since version 2.6 and will be removed in 3.0. Use "factory" instead.', $file), E_USER_DEPRECATED); $definition->setFactoryService($service['factory_service']); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/legacy-services6.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/legacy-services6.xml index 17fe00f8fe..708e10fd5d 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/legacy-services6.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/legacy-services6.xml @@ -6,5 +6,6 @@ + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml index 3a68c6e39a..9eb7b8915e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services6.xml @@ -47,7 +47,6 @@ - diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/legacy-services6.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/legacy-services6.yml index d6ca937a5e..46ac679940 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/legacy-services6.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/legacy-services6.yml @@ -1,3 +1,8 @@ services: constructor: { class: FooClass, factory_method: getInstance } factory_service: { class: BazClass, factory_method: getInstance, factory_service: baz_factory } + request: + class: Request + synthetic: true + synchronized: true + lazy: true diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml index 8820b274ee..78abf4d155 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services6.yml @@ -23,11 +23,6 @@ services: another_alias_for_foo: alias: foo public: false - request: - class: Request - synthetic: true - synchronized: true - lazy: true decorator_service: decorates: decorated decorator_service_with_name: diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php index 501d680fcc..56f599b7cc 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php @@ -206,6 +206,10 @@ class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase $this->assertNull($services['factory_service']->getClass()); $this->assertEquals('baz_factory', $services['factory_service']->getFactoryService()); $this->assertEquals('getInstance', $services['factory_service']->getFactoryMethod()); + $this->assertTrue($services['request']->isSynthetic(), '->load() parses the synthetic flag'); + $this->assertTrue($services['request']->isSynchronized(), '->load() parses the synchronized flag'); + $this->assertTrue($services['request']->isLazy(), '->load() parses the lazy flag'); + $this->assertNull($services['request']->getDecoratedService()); } public function testLoadServices() @@ -231,10 +235,6 @@ class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array(new Reference('baz', ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, false), 'getClass'), $services['new_factory2']->getFactory(), '->load() parses the factory tag'); $this->assertEquals(array('BazClass', 'getInstance'), $services['new_factory3']->getFactory(), '->load() parses the factory tag'); - $this->assertTrue($services['request']->isSynthetic(), '->load() parses the synthetic flag'); - $this->assertTrue($services['request']->isSynchronized(false), '->load() parses the synchronized flag'); - $this->assertTrue($services['request']->isLazy(), '->load() parses the lazy flag'); - $aliases = $container->getAliases(); $this->assertTrue(isset($aliases['alias_for_foo']), '->load() parses elements'); $this->assertEquals('foo', (string) $aliases['alias_for_foo'], '->load() parses aliases'); @@ -243,7 +243,6 @@ class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase $this->assertEquals('foo', (string) $aliases['another_alias_for_foo']); $this->assertFalse($aliases['another_alias_for_foo']->isPublic()); - $this->assertNull($services['request']->getDecoratedService()); $this->assertEquals(array('decorated', null), $services['decorator_service']->getDecoratedService()); $this->assertEquals(array('decorated', 'decorated.pif-pouf'), $services['decorator_service_with_name']->getDecoratedService()); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index 716aba5586..f329aace79 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -135,6 +135,10 @@ class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase $this->assertEquals('BazClass', $services['factory_service']->getClass()); $this->assertEquals('baz_factory', $services['factory_service']->getFactoryService()); $this->assertEquals('getInstance', $services['factory_service']->getFactoryMethod()); + $this->assertTrue($services['request']->isSynthetic(), '->load() parses the synthetic flag'); + $this->assertTrue($services['request']->isSynchronized(), '->load() parses the synchronized flag'); + $this->assertTrue($services['request']->isLazy(), '->load() parses the lazy flag'); + $this->assertNull($services['request']->getDecoratedService()); } public function testLoadServices() @@ -160,10 +164,6 @@ class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array(new Reference('baz'), 'getClass'), $services['new_factory2']->getFactory(), '->load() parses the factory tag'); $this->assertEquals(array('BazClass', 'getInstance'), $services['new_factory3']->getFactory(), '->load() parses the factory tag'); - $this->assertTrue($services['request']->isSynthetic(), '->load() parses the synthetic flag'); - $this->assertTrue($services['request']->isSynchronized(false), '->load() parses the synchronized flag'); - $this->assertTrue($services['request']->isLazy(), '->load() parses the lazy flag'); - $aliases = $container->getAliases(); $this->assertTrue(isset($aliases['alias_for_foo']), '->load() parses aliases'); $this->assertEquals('foo', (string) $aliases['alias_for_foo'], '->load() parses aliases'); @@ -172,7 +172,6 @@ class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase $this->assertEquals('foo', (string) $aliases['another_alias_for_foo']); $this->assertFalse($aliases['another_alias_for_foo']->isPublic()); - $this->assertNull($services['request']->getDecoratedService()); $this->assertEquals(array('decorated', null), $services['decorator_service']->getDecoratedService()); $this->assertEquals(array('decorated', 'decorated.pif-pouf'), $services['decorator_service_with_name']->getDecoratedService()); }