8000 feature #22185 [DI] Enhance DX by throwing instead of triggering a de… · symfony/symfony@05d15cf · GitHub
[go: up one dir, main page]
Skip to content

Commit 05d15cf

Browse files
nicolas-grekasnicolas-grekas
committed
feature #22185 [DI] Enhance DX by throwing instead of triggering a deprecation notice (nicolas-grekas)
This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Enhance DX by throwing instead of triggering a deprecation notice | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | yes - at the config file level, for edge cases | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22143 | License | MIT | Doc PR | - Looking at the linked issue, I'm reconsidering our decision to trigger a deprecation notice when one uses `_instanceof` or `_defaults` as a service name. While on the BC side, this is strict - on the DX side, it looks like this opens a trap where people fill fall into. The same occurs to me with named args: instead of silently accepting invalid args as was the case before, let's throw to help DX when people do mistakes. Last change in this PR: the complex logic required to force strings to be given as `$id` args into `Reference` or `Alias` makes no sense to me, especially considering that a `string` type hint on PHP7 will *do* a string cast. Commits ------- b07da3d [DI] Enhance DX by throwing instead of triggering a deprecation notice
2 parents 83a0aa3 + b07da3d commit 05d15cf

File tree

8 files changed

+31
-50
lines changed

8 files changed

+31
-50
lines changed

‎UPGRADE-3.3.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ Debug
8080
DependencyInjection
8181
-------------------
8282

83+
* [BC BREAK] `_defaults` and `_instanceof` are now reserved service names in Yaml configurations. Please rename any services with that names.
84+
85+
* [BC BREAK] non-numeric keys in methods and constructors arguments have never been supported and are now forbidden. Please remove them if you happen to have one.
86+
87+
* Service names that start with an underscore are deprecated in Yaml files and will be reserved in 4.0. Please rename any services with such names.
88+
8389
* Autowiring-types have been deprecated, use aliases instead.
8490

8591
Before:

‎UPGRADE-4.0.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ Debug
7373
DependencyInjection
7474
-------------------
7575

76+
* `_defaults` and `_instanceof` are now reserved service names in Yaml configurations. Please rename any services with that names.
77+
78+
* Non-numeric keys in methods and constructors arguments have never been supported and are now forbidden. Please remove them if you happen to have one.
79+
80+
* Service names that start with an underscore are now reserved in Yaml files. Please rename any services with such names.
81+
7682
* Autowiring-types have been removed, use aliases instead.
7783

7884
Before:

‎src/Symfony/Component/DependencyInjection/Alias.php‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,7 @@ class Alias
2222
*/
2323
public function __construct($id, $public = true)
2424
{
25-
if (!is_string($id)) {
26-
$type = is_object($id) ? get_class($id) : gettype($id);
27-
$id = (string) $id;
28-
@trigger_error(sprintf('Non-string identifiers are deprecated since Symfony 3.3 and won\'t be supported in 4.0 for Alias to "%s" ("%s" given.) Cast it to string beforehand.', $id, $type), E_USER_DEPRECATED);
29-
}
30-
$this->id = $id;
25+
$this->id = (string) $id;
3126
$this->public = $public;
3227
}
3328

‎src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ protected function processValue($value, $isRoot = false)
4646
$resolvedArguments = array();
4747

4848
foreach ($arguments as $key => $argument) {
49-
if (is_int($key) || '' === $key || '$' !== $key[0]) {
50-
if (!is_int($key)) {
51-
@trigger_error(sprintf('Using key "%s" for defining arguments of method "%s" for service "%s" is deprecated since Symfony 3.3 and will throw an exception in 4.0. Use no keys or $named arguments instead.', $key, $method, $this->currentId), E_USER_DEPRECATED);
52-
}
49+
if (is_int($key)) {
5350
$resolvedArguments[] = $argument;
5451
continue;
5552
}
53+
if ('' === $key || '$' !== $key[0]) {
54+
throw new InvalidArgumentException(sprintf('Invalid key "%s" found in arguments of method "%s" for service "%s": only integer or $named arguments are allowed.', $key, $method, $this->currentId));
55+
}
5656

5757
$parameters = null !== $parameters ? $parameters : $this->getParameters($class, $method);
5858

‎src/Symfony/Component/DependencyInjection/Container.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,9 +469,7 @@ protected function getEnv($name)
469469
public function normalizeId($id)
470470
{
471471
if (!is_string($id)) {
472-
$type = is_object($id) ? get_class($id) : gettype($id);
473472
$id = (string) $id;
474-
@trigger_error(sprintf('Non-string service identifiers are deprecated since Symfony 3.3 and won\'t be supported in 4.0 for service "%s" ("%s" given.) Cast it to string beforehand.', $id, $type), E_USER_DEPRECATED);
475473
}
476474
if (isset($this->normalizedIds[$normalizedId = strtolower($id)])) {
477475
$normalizedId = $this->normalizedIds[$normalizedId];

‎src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php‎

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,16 @@ private function parseDefinitions(array $content, $file)
205205
throw new InvalidArgumentException(sprintf('The "services" key should contain an array in %s. Check your YAML syntax.', $file));
206206
}
207207

208-
if ($this->isUnderscoredParamValid($content, '_instanceof', $file)) {
208+
if (isset($content['services']['_instanceof'])) {
209+
$instanceof = $content['services']['_instanceof'];
210+
unset($content['services']['_instanceof']);
211+
212+
if (!is_array($instanceof)) {
213+
throw new InvalidArgumentException(sprintf('Service "_instanceof" key must be an array, "%s" given in "%s".', gettype($instanceof), $file));
214+
}
209215
$this->instanceof = array();
210216
57A7 $this->isLoadingInstanceof = true;
211-
foreach ($content['services']['_instanceof'] as $id => $service) {
217+
foreach ($instanceof as $id => $service) {
212218
if (!$service || !is_array($service)) {
213219
throw new InvalidArgumentException(sprintf('Type definition "%s" must be a non-empty array within "_instanceof" in %s. Check your YAML syntax.', $id, $file));
214220
}
@@ -217,7 +223,6 @@ private function parseDefinitions(array $content, $file)
217223
}
218224
$this->parseDefinition($id, $service, $file, array());
219225
}
220-
unset($content['services']['_instanceof']);
221226
}
222227

223228
$this->isLoadingInstanceof = false;
@@ -237,13 +242,16 @@ private function parseDefinitions(array $content, $file)
237242
*/
238243
private function parseDefaults(array &$content, $file)
239244
{
240-
if (!$this->isUnderscoredParamValid($content, '_defaults', $file)) {
245+
if (!isset($content['services']['_defaults'])) {
241246
return array();
242247
}
243-
244248
$defaults = $content['services']['_defaults'];
245249
unset($content['services']['_defaults']);
246250

251+
if (!is_array($defaults)) {
252+
throw new InvalidArgumentException(sprintf('Service "_defaults" key must be an array, "%s" given in "%s".', gettype($defaults), $file));
253+
}
254+
247255
foreach ($defaults as $key => $default) {
248256
if (!isset(self::$defaultsKeywords[$key])) {
249257
throw new InvalidArgumentException(sprintf('The configuration key "%s" cannot be used to define a default value in "%s". Allowed keys are "%s".', $key, $file, implode('", "', self::$defaultsKeywords)));
@@ -281,21 +289,6 @@ private function parseDefaults(array &$content, $file)
281289
return $defaults;
282290
}
283291

284-
private function isUnderscoredParamValid($content, $name, $file)
285-
{
286-
if (!isset($content['services'][$name])) {
287-
return false;
288-
}
289-
290-
if (!is_array($underscoreParam = $content['services'][$name])) {
291-
throw new InvalidArgumentException(sprintf('Service "%s" key must be an array, "%s" given in "%s".', $name, gettype($underscoreParam), $file));
292-
}
293-
294-
// @deprecated condition, to be removed in 4.0
295-
296-
return !isset($underscoreParam['alias']) && !isset($underscoreParam['class']) && !isset($underscoreParam['factory']);
297-
}
298-
299292
/**
300293
* @param array $service
301294
*

‎src/Symfony/Component/DependencyInjection/Reference.php‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,7 @@ class Reference
2929
*/
3030
public function __construct($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE)
3131
{
32-
if (!is_string($id)) {
33-
$type = is_object($id) ? get_class($id) : gettype($id);
34-
$id = (string) $id;
35-
@trigger_error(sprintf('Non-string identifiers are deprecated since Symfony 3.3 and won\'t be supported in 4.0 for Reference to "%s" ("%s" given.) Cast it to string beforehand.', $id, $type), E_USER_DEPRECATED);
36-
}
37-
$this->id = $id;
32+
$this->id = (string) $id;
3833
$this->invalidBehavior = $invalidBehavior;
3934
}
4035

‎src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -229,18 +229,6 @@ public function testGetInsensitivity()
229229
$this->assertSame($foo, $sc->get('Foo'), '->get() returns the service for the given id, and converts id to lowercase');
230230
}
231231

232-
/**
233-
* @group legacy
234-
* @expectedDeprecation Non-string service identifiers are deprecated since Symfony 3.3 and won't be supported in 4.0 for service "foo" ("Symfony\Component\DependencyInjection\Alias" given.) Cast it to string beforehand.
235-
* @expectedDeprecation Service identifiers will be made case sensitive in Symfony 4.0. Using "Foo" instead of "foo" is deprecated since version 3.3.
236-
*/
237-
public function testNonStringNormalizeId()
238-
{
239-
$sc = new ProjectServiceContainer();
240-
$this->assertSame('foo', $sc->normalizeId(new Alias('foo')));
241-
$this->assertSame('foo', $sc->normalizeId('Foo'));
242-
}
243-
244232
/**
245233
* @group legacy
246234
* @expectedDeprecation Service identifiers will be made case sensitive in Symfony 4.0. Using "foo" instead of "Foo" is deprecated since version 3.3.

0 commit comments

Comments
 (0)
0