bug #21607 Improve tracking of environment variables in the case of private services (tgalopin)

This PR was merged into the 3.2 branch.

Discussion
----------

Improve tracking of environment variables in the case of private services

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21273
| License       | MIT

Recently, I stumbled upon an issue with environment variables that I had troubles to understand. Here it is:

I have two environment variables, defined in my `parameters.yml` with a default value of empty:

``` yaml
parameters:
    env(MAILJET_PUBLIC_KEY): ''
    env(MAILJET_PRIVATE_KEY): ''
```

I use these variables only in a private service:

``` xml
<service id="app.mailjet.transport" class="AppBundle\Mailjet\MailjetApiTransport" public="false">
    <argument type="service" id="csa_guzzle.client.mailjet_api"/>
    <argument>%env(MAILJET_PUBLIC_KEY)%</argument>
    <argument>%env(MAILJET_PRIVATE_KEY)%</argument>
</service>
```

This private service is not used by any other service for the moment.

As the service is private and not used by any other service, the `RemoveUnusedDefinitionsPass` removes it from the container. However, when the container dumper dumps the container, it checks if the environment variable was dumped. As the service is now gone, an expection is thrown (`EnvParameterException, Incompatible use of dynamic environment variables "MAILJET_PUBLIC_KEY", "MAILJET_PRIVATE_KEY" found in parameters.`).

Commits
-------

f380ba3 Improve tracking of environment variables in the case of private services

src/Symfony/Component/DependencyInjection/Compiler/RemoveUnusedDefinitionsPass.php

@@ -72,6 +72,7 @@ class RemoveUnusedDefinitionsPass implements RepeatablePassInterface
                 $compiler->addLogMessage($formatter->formatRemoveService($this, $id, 'replaces alias '.reset($referencingAliases)));
             } elseif (0 === count($referencingAliases) && false === $isReferenced) {
                 $container->removeDefinition($id);
+                $container->resolveEnvPlaceholders(serialize($definition));
                 $compiler->addLogMessage($formatter->formatRemoveService($this, $id, 'unused'));
                 $hasChanged = true;
             }

src/Symfony/Component/DependencyInjection/Tests/Compiler/RemoveUnusedDefinitionsPassTest.php

@@ -14,6 +14,7 @@ namespace Symfony\Component\DependencyInjection\Tests\Compiler;
 use Symfony\Component\DependencyInjection\Compiler\AnalyzeServiceReferencesPass;
 use Symfony\Component\DependencyInjection\Compiler\RepeatedPass;
 use Symfony\Component\DependencyInjection\Compiler\RemoveUnusedDefinitionsPass;
+use Symfony\Component\DependencyInjection\Compiler\ResolveParameterPlaceHoldersPass;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -105,6 +106,28 @@ class RemoveUnusedDefinitionsPassTest extends \PHPUnit_Framework_TestCase
         $this->assertTrue($container->hasDefinition('foobar'));
     }
 
+    public function testProcessConsiderEnvVariablesAsUsedEvenInPrivateServices()
+    {
+        $container = new ContainerBuilder();
+        $container->setParameter('env(FOOBAR)', 'test');
+        $container
+            ->register('foo')
+            ->setArguments(array('%env(FOOBAR)%'))
+            ->setPublic(false)
+        ;
+
+        $resolvePass = new ResolveParameterPlaceHoldersPass();
+        $resolvePass->process($container);
+
+        $this->process($container);
+
+        $this->assertFalse($container->hasDefinition('foo'));
+
+        $envCounters = $container->getEnvCounters();
+        $this->assertArrayHasKey('FOOBAR', $envCounters);
+        $this->assertSame(1, $envCounters['FOOBAR']);
+    }
+
     protected function process(ContainerBuilder $container)
     {
         $repeatedPass = new RepeatedPass(array(new AnalyzeServiceReferencesPass(), new RemoveUnusedDefinitionsPass()));