[DI] Shared services should not be inlined in non-shared ones #27265
Conversation
carsonbot
added
Status: Needs Review
DependencyInjection
Bug
Status: Reviewed
and removed
Status: Needs Review
labels
…nes (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [DI] Shared services should not be inlined in non-shared ones | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - [Already good on 2.7/2.8.](https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php#L152) Commits ------- 6f8b862 [DI] Shared services should not be inlined in non-shared ones
| @@ -138,6 +138,6 @@ private function isInlineableDefinition($id, Definition $definition, ServiceRefe | |||
| return false; | |||
| } | |||
|
|
|||
| return true; | |||
| return $this->container->getDefinition($ids[0])->isShared(); | |||
There was a problem hiding this comment.
Look like this change break the compilation with Undefined offset: 0 when the node doesn't have any InEdges
There was a problem hiding this comment.
hotfixed in 5eb17e5
but I don't understand how this can happen logically (we're here because we did find a reference to the service, to it must have at least one in-edge.)
could you share a reproduder?
There was a problem hiding this comment.
The use case is: I use a custom TestKernel which inject mocks in the container and set the service as Synthetic with a compiler pass
NB: Changing the type of the compiler pass to PassConfig::TYPE_AFTER_REMOVING (to run after InlineServiceDefinitionsPass) fix the issue
This minimal reproduct case is:
class Kernel extends BaseKernel implements CompilerPassInterface
{
// ...
public function process(ContainerBuilder $container)
{
// routing.resolver is just an example for the minimal reproduct case here
$container->getDefinition('routing.resolver')->setPublic(true)->setSynthetic(true);
}
}
There was a problem hiding this comment.
ok, so this means we are missing a check for synthetic service!
added in #27366
Already good on 2.7/2.8.