[DI] Incorrect service id report for non-existent services

[edefimov]

Symfony version(s) affected: 4.0.14 and above

Description
Imagine there are two services A and B in the container. The A service depends on non-existent service X, the B service depends on A. If the A is declared as public=false, then container compilation reports, that service B has requested non-existent service X. If the A service is public=true, the report message will be correct.

How to reproduce
Try to compile container with such definition:

<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
           xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
    <services>
        <service id="AppBundle\Service\Child" public="false">
            <argument type="service" id="serializer" /> <!-- Missed service -->
        </service>

        <service id="AppBundle\Service\ParentA" public="true">
            <argument type="service" id="AppBundle\Service\Child" />
        </service>
    </services>
</container>

Reproducing application: https://github.com/edefimov/symfony-reproduce/tree/eea1edabc1ab944603cf773d570e51ed0357f37d/

Additional context
The output when service AppBundle\Service\Child is public=false

In CheckExceptionOnInvalidReferenceBehaviorPass.php line 31:
                                                                                                    
  The service "AppBundle\Service\ParentA" has a dependency on a non-existent service "serializer".                                                                                                 

Correct output when service AppBundle\Service\Child is public=true

In CheckExceptionOnInvalidReferenceBehaviorPass.php line 31:
                                                                                                  
  The service "AppBundle\Service\Child" has a dependency on a non-existent service "serializer".                                  

[xabbuh]

Does this also happen with 4.1.8?

[edefimov]

@xabbuh , Yes 4.1.8 also affected this behaviour

[nicolas-grekas]

That's normal behavior: when a service is private and is unused, it is removed. Throwing an error because it misses a dependency would make no sense. When the service is made public, this dependency must be defined so it throws.

[edefimov]

In this reproducing application all services are used: the final service AppBundle\Service\ParentA is public and requires a private service with missed dependency.

[nicolas-grekas]

OK, then the behavior maps to inlining: Child is inlined into ParentA so that ParentA ends up referencing serializer.

[edefimov]

In actual application the service chain is much more longer than this two services. And when the report says that missed dependency in the top-most service, to find out exact service causes an exception will look as hell.

[stof]

@nicolas-grekas would it make sense to split the logic into several passes ? First remove any unused private service (repeated pass, as today), then warn about invalid references, and then perform the inlining and another removal pass (simple pass, for services becoming unused because they were inlined, or maybe even removed by the inlining pass itself) ?
This way, warning about invalid references could still report the right id, as the inlining is not done yet, but we preserve the semantic of not warning for unused services.

Currently, removing unused services and inlining services is done by the same RepeatedPass IIRC, but I don't think that's necessary. Inlining won't create any new unused services (it will remove some service ids when inlining used services).

[mmoreram]

@nicolas-grekas then... https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Resources/config/test.xml

I updated last versions for Symfony some minutes ago, and tests started to fail. This is because I use the service inside them, and seems that a compiler pass named TestServiceContainerWeakRefPass seems to delete one of the dependencies...

Update -
It started failing with 4.2.0, where the compiler pass does not exist yet