bug symfony#28760 [DI] fix dumping inline services again (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[DI] fix dumping inline services again

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony#28720 symfony#28710
| License       | MIT
| Doc PR        | -

The main fix is in `PhpDumper::addInlineService()`
Fixes a few other glitches found meanwhile.

Commits
-------

a854b0a [DI] fix dumping inline services again

Author: nicolas-grekas

src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

@@ -503,10 +503,6 @@ private function isTrivialInstance(Definition $definition)
             }
         }
 
-        if (false !== strpos($this->dumpLiteralClass($this->dumpValue($definition->getClass())), '$')) {
-            return false;
-        }
-
         return true;
     }
 
@@ -598,18 +594,16 @@ private function addService($id, Definition $definition, &$file = null)
         $return = array();
 
         if ($class = $definition->getClass()) {
-            $class = $this->container->resolveEnvPlaceholders($class);
+            $class = $class instanceof Parameter ? '%'.$class.'%' : $this->container->resolveEnvPlaceholders($class);
             $return[] = sprintf(0 === strpos($class, '%') ? '@return object A %1$s instance' : '@return \%s', ltrim($class, '\\'));
         } elseif ($definition->getFactory()) {
             $factory = $definition->getFactory();
             if (\is_string($factory)) {
                 $return[] = sprintf('@return object An instance returned by %s()', $factory);
             } elseif (\is_array($factory) && (\is_string($factory[0]) || $factory[0] instanceof Definition || $factory[0] instanceof Reference)) {
-                if (\is_string($factory[0]) || $factory[0] instanceof Reference) {
-                    $return[] = sprintf('@return object An instance returned by %s::%s()', (string) $factory[0], $factory[1]);
-                } elseif ($factory[0] instanceof Definition) {
-                    $return[] = sprintf('@return object An instance returned by %s::%s()', $factory[0]->getClass(), $factory[1]);
-                }
+                $class = $factory[0] instanceof Definition ? $factory[0]->getClass() : (string) $factory[0];
+                $class = $class instanceof Parameter ? '%'.$class.'%' : $this->container->resolveEnvPlaceholders($class);
+                $return[] = sprintf('@return object An instance returned by %s::%s()', $class, $factory[1]);
             }
         }
 
@@ -704,7 +698,7 @@ private function addInlineVariables(&$head, &$tail, $id, array $arguments, $forC
             if (\is_array($argument)) {
                 $hasSelfRef = $this->addInlineVariables($head, $tail, $id, $argument, $forConstructor) || $hasSelfRef;
             } elseif ($argument instanceof Reference) {
-                $hasSelfRef = $this->addInlineReference($head, $tail, $id, $this->container->normalizeId($argument), $forConstructor) || $hasSelfRef;
+                $hasSelfRef = $this->addInlineReference($head, $id, $this->container->normalizeId($argument), $forConstructor) || $hasSelfRef;
             } elseif ($argument instanceof Definition) {
                 $hasSelfRef = $this->addInlineService($head, $tail, $id, $argument, $forConstructor) || $hasSelfRef;
             }
@@ -713,37 +707,31 @@ private function addInlineVariables(&$head, &$tail, $id, array $arguments, $forC
         return $hasSelfRef;
     }
 
-    private function addInlineReference(&$head, &$tail, $id, $targetId, $forConstructor)
+    private function addInlineReference(&$code, $id, $targetId, $forConstructor)
     {
+        $hasSelfRef = isset($this->circularReferences[$id][$targetId]);
+
         if ('service_container' === $targetId || isset($this->referenceVariables[$targetId])) {
-            return isset($this->circularReferences[$id][$targetId]);
+            return $hasSelfRef;
         }
 
         list($callCount, $behavior) = $this->serviceCalls[$targetId];
 
-        if (2 > $callCount && (!$forConstructor || !isset($this->circularReferences[$id][$targetId]))) {
-            return isset($this->circularReferences[$id][$targetId]);
+        if (2 > $callCount && (!$hasSelfRef || !$forConstructor)) {
+            return $hasSelfRef;
         }
 
         $name = $this->getNextVariableName();
         $this->referenceVariables[$targetId] = new Variable($name);
 
         $reference = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE >= $behavior ? new Reference($targetId, $behavior) : null;
-        $code = sprintf("        \$%s = %s;\n", $name, $this->getServiceCall($targetId, $reference));
-
-        if (!isset($this->circularReferences[$id][$targetId])) {
-            $head .= $code;
+        $code .= sprintf("        \$%s = %s;\n", $name, $this->getServiceCall($targetId, $reference));
 
-            return false;
+        if (!$hasSelfRef || !$forConstructor) {
+            return $hasSelfRef;
         }
 
-        if (!$forConstructor) {
-            $tail .= $code;
-
-            return true;
-        }
-
-        $head .= $code.sprintf(<<<'EOTXT'
+        $code .= sprintf(<<<'EOTXT'
 
         if (isset($this->%s['%s'])) {
             return $this->%1$s['%2$s'];
@@ -766,17 +754,21 @@ private function addInlineService(&$head, &$tail, $id, Definition $definition, $
 
         $arguments = array($definition->getArguments(), $definition->getFactory());
 
-        if (2 > $this->inlinedDefinitions[$definition] && !$definition->getMethodCalls() && !$definition->getProperties() && !$definition->getConfigurator() && false === strpos($this->dumpValue($definition->getClass()), '$')) {
+        if (2 > $this->inlinedDefinitions[$definition] && !$definition->getMethodCalls() && !$definition->getProperties() && !$definition->getConfigurator()) {
             return $this->addInlineVariables($head, $tail, $id, $arguments, $forConstructor);
         }
 
         $name = $this->getNextVariableName();
         $this->definitionVariables[$definition] = new Variable($name);
 
         $code = '';
-        $hasSelfRef = $this->addInlineVariables($code, $tail, $id, $arguments, $forConstructor);
+        if ($forConstructor) {
+            $hasSelfRef = $this->addInlineVariables($code, $tail, $id, $arguments, $forConstructor);
+        } else {
+            $hasSelfRef = $this->addInlineVariables($code, $code, $id, $arguments, $forConstructor);
+        }
         $code .= $this->addNewInstance($definition, '$'.$name, ' = ', $id);
-        $hasSelfRef ? $tail .= ('' !== $tail ? "\n" : '').$code : $head .= ('' !== $head ? "\n" : '').$code;
+        $hasSelfRef && !$forConstructor ? $tail .= ('' !== $tail ? "\n" : '').$code : $head .= ('' !== $head ? "\n" : '').$code;
 
         $code = '';
         $arguments = array($definition->getProperties(), $definition->getMethodCalls(), $definition->getConfigurator());

src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php

@@ -863,6 +863,28 @@ public function testDeepServiceGraph()
         $this->assertEquals((object) array('p2' => (object) array('p3' => (object) array())), $container->get('foo')->bClone);
     }
 
+    public function testInlineSelfRef()
+    {
+        $container = new ContainerBuilder();
+
+        $bar = (new Definition('App\Bar'))
+            ->setProperty('foo', new Reference('App\Foo'));
+
+        $baz = (new Definition('App\Baz'))
+            ->setProperty('bar', $bar)
+            ->addArgument($bar);
+
+        $container->register('App\Foo')
+            ->setPublic(true)
+            ->addArgument($baz);
+
+        $passConfig = $container->getCompiler()->getPassConfig();
+        $container->compile();
+
+        $dumper = new PhpDumper($container);
+        $this->assertStringEqualsFile(self::$fixturesPath.'/php/services_inline_self_ref.php', $dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Inline_Self_Ref')));
+    }
+
     public function testHotPathOptimizations()
     {
         $container = include self::$fixturesPath.'/containers/container_inline_requires.php';
@@ -940,6 +962,18 @@ public function testUninitializedSyntheticReference()
         $this->assertEquals((object) array('foo' => (object) array(123)), $container->get('bar'));
     }
 
+    public function testAdawsonContainer()
+    {
+        $container = new ContainerBuilder();
+        $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
+        $loader->load('services_adawson.yml');
+        $container->compile();
+
+        $dumper = new PhpDumper($container);
+        $dump = $dumper->dump();
+        $this->assertStringEqualsFile(self::$fixturesPath.'/php/services_adawson.php', $dumper->dump());
+    }
+
     /**
      * @group legacy
      * @expectedDeprecation The "private" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.

src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container19.php

@@ -7,9 +7,12 @@
 
 $container = new ContainerBuilder();
 
+$container->setParameter('env(FOO)', 'Bar\FaooClass');
+$container->setParameter('foo', '%env(FOO)%');
+
 $container
-    ->register('service_from_anonymous_factory', 'Bar\FooClass')
-    ->setFactory(array(new Definition('Bar\FooClass'), 'getInstance'))
+    ->register('service_from_anonymous_factory', '%foo%')
+    ->setFactory(array(new Definition('%foo%'), 'getInstance'))
     ->setPublic(true)
 ;
 

src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services19.php

@@ -21,6 +21,8 @@ class ProjectServiceContainer extends Container
 
     public function __construct()
     {
+        $this->parameters = $this->getDefaultParameters();
+
         $this->services = array();
         $this->methodMap = array(
             'service_from_anonymous_factory' => 'getServiceFromAnonymousFactoryService',
@@ -58,11 +60,11 @@ public function isFrozen()
     /**
      * Gets the public 'service_from_anonymous_factory' shared service.
      *
-     * @return \Bar\FooClass
+     * @return object A %env(FOO)% instance
      */
     protected function getServiceFromAnonymousFactoryService()
     {
-        return $this->services['service_from_anonymous_factory'] = (new \Bar\FooClass())->getInstance();
+        return $this->services['service_from_anonymous_factory'] = (new ${($_ = $this->getEnv('FOO')) && false ?: "_"}())->getInstance();
     }
 
     /**
@@ -78,4 +80,102 @@ protected function getServiceWithMethodCallAndFactoryService()
 
         return $instance;
     }
+
+    public function getParameter($name)
+    {
+        $name = (string) $name;
+        if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+            $name = $this->normalizeParameterName($name);
+
+            if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+                throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            }
+        }
+        if (isset($this->loadedDynamicParameters[$name])) {
+            return $this->loadedDynamicParameters[$name] ? $this->dynamicParameters[$name] : $this->getDynamicParameter($name);
+        }
+
+        return $this->parameters[$name];
+    }
+
+    public function hasParameter($name)
+    {
+        $name = (string) $name;
+        $name = $this->normalizeParameterName($name);
+
+        return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
+    }
+
+    public function setParameter($name, $value)
+    {
+        throw new LogicException('Impossible to call set() on a frozen ParameterBag.');
+    }
+
+    public function getParameterBag()
+    {
+        if (null === $this->parameterBag) {
+            $parameters = $this->parameters;
+            foreach ($this->loadedDynamicParameters as $name => $loaded) {
+                $parameters[$name] = $loaded ? $this->dynamicParameters[$name] : $this->getDynamicParameter($name);
+            }
+            $this->parameterBag = new FrozenParameterBag($parameters);
+        }
+
+        return $this->parameterBag;
+    }
+
+    private $loadedDynamicParameters = array(
+        'foo' => false,
+    );
+    private $dynamicParameters = array();
+
+    /**
+     * Computes a dynamic parameter.
+     *
+     * @param string The name of the dynamic parameter to load
+     *
+     * @return mixed The value of the dynamic parameter
+     *
+     * @throws InvalidArgumentException When the dynamic parameter does not exist
+     */
+    private function getDynamicParameter($name)
+    {
+        switch ($name) {
+            case 'foo': $value = $this->getEnv('FOO'); break;
+            default: throw new InvalidArgumentException(sprintf('The dynamic parameter "%s" must be defined.', $name));
+        }
+        $this->loadedDynamicParameters[$name] = true;
+
+        return $this->dynamicParameters[$name] = $value;
+    }
+
+    private $normalizedParameterNames = array(
+        'env(foo)' => 'env(FOO)',
+    );
+
+    private function normalizeParameterName($name)
+    {
+        if (isset($this->normalizedParameterNames[$normalizedName = strtolower($name)]) || isset($this->parameters[$normalizedName]) || array_key_exists($normalizedName, $this->parameters)) {
+            $normalizedName = isset($this->normalizedParameterNames[$normalizedName]) ? $this->normalizedParameterNames[$normalizedName] : $normalizedName;
+            if ((string) $name !== $normalizedName) {
+                @trigger_error(sprintf('Parameter names will be made case sensitive in Symfony 4.0. Using "%s" instead of "%s" is deprecated since Symfony 3.4.', $name, $normalizedName), E_USER_DEPRECATED);
+            }
+        } else {
+            $normalizedName = $this->normalizedParameterNames[$normalizedName] = (string) $name;
+        }
+
+        return $normalizedName;
+    }
+
+    /**
+     * Gets the default parameters.
+     *
+     * @return array An array of the default parameters
+     */
+    protected function getDefaultParameters()
+    {
+        return array(
+            'env(FOO)' => 'Bar\\FaooClass',
+        );
+    }
 }