Skip to content

Commit 6200eb5

Browse files
committed
feature #15491 Add support for deprecated definitions (Taluu)
This PR was merged into the 2.8 branch. Discussion ---------- Add support for deprecated definitions | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #14307 | License | MIT | Doc PR | symfony/symfony-docs#5689 This add a sort of marker in the Definition of a service that marks it as "deprecated". This is useful when we have a bunch of service and a bunch of where it is used, and we need to track if there are any uses before removing it (in a later version or right now). I was not sure if the `trigger_error` would be enough, or if I should log them instead. I'm first gathering some feedback, and then I'll try to update the doc. I was not sure if it should target 2.8 or master (3.0) though. What's left ? ========== - [x] Make a POC - [x] Gather some feedbacks - [x] Dump the tag in XML, YAML and PHP - [x] Load the definition from XML, YAML and PHP - [x] Fix some forgotten things such as the key existence check - [x] Work on inline services in the php dumper - [x] Handle deprecations for decorators - ~~Possibility to overwrite the deprecated flag in the decorators in `XmlFileLoader` ?~~ Nope, and this behavior is also ported to the `YamlFileLoader`. Commits ------- 83f4e9c [DI] Support deprecated definitions in decorators 0b3d0a0 [DI] Allow to change the deprecation message in Definition 954247d [DI] Trigger a deprecated error on the container builder 2f37cb1 [DI] Dump the deprecated status 8f6c21c [DI] Supports the deprecated tag in loaders 4b6fab0 [DI] Add a deprecated status to definitions
2 parents 2377994 + 83f4e9c commit 6200eb5

19 files changed

+249
-1
lines changed

Diff for: src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php

+6
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,9 @@ private function resolveDefinition(ContainerBuilder $container, DefinitionDecora
127127
if ($parentDef->getFactoryService(false)) {
128128
$def->setFactoryService($parentDef->getFactoryService(false));
129129
}
130+
if ($parentDef->isDeprecated()) {
131+
$def->setDeprecated(true, $parentDef->getDeprecationMessage('%service_id%'));
132+
}
130133
$def->setFactory($parentDef->getFactory());
131134
$def->setConfigurator($parentDef->getConfigurator());
132135
$def->setFile($parentDef->getFile());
@@ -162,6 +165,9 @@ private function resolveDefinition(ContainerBuilder $container, DefinitionDecora
162165
if (isset($changes['lazy'])) {
163166
$def->setLazy($definition->isLazy());
164167
}
168+
if (isset($changes['deprecated'])) {
169+
$def->setDeprecated($definition->isDeprecated(), $definition->getDeprecationMessage('%service_id%'));
170+
}
165171
if (isset($changes['decorated_service'])) {
166172
$decoratedService = $definition->getDecoratedService();
167173
if (null === $decoratedService) {

Diff for: src/Symfony/Component/DependencyInjection/ContainerBuilder.php

+4
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,10 @@ public function createService(Definition $definition, $id, $tryProxy = true)
931931
throw new RuntimeException(sprintf('You have requested a synthetic service ("%s"). The DIC does not know how to construct this service.', $id));
932932
}
933933

934+
if ($definition->isDeprecated()) {
935+
@trigger_error($definition->getDeprecationMessage($id), E_USER_DEPRECATED);
936+
}
937+
934938
if ($tryProxy && $definition->isLazy()) {
935939
$container = $this;
936940

Diff for: src/Symfony/Component/DependencyInjection/Definition.php

+61
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ class Definition
3030
private $factoryMethod;
3131
private $factoryService;
3232
private $shared = true;
33+
private $deprecated = false;
34+
private $deprecationTemplate = 'The "%service_id%" service is deprecated. You should stop using it, as it will soon be removed.';
3335
private $scope = ContainerInterface::SCOPE_CONTAINER;
3436
private $properties = array();
3537
private $calls = array();
@@ -829,6 +831,65 @@ public function isAbstract()
829831
return $this->abstract;
830832
}
831833

834+
/**
835+
* Whether this definition is deprecated, that means it should not be called
836+
* anymore.
837+
*
838+
* @param bool $status
839+
* @param string $template Template message to use if the definition is deprecated
840+
*
841+
* @return Definition the current instance
842+
*
843+
* @throws InvalidArgumentException When the message template is invalid.
844+
*
845+
* @api
846+
*/
847+
public function setDeprecated($status = true, $template = null)
848+
{
849+
if (null !== $template) {
850+
if (preg_match('#[\r\n]|\*/#', $template)) {
851+
throw new InvalidArgumentException('Invalid characters found in deprecation template.');
852+
}
853+
854+
if (false === strpos($template, '%service_id%')) {
855+
throw new InvalidArgumentException('The deprecation template must contain the "%service_id%" placeholder.');
856+
}
857+
858+
$this->deprecationTemplate = $template;
859+
}
860+
861+
$this->deprecated = (bool) $status;
862+
863+
return $this;
864+
}
865+
866+
/**
867+
* Whether this definition is deprecated, that means it should not be called
868+
* anymore.
869+
*
870+
* @return bool
871+
*
872+
* @api
873+
*/
874+
public function isDeprecated()
875+
{
876+
return $this->deprecated;
877+
}
878+
879+
/**
880+
* Message to use if this definition is deprecated.
881+
*
882+
* @param string $id Service id relying on this definition
883+
*
884+
* @return string
885+
*
886+
* @api
887+
*/
888+
public function getDeprecationMessage($id)
889+
{
890+
return str_replace('%service_id%', $id, $this->deprecationTemplate);
891+
}
892+
832893
/**
833894
* Sets a configurator to call after the service is fully initialized.
834895
*

Diff for: src/Symfony/Component/DependencyInjection/DefinitionDecorator.php

+10
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,16 @@ public function setDecoratedService($id, $renamedId = null, $priority = 0)
180180
return parent::setDecoratedService($id, $renamedId, $priority);
181181
}
182182

183+
/**
184+
* {@inheritdoc}
185+
*/
186+
public function setDeprecated($boolean = true, $template = null)
187+
{
188+
$this->changes['deprecated'] = true;
189+
190+
return parent::setDeprecated($boolean, $template);
191+
}
192+
183193
/**
184194
* Gets an argument to pass to the service constructor/factory method.
185195
*

Diff for: src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

+13-1
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,15 @@ private function addService($id, $definition)
592592
$return[] = sprintf("@throws InactiveScopeException when the '%s' service is requested while the '%s' scope is not active", $id, $scope);
593593
}
594594

595-
$return = implode("\n * ", $return);
595+
if ($definition->isDeprecated()) {
596+
if ($return && 0 === strpos($return[count($return) - 1], '@return')) {
597+
$return[] = '';
598+
}
599+
600+
$return[] = sprintf('@deprecated %s', $definition->getDeprecationMessage($id));
601+
}
602+
603+
$return = str_replace("\n * \n", "\n *\n", implode("\n * ", $return));
596604

597605
$doc = '';
598606
if ($definition->isShared() && ContainerInterface::SCOPE_PROTOTYPE !== $scope) {
@@ -652,6 +660,10 @@ private function addService($id, $definition)
652660
if ($definition->isSynthetic()) {
653661
$code .= sprintf(" throw new RuntimeException('You have requested a synthetic service (\"%s\"). The DIC does not know how to construct this service.');\n }\n", $id);
654662
} else {
663+
if ($definition->isDeprecated()) {
664+
$code .= sprintf(" @trigger_error(%s, E_USER_DEPRECATED);\n\n", var_export($definition->getDeprecationMessage($id), true));
665+
}
666+
655667
$code .=
656668
$this->addServiceInclude($id, $definition).
657669
$this->addServiceLocalTempVariables($id, $definition).

Diff for: src/Symfony/Component/DependencyInjection/Dumper/XmlDumper.php

+7
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,13 @@ private function addService($definition, $id, \DOMElement $parent)
201201
$service->appendChild($factory);
202202
}
203203

204+
if ($definition->isDeprecated()) {
205+
$deprecated = $this->document->createElement('deprecated');
206+
$deprecated->appendChild($this->document->createTextNode($definition->getDeprecationMessage('%service_id%')));
207+
208+
$service->appendChild($deprecated);
209+
}
210+
204211
if ($callable = $definition->getConfigurator()) {
205212
$configurator = $this->document->createElement('configurator');
206213

Diff for: src/Symfony/Component/DependencyInjection/Dumper/YamlDumper.php

+4
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ private function addService($id, $definition)
104104
$code .= sprintf(" synchronized: true\n");
105105
}
106106

107+
if ($definition->isDeprecated()) {
108+
$code .= sprintf(" deprecated: %s\n", $definition->getDeprecationMessage('%service_id%'));
109+
}
110+
107111
if ($definition->getFactoryClass(false)) {
108112
$code .= sprintf(" factory_class: %s\n", $definition->getFactoryClass(false));
109113
}

Diff for: src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php

+4
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,10 @@ private function parseDefinition(\DOMElement $service, $file)
181181
$definition->setFile($files[0]->nodeValue);
182182
}
183183

184+
if ($deprecated = $this->getChildren($service, 'deprecated')) {
185+
$definition->setDeprecated(true, $deprecated[0]->nodeValue);
186+
}
187+
184188
$definition->setArguments($this->getArgumentsAsPhp($service, 'argument'));
185189
$definition->setProperties($this->getArgumentsAsPhp($service, 'property'));
186190

Diff for: src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php

+4
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,10 @@ private function parseDefinition($id, $service, $file)
196196
$definition->setAbstract($service['abstract']);
197197
}
198198

199+
if (array_key_exists('deprecated', $service)) {
200+
$definition->setDeprecated(true, $service['deprecated']);
201+
}
202+
199203
if (isset($service['factory'])) {
200204
if (is_string($service['factory'])) {
201205
if (strpos($service['factory'], ':') !== false && strpos($service['factory'], '::') === false) {

Diff for: src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd

+1
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
<xsd:element name="argument" type="argument" minOccurs="0" maxOccurs="unbounded" />
8282
<xsd:element name="configurator" type="callable" minOccurs="0" maxOccurs="1" />
8383
<xsd:element name="factory" type="callable" minOccurs="0" maxOccurs="1" />
84+
<xsd:element name="deprecated" type="xsd:string" minOccurs="0" maxOccurs="1" />
8485
<xsd:element name="call" type="call" minOccurs="0" maxOccurs="unbounded" />
8586
<xsd:element name="tag" type="tag" minOccurs="0" maxOccurs="unbounded" />
8687
<xsd:element name="property" type="property" minOccurs="0" maxOccurs="unbounded" />

Diff for: src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveDefinitionTemplatesPassTest.php

+30
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,36 @@ public function testSetDecoratedServiceOnServiceHasParent()
244244
$this->assertEquals(array('foo', 'foo_inner', 0), $container->getDefinition('child1')->getDecoratedService());
245245
}
246246

247+
public function testDecoratedServiceCopiesDeprecatedStatusFromParent()
248+
{
249+
$container = new ContainerBuilder();
250+
$container->register('deprecated_parent')
251+
->setDeprecated(true)
252+
;
253+
254+
$container->setDefinition('decorated_deprecated_parent', new DefinitionDecorator('deprecated_parent'));
255+
256+
$this->process($container);
257+
258+
$this->assertTrue($container->getDefinition('decorated_deprecated_parent')->isDeprecated());
259+
}
260+
261+
public function testDecoratedServiceCanOverwriteDeprecatedParentStatus()
262+
{
263+
$container = new ContainerBuilder();
264+
$container->register('deprecated_parent')
265+
->setDeprecated(true)
266+
;
267+
268+
$container->setDefinition('decorated_deprecated_parent', new DefinitionDecorator('deprecated_parent'))
269+
->setDeprecated(false)
270+
;
271+
272+
$this->process($container);
273+
274+
$this->assertFalse($container->getDefinition('decorated_deprecated_parent')->isDeprecated());
275+
}
276+
247277
protected function process(ContainerBuilder $container)
248278
{
249279
$pass = new ResolveDefinitionTemplatesPass();

Diff for: src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php

+22
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,28 @@ public function testDefinitions()
6363
}
6464
}
6565

66+
public function testCreateDeprecatedService()
67+
{
68+
$definition = new Definition('stdClass');
69+
$definition->setDeprecated(true);
70+
71+
$that = $this;
72+
$wasTriggered = false;
73+
74+
set_error_handler(function ($errno, $errstr) use ($that, &$wasTriggered) {
75+
$that->assertSame(E_USER_DEPRECATED, $errno);
76+
$that->assertSame('The "deprecated_foo" service is deprecated. You should stop using it, as it will soon be removed.', $errstr);
77+
$wasTriggered = true;
78+
});
79+
80+
$builder = new ContainerBuilder();
81+
$builder->createService($definition, 'deprecated_foo');
82+
83+
restore_error_handler();
84+
85+
$this->assertTrue($wasTriggered);
86+
}
87+
6688
/**
6789
* @covers Symfony\Component\DependencyInjection\ContainerBuilder::register
6890
*/

Diff for: src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php

+36
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,42 @@ public function testSetIsAbstract()
233233
$this->assertTrue($def->isAbstract(), '->isAbstract() returns true if the instance must not be public.');
234234
}
235235

236+
/**
237+
* @covers Symfony\Component\DependencyInjection\Definition::setDeprecated
238+
* @covers Symfony\Component\DependencyInjection\Definition::isDeprecated
239+
* @covers Symfony\Component\DependencyInjection\Definition::hasCustomDeprecationTemplate
240+
* @covers Symfony\Component\DependencyInjection\Definition::getDeprecationMessage
241+
*/
242+
public function testSetIsDeprecated()
243+
{
244+
$def = new Definition('stdClass');
245+
$this->assertFalse($def->isDeprecated(), '->isDeprecated() returns false by default');
246+
$this->assertSame($def, $def->setDeprecated(true), '->setDeprecated() implements a fluent interface');
247+
$this->assertTrue($def->isDeprecated(), '->isDeprecated() returns true if the instance should not be used anymore.');
248+
$this->assertSame('The "deprecated_service" service is deprecated. You should stop using it, as it will soon be removed.', $def->getDeprecationMessage('deprecated_service'), '->getDeprecationMessage() should return a formatted message template');
249+
}
250+
251+
/**
252+
* @dataProvider invalidDeprecationMessageProvider
253+
* @covers Symfony\Component\DependencyInjection\Definition::setDeprecated
254+
* @expectedException Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
255+
*/
256+
public function testSetDeprecatedWithInvalidDeprecationTemplate($message)
257+
{
258+
$def = new Definition('stdClass');
259+
$def->setDeprecated(false, $message);
260+
}
261+
262+
public function invalidDeprecationMessageProvider()
263+
{
264+
return array(
265+
"With \rs" => array("invalid \r message %service_id%"),
266+
"With \ns" => array("invalid \n message %service_id%"),
267+
'With */s' => array('invalid */ message %service_id%'),
268+
'message not containing require %service_id% variable' => array('this is deprecated'),
269+
);
270+
}
271+
236272
/**
237273
* @covers Symfony\Component\DependencyInjection\Definition::setConfigurator
238274
* @covers Symfony\Component\DependencyInjection\Definition::getConfigurator

Diff for: src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php

+4
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@
8989
->register('decorator_service_with_name', 'stdClass')
9090
->setDecoratedService('decorated', 'decorated.pif-pouf')
9191
;
92+
$container
93+
->register('deprecated_service', 'stdClass')
94+
->setDeprecated(true)
95+
;
9296
$container
9397
->register('new_factory', 'FactoryClass')
9498
->setProperty('foo', 'bar')

Diff for: src/Symfony/Component/DependencyInjection/Tests/Fixtures/graphviz/services9.dot

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ digraph sc {
1717
node_decorated [label="decorated\nstdClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
1818
node_decorator_service [label="decorator_service\nstdClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
1919
node_decorator_service_with_name [label="decorator_service_with_name\nstdClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
20+
node_deprecated_service [label="deprecated_service\nstdClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
2021
node_new_factory [label="new_factory\nFactoryClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
2122
node_factory_service [label="factory_service\nBar\n", shape=record, fillcolor="#eeeeee", style="filled"];
2223
node_new_factory_service [label="new_factory_service\nFooBarBaz\n", shape=record, fillcolor="#eeeeee", style="filled"];

Diff for: src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php

+18
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public function __construct()
3333
'decorated' => 'getDecoratedService',
3434
'decorator_service' => 'getDecoratorServiceService',
3535
'decorator_service_with_name' => 'getDecoratorServiceWithNameService',
36+
'deprecated_service' => 'getDeprecatedServiceService',
3637
'factory_service' => 'getFactoryServiceService',
3738
'foo' => 'getFooService',
3839
'foo.baz' => 'getFoo_BazService',
@@ -143,6 +144,23 @@ protected function getDecoratorServiceWithNameService()
143144
return $this->services['decorator_service_with_name'] = new \stdClass();
144145
}
145146

147+
/**
148+
* Gets the 'deprecated_service' service.
149+
*
150+
* This service is shared.
151+
* This method always returns the same instance of the service.
152+
*
153+
* @return \stdClass A stdClass instance.
154+
*
155+
* @deprecated The "deprecated_service" service is deprecated. You should stop using it, as it will soon be removed.
156+
*/
157+
protected function getDeprecatedServiceService()
158+
{
159+
@trigger_error('The "deprecated_service" service is deprecated. You should stop using it, as it will soon be removed.', E_USER_DEPRECATED);
160+
161+
return $this->services['deprecated_service'] = new \stdClass();
162+
}
163+
146164
/**
147165
* Gets the 'factory_service' service.
148166
*

Diff for: src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php

+18
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public function __construct()
3737
'configured_service' => 'getConfiguredServiceService',
3838
'decorator_service' => 'getDecoratorServiceService',
3939
'decorator_service_with_name' => 'getDecoratorServiceWithNameService',
40+
'deprecated_service' => 'getDeprecatedServiceService',
4041
'factory_service' => 'getFactoryServiceService',
4142
'foo' => 'getFooService',
4243
'foo.baz' => 'getFoo_BazService',
@@ -144,6 +145,23 @@ protected function getDecoratorServiceWithNameService()
144145
return $this->services['decorator_service_with_name'] = new \stdClass();
145146
}
146147

148+
/**
149+
* Gets the 'deprecated_service' service.
150+
*
151+
* This service is shared.
152+
* This method always returns the same instance of the service.
153+
*
154+
* @return \stdClass A stdClass instance.
155+
*
156+
* @deprecated The "deprecated_service" service is deprecated. You should stop using it, as it will soon be removed.
157+
*/
158+
protected function getDeprecatedServiceService()
159+
{
160+
@trigger_error('The "deprecated_service" service is deprecated. You should stop using it, as it will soon be removed.', E_USER_DEPRECATED);
161+
162+
return $this->services['deprecated_service'] = new \stdClass();
163+
}
164+
147165
/**
148166
* Gets the 'factory_service' service.
149167
*

0 commit comments

Comments
 (0)