Skip to content

Commit

Permalink
minor #19608 [DependencyInjection] ContainerBuilder: Remove obsolete …
Browse files Browse the repository at this point in the history
…definitions (ogizanagi)

This PR was merged into the 3.1 branch.

Discussion
----------

[DependencyInjection] ContainerBuilder: Remove obsolete definitions

| Q             | A
| ------------- | ---
| Branch?       | 3.1
| Bug fix?      | yes
| New feature?  |no
| BC breaks?    | may be considered
| Deprecations? | may be considered
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

IIUC, [the obsolete definitions thing was tied to scoped & sync services](#7007).
Thus, this code [is not meant to be used since 3.0 and can be removed](#13289).

However, it may be considered as a BC break and would require introducing a new deprecation instead, because the current code allows to set a service on a frozen container if it has an obsolete synthetic definition...which is weird, isn't it ? (I doubt this feature was explicitly intended to allow setting a synthetic service more than once, and it wasn't before sync services introduction)

I suggest this as a patch for 3.1, under the pretext of forgotten code tied to a previous deprecation & feature removal, but let me know if it should be considered differently.

Commits
-------

daa7d00 [DependencyInjection] ContainerBuilder: Remove obsolete definitions
  • Loading branch information
fabpot committed Aug 15, 2016
2 parents 74b8561 + daa7d00 commit 9784301
Showing 1 changed file with 2 additions and 20 deletions.
22 changes: 2 additions & 20 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
Expand Up @@ -50,11 +50,6 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
*/
private $definitions = array();

/**
* @var Definition[]
*/
private $obsoleteDefinitions = array();

/**
* @var Alias[]
*/
Expand Down Expand Up @@ -346,21 +341,8 @@ public function set($id, $service)
{
$id = strtolower($id);

if ($this->isFrozen()) {
// setting a synthetic service on a frozen container is alright
if (
(!isset($this->definitions[$id]) && !isset($this->obsoleteDefinitions[$id]))
||
(isset($this->definitions[$id]) && !$this->definitions[$id]->isSynthetic())
||
(isset($this->obsoleteDefinitions[$id]) && !$this->obsoleteDefinitions[$id]->isSynthetic())
) {
throw new BadMethodCallException(sprintf('Setting service "%s" on a frozen container is not allowed.', $id));
}
}

if (isset($this->definitions[$id])) {
$this->obsoleteDefinitions[$id] = $this->definitions[$id];
if ($this->isFrozen() && (!isset($this->definitions[$id]) || !$this->definitions[$id]->isSynthetic())) {
throw new BadMethodCallException(sprintf('Setting service "%s" for an unknown or non-synthetic service definition on a frozen container is not allowed.', $id));
}

unset($this->definitions[$id], $this->aliasDefinitions[$id]);
Expand Down

0 comments on commit 9784301

Please sign in to comment.