ContainerBuilder: removeDefinition removes from classes too

[TomasVotruba]

Use case:

$containerBuilder->addDefintion('one')->setClass('stdClass');
$containerBuilder->addDefintion('two')->setClass('stdClass');
$containerBuilder->removeDefinition('two');
$containerBuilder->getByType('stdClass'); // breaks, because reference to 'two' is still saved under 'stdClass'

[fprochazka]

Nice addition. If you could also add tests, that would be awesome.

[TomasVotruba]

I'm on it.

[TomasVotruba]

It's ready now.

[fprochazka]

nice 👍

[dg]

What about $this->classes[$class][FALSE]?

[TomasVotruba]

@dg Added!

[dg]

All occurrences should be removed, not only in $this->classes[$class].

[TomasVotruba]

@dg What do you mean?

[dg]

for example $this->classes[get_parent_class($class)] etc.

[TomasVotruba]

@dg I still don't undestand the point. I'm not familiar with $classes content in such depth. Could you provide failing usecase?

[dg]

class A extends stdClass {}

$containerBuilder->addDefintion('two')->setClass('A');

[TomasVotruba]

This might do it. Please check.

Edit: other test is failing. Will check tomorrow

[dg]

foreach ($this->classes as $class => $tmp) 
   foreach ($tmp as $mode => $nm) {
     if ($name = $nm) unset($this->classes[$class][$tmp])
}

(cca, written on phone on a bike)

[dg]

Hmmm, and what about this? $this->classes[$class][$mode] = array_diff($this->classes[$class][$mode], [$name]).

[TomasVotruba]

I think it's less readable. It might be faster, container compilation is cached though.

[dg]

?

[TomasVotruba]

Dev code, I shall removed it.

[TomasVotruba]

Parfe!

[dg]

Thanx

[TomasVotruba]

One more related bug:

$someInterface => [
    0 => FirstService
    1 => SecondService
];

Remove 'FirstService':

$someInterface => [
    1 => SecondService
];

Then, getByType will try to get [0] => FAIL

Solution? Something like:

return reset($this->classes[$class][TRUE]);

What do you think?