[DI] Deprecate underscore-services in YamlFileLoader #21484
Conversation
| @@ -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); | |||
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
|
Maybe we could use a yaml tag for default values (or other non-service keys) instead and keep allowing anything for service names. |
|
We wondered about tags for eg |
|
Probably something like this services:
defaults: !default
public: false
Foo\Bar: [@foo]Any service tagged with default would be used as default instead of a service. And the key would be ignored. |
|
That's what I meant by:
I keep this opinion :) |
|
Yeah I don't like it that much, but using underscore-names to convey special meanings looks to much like a hack to me to like it. |
|
Is it too late to create a new config option called services_defaults:
public: false
services:
Foo\Bar: [@foo] |
|
@javiereguiluz this discussion already happened, see #21071 :) |
|
@javiereguiluz this is also a BC break, as it makes this key forbidden for DI extensions (and it is much harder to write a BC layer, as we would be unable to detect this case as the DI extension could expose a config similar to our defaults) |
53a1e98 to
7e2e4ce
Compare
7e2e4ce to
7781082
Compare
|
Thank you @nicolas-grekas. |
…nicolas-grekas) This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Deprecate underscore-services in YamlFileLoader | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | no | Fixed tickets | - | License | MIT | Doc PR | - As discussed when introducing `_defaults` Commits ------- 7781082 [DI] Deprecate underscore-services in YamlFileLoader
As discussed when introducing
_defaults