-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DI] Deprecate case insentivity of service identifiers #21223
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,6 +70,11 @@ class Container implements ResettableContainerInterface | |
protected $aliases = array(); | ||
protected $loading = array(); | ||
|
||
/** | ||
* @internal | ||
*/ | ||
protected $normalizedIds = array(); | ||
|
||
private $underscoreMap = array('_' => '', '.' => '_', '\\' => '_'); | ||
private $envCache = array(); | ||
|
||
|
@@ -164,7 +169,7 @@ public function setParameter($name, $value) | |
*/ | ||
public function set($id, $service) | ||
{ | ||
$id = strtolower($id); | ||
$id = $this->normalizeId($id); | ||
|
||
if ('service_container' === $id) { | ||
throw new InvalidArgumentException('You cannot set service "service_container".'); | ||
|
@@ -215,8 +220,8 @@ public function has($id) | |
return true; | ||
} | ||
|
||
if (--$i && $id !== $lcId = strtolower($id)) { | ||
$id = $lcId; | ||
if (--$i && $id !== $normalizedId = $this->normalizeId($id)) { | ||
$id = $normalizedId; | ||
continue; | ||
} | ||
|
||
|
@@ -254,7 +259,7 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE | |
// Attempt to retrieve the service by checking first aliases then | ||
// available services. Service IDs are case insensitive, however since | ||
// this method can be called thousands of times during a request, avoid | ||
// calling strtolower() unless necessary. | ||
// calling $this->normalizeId($id) unless necessary. | ||
for ($i = 2;;) { | ||
if ('service_container' === $id) { | ||
return $this; | ||
|
@@ -273,8 +278,8 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE | |
|
||
if (isset($this->methodMap[$id])) { | ||
$method = $this->methodMap[$id]; | ||
} elseif (--$i && $id !== $lcId = strtolower($id)) { | ||
$id = $lcId; | ||
} elseif (--$i && $id !== $normalizedId = $this->normalizeId($id)) { | ||
$id = $normalizedId; | ||
continue; | ||
} elseif (!$this->methodMap && !$this instanceof ContainerBuilder && __CLASS__ !== static::class && method_exists($this, $method = 'get'.strtr($id, $this->underscoreMap).'Service')) { | ||
// We only check the convention-based factory in a compiled container (i.e. a child class other than a ContainerBuilder, | ||
|
@@ -329,7 +334,7 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE | |
*/ | ||
public function initialized($id) | ||
{ | ||
$id = strtolower($id); | ||
$id = $this->normalizeId($id); | ||
|
||
if ('service_container' === $id) { | ||
return false; | ||
|
@@ -426,6 +431,34 @@ protected function getEnv($name) | |
return $this->envCache[$name] = $this->getParameter("env($name)"); | ||
} | ||
|
||
/** | ||
* Returns the case sensitive id used at registration time. | ||
* | ||
* @param string $id | ||
* | ||
* @return string | ||
* | ||
* @internal | ||
*/ | ||
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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At this point we dont really know what the case of the non-string-service-id is right? Meaning this deprecation can be a false positive? Ie. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should read the deprecation message twice, same for your other comments :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Holy crap.. i got it :) Sorry. |
||
} | ||
if (isset($this->normalizedIds[$normalizedId = strtolower($id)])) { | ||
$normalizedId = $this->normalizedIds[$normalizedId]; | ||
if ($id !== $normalizedId) { | ||
@trigger_error(sprintf('Service identifiers will be made case sensitive in Symfony 4.0. Using "%s" instead of "%s" is deprecated since version 3.3.', $id, $normalizedId), E_USER_DEPRECATED); | ||
} | ||
} else { | ||
$normalizedId = $this->normalizedIds[$normalizedId] = $id; | ||
} | ||
|
||
return $normalizedId; | ||
} | ||
|
||
private function __clone() | ||
{ | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need this? The docblock already specifies that it must be string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what was said also about container->get, then I had to explicitly add string casts in our own tests. So, yes: a BC layer is a friendly default policy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, fine for me :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicolas-grekas just finished mine :).. probably related. Not sure i follow.. but ill dig in :)