bug #22790 [DependencyInjection] Fix dumping of RewindableGenerator with empty IteratorArgument (meyerbaptiste)

This PR was merged into the 3.3 branch. Discussion ---------- [DependencyInjection] Fix dumping of RewindableGenerator with empty IteratorArgument | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22780 | License | MIT | Doc PR | N/A According with https://github.com/symfony/symfony/issues/22780#issuecomment-302747395, when an `IteratorArgument` is empty, the closure generated by the `PhpDumper` should be `function () { return new EmptyIterator();` instead of `function () {}`, which is an invalid traversable for the `RewindableGenerator`. Commits ------- c2db0c1 [DependencyInjection] Fix dumping of RewindableGenerator with empty IteratorArgument
2017-05-21 10:44:23 +02:00 · 2017-05-21 10:44:23 +02:00 · 5c40e1264c
commit 5c40e1264c
parent 297fabaced c2db0c14e1
11 changed files with 62 additions and 26 deletions
--- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
+++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@ -1429,24 +1429,29 @@ EOF;
                }

                if ($value instanceof IteratorArgument) {
-                    $countCode = array();
-                    $countCode[] = 'function () {';
                    $operands = array(0);
-
                    $code = array();
                    $code[] = 'new RewindableGenerator(function () {';
-                    foreach ($value->getValues() as $k => $v) {
-                        ($c = $this->getServiceConditionals($v)) ? $operands[] = "(int) ($c)" : ++$operands[0];
-                        $v = $this->wrapServiceConditionals($v, sprintf("        yield %s => %s;\n", $this->dumpValue($k, $interpolate), $this->dumpValue($v, $interpolate)));
-                        foreach (explode("\n", $v) as $v) {
-                            if ($v) {
-                                $code[] = '    '.$v;
+
+                    if (!$values = $value->getValues()) {
+                        $code[] = '            return new \EmptyIterator();';
+                    } else {
+                        $countCode = array();
+                        $countCode[] = 'function () {';
+
+                        foreach ($values as $k => $v) {
+                            ($c = $this->getServiceConditionals($v)) ? $operands[] = "(int) ($c)" : ++$operands[0];
+                            $v = $this->wrapServiceConditionals($v, sprintf("        yield %s => %s;\n", $this->dumpValue($k, $interpolate), $this->dumpValue($v, $interpolate)));
+                            foreach (explode("\n", $v) as $v) {
+                                if ($v) {
+                                    $code[] = '    '.$v;
+                                }
                            }
                        }
-                    }

-                    $countCode[] = sprintf('            return %s;', implode(' + ', $operands));
-                    $countCode[] = '        }';
+                        $countCode[] = sprintf('            return %s;', implode(' + ', $operands));
+                        $countCode[] = '        }';
+                    }

                    $code[] = sprintf('        }, %s)', count($operands) > 1 ? implode("\n", $countCode) : $operands[0]);

--- a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
@ -449,12 +449,17 @@ class ContainerBuilderTest extends TestCase
        $builder->register('bar', 'stdClass');
        $builder
            ->register('lazy_context', 'LazyContext')
-            ->setArguments(array(new IteratorArgument(array('k1' => new Reference('bar'), new Reference('invalid', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)))))
+            ->setArguments(array(
+                new IteratorArgument(array('k1' => new Reference('bar'), new Reference('invalid', ContainerInterface::IGNORE_ON_INVALID_REFERENCE))),
+                new IteratorArgument(array()),
+            ))
        ;

        $lazyContext = $builder->get('lazy_context');
        $this->assertInstanceOf(RewindableGenerator::class, $lazyContext->lazyValues);
+        $this->assertInstanceOf(RewindableGenerator::class, $lazyContext->lazyEmptyValues);
        $this->assertCount(1, $lazyContext->lazyValues);
+        $this->assertCount(0, $lazyContext->lazyEmptyValues);

        $i = 0;
        foreach ($lazyContext->lazyValues as $k => $v) {
@ -465,6 +470,13 @@ class ContainerBuilderTest extends TestCase

        // The second argument should have been ignored.
        $this->assertEquals(1, $i);
+
+        $i = 0;
+        foreach ($lazyContext->lazyEmptyValues as $k => $v) {
+            ++$i;
+        }
+
+        $this->assertEquals(0, $i);
    }

    /**
--- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
@ -436,7 +436,10 @@ class PhpDumperTest extends TestCase
        $container->register('lazy_referenced', 'stdClass');
        $container
            ->register('lazy_context', 'LazyContext')
-            ->setArguments(array(new IteratorArgument(array('k1' => new Reference('lazy_referenced'), 'k2' => new Reference('service_container')))))
+            ->setArguments(array(
+                new IteratorArgument(array('k1' => new Reference('lazy_referenced'), 'k2' => new Reference('service_container'))),
+                new IteratorArgument(array()),
+            ))
        ;
        $container->compile();

@ -447,6 +450,9 @@ class PhpDumperTest extends TestCase
        $lazyContext = $container->get('lazy_context');

        $this->assertInstanceOf(RewindableGenerator::class, $lazyContext->lazyValues);
+        $this->assertInstanceOf(RewindableGenerator::class, $lazyContext->lazyEmptyValues);
+        $this->assertCount(2, $lazyContext->lazyValues);
+        $this->assertCount(0, $lazyContext->lazyEmptyValues);

        $i = -1;
        foreach ($lazyContext->lazyValues as $k => $v) {
@ -461,6 +467,8 @@ class PhpDumperTest extends TestCase
                    break;
            }
        }
+
+        $this->assertEmpty(iterator_to_array($lazyContext->lazyEmptyValues));
    }

    public function testClosureProxy()
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php
@ -134,11 +134,11 @@ $container
 ;
 $container
    ->register('lazy_context', 'LazyContext')
-    ->setArguments(array(new IteratorArgument(array('k1' => new Reference('foo.baz'), 'k2' => new Reference('service_container')))))
+    ->setArguments(array(new IteratorArgument(array('k1' => new Reference('foo.baz'), 'k2' => new Reference('service_container'))), new IteratorArgument(array())))
 ;
 $container
    ->register('lazy_context_ignore_invalid_ref', 'LazyContext')
-    ->setArguments(array(new IteratorArgument(array(new Reference('foo.baz'), new Reference('invalid', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)))))
+    ->setArguments(array(new IteratorArgument(array(new Reference('foo.baz'), new Reference('invalid', ContainerInterface::IGNORE_ON_INVALID_REFERENCE))), new IteratorArgument(array())))
 ;
 $container
    ->register('closure_proxy', 'BarClass')
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php
@ -102,9 +102,11 @@ class DummyProxyDumper implements ProxyDumper
 class LazyContext
 {
    public $lazyValues;
+    public $lazyEmptyValues;

-    public function __construct($lazyValues)
+    public function __construct($lazyValues, $lazyEmptyValues)
    {
        $this->lazyValues = $lazyValues;
+        $this->lazyEmptyValues = $lazyEmptyValues;
    }
 }
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php
@ -323,7 +323,9 @@ class ProjectServiceContainer extends Container
        return $this->services['lazy_context'] = new \LazyContext(new RewindableGenerator(function () {
            yield 'k1' => ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->get('foo.baz')) && false ?: '_'};
            yield 'k2' => $this;
-        }, 2));
+        }, 2), new RewindableGenerator(function () {
+            return new \EmptyIterator();
+        }, 0));
    }

    /**
@ -343,7 +345,9 @@ class ProjectServiceContainer extends Container
            }
        }, function () {
            return 1 + (int) ($this->has('invalid'));
-        }));
+        }), new RewindableGenerator(function () {
+            return new \EmptyIterator();
+        }, 0));
    }

    /**
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php
@ -326,7 +326,9 @@ class ProjectServiceContainer extends Container
        return $this->services['lazy_context'] = new \LazyContext(new RewindableGenerator(function () {
            yield 'k1' => ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->get('foo.baz')) && false ?: '_'};
            yield 'k2' => $this;
-        }, 2));
+        }, 2), new RewindableGenerator(function () {
+            return new \EmptyIterator();
+        }, 0));
    }

    /**
@ -341,7 +343,9 @@ class ProjectServiceContainer extends Container
    {
        return $this->services['lazy_context_ignore_invalid_ref'] = new \LazyContext(new RewindableGenerator(function () {
            yield 0 => ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->get('foo.baz')) && false ?: '_'};
-        }, 1));
+        }, 1), new RewindableGenerator(function () {
+            return new \EmptyIterator();
+        }, 0));
    }

    /**
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml
@ -121,12 +121,14 @@
        <argument key="k1" type="service" id="foo.baz"/>
        <argument key="k2" type="service" id="service_container"/>
      </argument>
+      <argument type="iterator"/>
    </service>
    <service id="lazy_context_ignore_invalid_ref" class="LazyContext">
      <argument type="iterator">
        <argument type="service" id="foo.baz"/>
        <argument type="service" id="invalid" on-invalid="ignore"/>
      </argument>
+      <argument type="iterator"/>
    </service>
    <service id="closure_proxy" class="BarClass">
      <argument type="closure-proxy" id="closure_proxy" method="getBaz"/>
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml
@ -112,10 +112,10 @@ services:
        factory: ['@factory_simple', getInstance]
    lazy_context:
        class: LazyContext
-        arguments: [!iterator {'k1': '@foo.baz', 'k2': '@service_container'}]
+        arguments: [!iterator {'k1': '@foo.baz', 'k2': '@service_container'}, !iterator []]
    lazy_context_ignore_invalid_ref:
        class: LazyContext
-        arguments: [!iterator ['@foo.baz', '@?invalid']]
+        arguments: [!iterator ['@foo.baz', '@?invalid'], !iterator []]
    closure_proxy:
        class: BarClass
        arguments: [!closure_proxy ['@closure_proxy', getBaz]]
--- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
@ -273,7 +273,7 @@ class XmlFileLoaderTest extends TestCase

        $lazyDefinition = $container->getDefinition('lazy_context');

-        $this->assertEquals(array(new IteratorArgument(array('k1' => new Reference('foo.baz'), 'k2' => new Reference('service_container')))), $lazyDefinition->getArguments(), '->load() parses lazy arguments');
+        $this->assertEquals(array(new IteratorArgument(array('k1' => new Reference('foo.baz'), 'k2' => new Reference('service_container'))), new IteratorArgument(array())), $lazyDefinition->getArguments(), '->load() parses lazy arguments');
    }

    public function testParsesTags()
@ -659,7 +659,6 @@ class XmlFileLoaderTest extends TestCase

        $this->assertFalse($container->getDefinition('no_defaults')->isAutowired());

-
        $this->assertTrue($container->getDefinition('child_def')->isPublic());
        $this->assertSame(array('foo' => array(array())), $container->getDefinition('child_def')->getTags());
        $this->assertFalse($container->getDefinition('child_def')->isAutowired());
--- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php
@ -346,7 +346,7 @@ class YamlFileLoaderTest extends TestCase

        $lazyDefinition = $container->getDefinition('lazy_context');

-        $this->assertEquals(array(new IteratorArgument(array('k1' => new Reference('foo.baz'), 'k2' => new Reference('service_container')))), $lazyDefinition->getArguments(), '->load() parses lazy arguments');
+        $this->assertEquals(array(new IteratorArgument(array('k1' => new Reference('foo.baz'), 'k2' => new Reference('service_container'))), new IteratorArgument(array())), $lazyDefinition->getArguments(), '->load() parses lazy arguments');
    }

    public function testAutowire()