Permalink
Browse files

feature #22185 [DI] Enhance DX by throwing instead of triggering a de…

…precation 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
  • Loading branch information...
nicolas-grekas committed Mar 28, 2017
2 parents 83a0aa3 + b07da3d commit 05d15cf4dfbfd8667a0b27cc8e889cbc6ad97fb1
View
@@ -80,6 +80,12 @@ Debug
DependencyInjection
-------------------
+ * [BC BREAK] `_defaults` and `_instanceof` are now reserved service names in Yaml configurations. Please rename any services with that names.
+
+ * [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.
+
+ * 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.
+
* Autowiring-types have been deprecated, use aliases instead.
Before:
View
@@ -73,6 +73,12 @@ Debug
DependencyInjection
-------------------
+ * `_defaults` and `_instanceof` are now reserved service names in Yaml configurations. Please rename any services with that names.
+
+ * 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.
+
+ * Service names that start with an underscore are now reserved in Yaml files. Please rename any services with such names.
+
* Autowiring-types have been removed, use aliases instead.
Before:
@@ -22,12 +22,7 @@ class Alias
*/
public function __construct($id, $public = true)
{
- if (!is_string($id)) {
- $type = is_object($id) ? get_class($id) : gettype($id);
- $id = (string) $id;
- @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);
- }
- $this->id = $id;
+ $this->id = (string) $id;
$this->public = $public;
}
@@ -46,13 +46,13 @@ protected function processValue($value, $isRoot = false)
$resolvedArguments = array();
foreach ($arguments as $key => $argument) {
- if (is_int($key) || '' === $key || '$' !== $key[0]) {
- if (!is_int($key)) {
- @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);
- }
+ if (is_int($key)) {
$resolvedArguments[] = $argument;
continue;
}
+ if ('' === $key || '$' !== $key[0]) {
+ 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));
+ }
$parameters = null !== $parameters ? $parameters : $this->getParameters($class, $method);
@@ -469,9 +469,7 @@ protected function getEnv($name)
public function normalizeId($id)
{
if (!is_string($id)) {
- $type = is_object($id) ? get_class($id) : gettype($id);
$id = (string) $id;
- @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);
}
if (isset($this->normalizedIds[$normalizedId = strtolower($id)])) {
$normalizedId = $this->normalizedIds[$normalizedId];
@@ -205,10 +205,16 @@ private function parseDefinitions(array $content, $file)
throw new InvalidArgumentException(sprintf('The "services" key should contain an array in %s. Check your YAML syntax.', $file));
}
- if ($this->isUnderscoredParamValid($content, '_instanceof', $file)) {
+ if (isset($content['services']['_instanceof'])) {
+ $instanceof = $content['services']['_instanceof'];
+ unset($content['services']['_instanceof']);
+
+ if (!is_array($instanceof)) {
+ throw new InvalidArgumentException(sprintf('Service "_instanceof" key must be an array, "%s" given in "%s".', gettype($instanceof), $file));
+ }
$this->instanceof = array();
$this->isLoadingInstanceof = true;
- foreach ($content['services']['_instanceof'] as $id => $service) {
+ foreach ($instanceof as $id => $service) {
if (!$service || !is_array($service)) {
throw new InvalidArgumentException(sprintf('Type definition "%s" must be a non-empty array within "_instanceof" in %s. Check your YAML syntax.', $id, $file));
}
@@ -217,7 +223,6 @@ private function parseDefinitions(array $content, $file)
}
$this->parseDefinition($id, $service, $file, array());
}
- unset($content['services']['_instanceof']);
}
$this->isLoadingInstanceof = false;
@@ -237,13 +242,16 @@ private function parseDefinitions(array $content, $file)
*/
private function parseDefaults(array &$content, $file)
{
- if (!$this->isUnderscoredParamValid($content, '_defaults', $file)) {
+ if (!isset($content['services']['_defaults'])) {
return array();
}
-
$defaults = $content['services']['_defaults'];
unset($content['services']['_defaults']);
+ if (!is_array($defaults)) {
+ throw new InvalidArgumentException(sprintf('Service "_defaults" key must be an array, "%s" given in "%s".', gettype($defaults), $file));
+ }
+
foreach ($defaults as $key => $default) {
if (!isset(self::$defaultsKeywords[$key])) {
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)
return $defaults;
}
- private function isUnderscoredParamValid($content, $name, $file)
- {
- if (!isset($content['services'][$name])) {
- return false;
- }
-
- if (!is_array($underscoreParam = $content['services'][$name])) {
- throw new InvalidArgumentException(sprintf('Service "%s" key must be an array, "%s" given in "%s".', $name, gettype($underscoreParam), $file));
- }
-
- // @deprecated condition, to be removed in 4.0
-
- return !isset($underscoreParam['alias']) && !isset($underscoreParam['class']) && !isset($underscoreParam['factory']);
- }
-
/**
* @param array $service
*
@@ -29,12 +29,7 @@ class Reference
*/
public function __construct($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE)
{
- if (!is_string($id)) {
- $type = is_object($id) ? get_class($id) : gettype($id);
- $id = (string) $id;
- @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);
- }
- $this->id = $id;
+ $this->id = (string) $id;
$this->invalidBehavior = $invalidBehavior;
}
@@ -229,18 +229,6 @@ public function testGetInsensitivity()
$this->assertSame($foo, $sc->get('Foo'), '->get() returns the service for the given id, and converts id to lowercase');
}
- /**
- * @group legacy
- * @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.
- * @expectedDeprecation Service identifiers will be made case sensitive in Symfony 4.0. Using "Foo" instead of "foo" is deprecated since version 3.3.
- */
- public function testNonStringNormalizeId()
- {
- $sc = new ProjectServiceContainer();
- $this->assertSame('foo', $sc->normalizeId(new Alias('foo')));
- $this->assertSame('foo', $sc->normalizeId('Foo'));
- }
-
/**
* @group legacy
* @expectedDeprecation Service identifiers will be made case sensitive in Symfony 4.0. Using "foo" instead of "Foo" is deprecated since version 3.3.

0 comments on commit 05d15cf

Please sign in to comment.