Skip to content

[DI] Deprecate underscore-services in YamlFileLoader #21484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 16, 2017
Merged

[DI] Deprecate underscore-services in YamlFileLoader #21484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ private function parseDefaults(array &$content, $file)
throw new InvalidArgumentException(sprintf('Service defaults must be an array, "%s" given in "%s".', gettype($defaults), $file));
}
if (isset($defaults['alias']) || isset($defaults['class']) || isset($defaults['factory'])) {
@trigger_error('Giving a service the "_defaults" name is deprecated since Symfony 3.3 and will be forbidden in 4.0. Rename your service.', E_USER_DEPRECATED);
// @deprecated code path, to be removed in 4.0

return array();
}
Expand Down Expand Up @@ -239,6 +239,9 @@ private function parseDefaults(array &$content, $file)
*/
private function parseDefinition($id, $service, $file, array $defaults)
{
if (preg_match('/^_[a-zA-Z0-9_]*$/', $id)) {
@trigger_error(sprintf('Service names that start with an underscore are deprecated since Symfony 3.3 and will be reserved in 4.0. Rename the "%s" service or define it in XML instead.', $id), E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a good idea that the allowed service ids depend on the config format. In the past, we've worked hard to make all formats work the same in spite of their differences (for example, by adding PHP constants support for Yaml files). If we want to deprecate something, we could deprecate just the _defaults string as service id (whatever the config format used).

Copy link
Member Author

@nicolas-grekas nicolas-grekas Feb 1, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is little comparison between supporting a feature in all formats (eg constants), and excluding a very specific narrow case for one specific loader which misses a few oops of expressiveness (eg underscore-services, which are not a "feature").
We also stated previously that we don't want parity with all loader formats - quite the contrary in fact: we want to embrace the full expressiveness of each format. Here, it's exactly a matter of expressiveness. Thus falls into this policy to me - thus only the yaml loader should be affected.

}
if (is_string($service) && 0 === strpos($service, '@')) {
$public = isset($defaults['public']) ? $defaults['public'] : true;
$this->container->setAlias($id, new Alias(substr($service, 1), $public));
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services_underscore.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
services:
_foo:
class: Foo
11 changes: 11 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,4 +436,15 @@ public function testInvalidTagsWithDefaults()
$loader = new YamlFileLoader(new ContainerBuilder(), new FileLocator(self::$fixturesPath.'/yaml'));
$loader->load('services31_invalid_tags.yml');
}

/**
* @group legacy
* @expectedDeprecation Service names that start with an underscore are deprecated since Symfony 3.3 and will be reserved in 4.0. Rename the "_foo" service or define it in XML instead.
*/
public function testUnderscoreServiceId()
{
$container = new ContainerBuilder();
$loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
$loader->load('services_underscore.yml');
}
}