diff --git a/src/Symfony/Component/DependencyInjection/Argument/IteratorArgument.php b/src/Symfony/Component/DependencyInjection/Argument/IteratorArgument.php index 5c35a68863..ab3a87900a 100644 --- a/src/Symfony/Component/DependencyInjection/Argument/IteratorArgument.php +++ b/src/Symfony/Component/DependencyInjection/Argument/IteratorArgument.php @@ -11,6 +11,9 @@ namespace Symfony\Component\DependencyInjection\Argument; +use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; +use Symfony\Component\DependencyInjection\Reference; + /** * Represents a collection of values to lazily iterate over. * @@ -20,9 +23,12 @@ class IteratorArgument implements ArgumentInterface { private $values; + /** + * @param Reference[] $values + */ public function __construct(array $values) { - $this->values = $values; + $this->setValues($values); } /** @@ -34,10 +40,16 @@ class IteratorArgument implements ArgumentInterface } /** - * @param array $values The values to lazily iterate over + * @param Reference[] $values The service references to lazily iterate over */ public function setValues(array $values) { + foreach ($values as $k => $v) { + if (null !== $v && !$v instanceof Reference) { + throw new InvalidArgumentException(sprintf('An IteratorArgument must hold only Reference instances, "%s" given.', is_object($v) ? get_class($v) : gettype($v))); + } + } + $this->values = $values; } } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php index 8f7a2fed3f..cec68cbb57 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/InlineServiceDefinitionsPass.php @@ -38,8 +38,7 @@ class InlineServiceDefinitionsPass extends AbstractRecursivePass implements Repe protected function processValue($value, $isRoot = false) { if ($value instanceof ArgumentInterface) { - $this->processValue($value->getValues()); - + // Reference found in ArgumentInterface::getValues() are not inlineable return $value; } if ($value instanceof Reference && $this->container->hasDefinition($id = (string) $value)) { diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 658331791c..910427b210 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -1135,8 +1135,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface return $this->resolveServices($reference); }; } elseif ($value instanceof IteratorArgument) { - $parameterBag = $this->getParameterBag(); - $value = new RewindableGenerator(function () use ($value, $parameterBag) { + $value = new RewindableGenerator(function () use ($value) { foreach ($value->getValues() as $k => $v) { foreach (self::getServiceConditionals($v) as $s) { if (!$this->has($s)) { @@ -1144,7 +1143,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface } } - yield $k => $this->resolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($v))); + yield $k => $this->resolveServices($v); } }, function () use ($value) { $count = 0; @@ -1444,7 +1443,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface * Shares a given service in the container. * * @param Definition $definition - * @param mixed $service + * @param object $service * @param string|null $id */ private function shareService(Definition $definition, $service, $id) diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index f8259008ac..bee7c47731 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -202,22 +202,20 @@ class PhpDumper extends Dumper /** * Generates Service local temp variables. * - * @param string $cId - * @param string $definition + * @param string $cId + * @param Definition $definition + * @param array $inlinedDefinitions * * @return string */ - private function addServiceLocalTempVariables($cId, $definition) + private function addServiceLocalTempVariables($cId, Definition $definition, array $inlinedDefinitions) { static $template = " \$%s = %s;\n"; - $localDefinitions = array_merge( - array($definition), - $this->getInlinedDefinitions($definition) - ); + array_unshift($inlinedDefinitions, $definition); $calls = $behavior = array(); - foreach ($localDefinitions as $iDefinition) { + foreach ($inlinedDefinitions as $iDefinition) { $this->getServiceCallsFromArguments($iDefinition->getArguments(), $calls, $behavior); $this->getServiceCallsFromArguments($iDefinition->getMethodCalls(), $calls, $behavior); $this->getServiceCallsFromArguments($iDefinition->getProperties(), $calls, $behavior); @@ -280,12 +278,13 @@ class PhpDumper extends Dumper /** * Generates the require_once statement for service includes. * - * @param string $id The service id + * @param string $id * @param Definition $definition + * @param array $inlinedDefinitions * * @return string */ - private function addServiceInclude($id, $definition) + private function addServiceInclude($id, Definition $definition, array $inlinedDefinitions) { $template = " require_once %s;\n"; $code = ''; @@ -294,7 +293,7 @@ class PhpDumper extends Dumper $code .= sprintf($template, $this->dumpValue($file)); } - foreach ($this->getInlinedDefinitions($definition) as $definition) { + foreach ($inlinedDefinitions as $definition) { if (null !== $file = $definition->getFile()) { $code .= sprintf($template, $this->dumpValue($file)); } @@ -310,21 +309,20 @@ class PhpDumper extends Dumper /** * Generates the inline definition of a service. * - * @param string $id - * @param Definition $definition + * @param string $id + * @param array $inlinedDefinitions * * @return string * * @throws RuntimeException When the factory definition is incomplete * @throws ServiceCircularReferenceException When a circular reference is detected */ - private function addServiceInlinedDefinitions($id, $definition) + private function addServiceInlinedDefinitions($id, array $inlinedDefinitions) { $code = ''; $variableMap = $this->definitionVariables; $nbOccurrences = new \SplObjectStorage(); $processed = new \SplObjectStorage(); - $inlinedDefinitions = $this->getInlinedDefinitions($definition); foreach ($inlinedDefinitions as $definition) { if (false === $nbOccurrences->contains($definition)) { @@ -375,14 +373,14 @@ class PhpDumper extends Dumper /** * Adds the service return statement. * - * @param string $id Service id - * @param Definition $definition + * @param string $id + * @param bool $isSimpleInstance * * @return string */ - private function addServiceReturn($id, $definition) + private function addServiceReturn($id, $isSimpleInstance) { - if ($this->isSimpleInstance($id, $definition)) { + if ($isSimpleInstance) { return " }\n"; } @@ -394,13 +392,14 @@ class PhpDumper extends Dumper * * @param string $id * @param Definition $definition + * @param bool $isSimpleInstance * * @return string * * @throws InvalidArgumentException * @throws RuntimeException */ - private function addServiceInstance($id, Definition $definition) + private function addServiceInstance($id, Definition $definition, $isSimpleInstance) { $class = $definition->getClass(); @@ -414,18 +413,17 @@ class PhpDumper extends Dumper throw new InvalidArgumentException(sprintf('"%s" is not a valid class name for the "%s" service.', $class, $id)); } - $simple = $this->isSimpleInstance($id, $definition); $isProxyCandidate = $this->getProxyDumper()->isProxyCandidate($definition); $instantiation = ''; if (!$isProxyCandidate && $definition->isShared()) { - $instantiation = "\$this->services['$id'] = ".($simple ? '' : '$instance'); - } elseif (!$simple) { + $instantiation = "\$this->services['$id'] = ".($isSimpleInstance ? '' : '$instance'); + } elseif (!$isSimpleInstance) { $instantiation = '$instance'; } $return = ''; - if ($simple) { + if ($isSimpleInstance) { $return = 'return '; } else { $instantiation .= ' = '; @@ -433,7 +431,7 @@ class PhpDumper extends Dumper $code = $this->addNewInstance($definition, $return, $instantiation, $id); - if (!$simple) { + if (!$isSimpleInstance) { $code .= "\n"; } @@ -500,20 +498,21 @@ class PhpDumper extends Dumper /** * Generates the inline definition setup. * - * @param string $id - * @param Definition $definition + * @param string $id + * @param array $inlinedDefinitions + * @param bool $isSimpleInstance * * @return string * * @throws ServiceCircularReferenceException when the container contains a circular reference */ - private function addServiceInlinedDefinitionsSetup($id, Definition $definition) + private function addServiceInlinedDefinitionsSetup($id, array $inlinedDefinitions, $isSimpleInstance) { $this->referenceVariables[$id] = new Variable('instance'); $code = ''; $processed = new \SplObjectStorage(); - foreach ($this->getInlinedDefinitions($definition) as $iDefinition) { + foreach ($inlinedDefinitions as $iDefinition) { if ($processed->contains($iDefinition)) { continue; } @@ -525,7 +524,7 @@ class PhpDumper extends Dumper // if the instance is simple, the return statement has already been generated // so, the only possible way to get there is because of a circular reference - if ($this->isSimpleInstance($id, $definition)) { + if ($isSimpleInstance) { throw new ServiceCircularReferenceException($id, array($id)); } @@ -683,16 +682,19 @@ EOF; $code .= sprintf(" @trigger_error(%s, E_USER_DEPRECATED);\n\n", $this->export($definition->getDeprecationMessage($id))); } + $inlinedDefinitions = $this->getInlinedDefinitions($definition); + $isSimpleInstance = $this->isSimpleInstance($id, $definition, $inlinedDefinitions); + $code .= - $this->addServiceInclude($id, $definition). - $this->addServiceLocalTempVariables($id, $definition). - $this->addServiceInlinedDefinitions($id, $definition). - $this->addServiceInstance($id, $definition). - $this->addServiceInlinedDefinitionsSetup($id, $definition). + $this->addServiceInclude($id, $definition, $inlinedDefinitions). + $this->addServiceLocalTempVariables($id, $definition, $inlinedDefinitions). + $this->addServiceInlinedDefinitions($id, $inlinedDefinitions). + $this->addServiceInstance($id, $definition, $isSimpleInstance). + $this->addServiceInlinedDefinitionsSetup($id, $inlinedDefinitions, $isSimpleInstance). $this->addServiceProperties($id, $definition). $this->addServiceMethodCalls($id, $definition). $this->addServiceConfigurator($id, $definition). - $this->addServiceReturn($id, $definition) + $this->addServiceReturn($id, $isSimpleInstance) ; $this->definitionVariables = null; @@ -1331,8 +1333,6 @@ EOF; foreach ($arguments as $argument) { if (is_array($argument)) { $definitions = array_merge($definitions, $this->getDefinitionsFromArguments($argument)); - } elseif ($argument instanceof ArgumentInterface) { - $definitions = array_merge($definitions, $this->getDefinitionsFromArguments($argument->getValues())); } elseif ($argument instanceof Definition) { $definitions = array_merge( $definitions, diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index c31fc7fd50..9f730472d7 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -490,7 +490,12 @@ class XmlFileLoader extends FileLoader $arguments[$key] = $this->getArgumentsAsPhp($arg, $name, false); break; case 'iterator': - $arguments[$key] = new IteratorArgument($this->getArgumentsAsPhp($arg, $name, false)); + $arg = $this->getArgumentsAsPhp($arg, $name, false); + try { + $arguments[$key] = new IteratorArgument($arg); + } catch (InvalidArgumentException $e) { + throw new InvalidArgumentException(sprintf('Tag "<%s>" with type="iterator" only accepts collections of type="service" references.', $name)); + } break; case 'string': $arguments[$key] = $arg->nodeValue; diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 735cb8244d..e7703aabf8 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -665,14 +665,18 @@ class YamlFileLoader extends FileLoader $argument = $value->getValue(); if ('iterator' === $value->getTag()) { if (!is_array($argument)) { - throw new InvalidArgumentException('"!iterator" tag only accepts sequences.'); + throw new InvalidArgumentException(sprintf('"!iterator" tag only accepts sequences in "%s".', $file)); + } + $argument = $this->resolveServices($argument, $file, $isParameter); + try { + return new IteratorArgument($argument); + } catch (InvalidArgumentException $e) { + throw new InvalidArgumentException(sprintf('"!iterator" tag only accepts arrays of "@service" references in "%s".', $file)); } - - return new IteratorArgument($this->resolveServices($argument, $file, $isParameter)); } if ('closure_proxy' === $value->getTag()) { if (!is_array($argument) || array(0, 1) !== array_keys($argument) || !is_string($argument[0]) || !is_string($argument[1]) || 0 !== strpos($argument[0], '@') || 0 === strpos($argument[0], '@@')) { - throw new InvalidArgumentException('"!closure_proxy" tagged values must be arrays of [@service, method].'); + throw new InvalidArgumentException(sprintf('"!closure_proxy" tagged values must be arrays of [@service, method] in "%s".', $file)); } if (0 === strpos($argument[0], '@?')) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php index 1ef7b8771b..88e57f30a5 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php @@ -434,7 +434,7 @@ class PhpDumperTest extends TestCase $container->register('lazy_referenced', 'stdClass'); $container ->register('lazy_context', 'LazyContext') - ->setArguments(array(new IteratorArgument(array('foo', new Reference('lazy_referenced'), 'k1' => array('foo' => 'bar'), true, 'k2' => new Reference('service_container'))))) + ->setArguments(array(new IteratorArgument(array('k1' => new Reference('lazy_referenced'), 'k2' => new Reference('service_container'))))) ; $container->compile(); @@ -450,24 +450,12 @@ class PhpDumperTest extends TestCase foreach ($lazyContext->lazyValues as $k => $v) { switch (++$i) { case 0: - $this->assertEquals(0, $k); - $this->assertEquals('foo', $v); + $this->assertEquals('k1', $k); + $this->assertInstanceOf('stdCLass', $v); break; case 1: - $this->assertEquals(1, $k); - $this->assertInstanceOf('stdClass', $v); - break; - case 2: - $this->assertEquals('k1', $k); - $this->assertEquals(array('foo' => 'bar'), $v); - break; - case 3: - $this->assertEquals(2, $k); - $this->assertTrue($v); - break; - case 4: $this->assertEquals('k2', $k); - $this->assertInstanceOf('\Symfony_DI_PhpDumper_Test_Lazy_Argument_Provide_Generator', $v); + $this->assertInstanceOf('Symfony_DI_PhpDumper_Test_Lazy_Argument_Provide_Generator', $v); break; } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php index 6d1cc0cb3a..9a18388be0 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php @@ -134,7 +134,7 @@ $container ; $container ->register('lazy_context', 'LazyContext') - ->setArguments(array(new IteratorArgument(array('foo', new Reference('foo.baz'), array('%foo%' => 'foo is %foo%', 'foobar' => '%foo%'), true, new Reference('service_container'))))) + ->setArguments(array(new IteratorArgument(array('k1' => new Reference('foo.baz'), 'k2' => new Reference('service_container'))))) ; $container ->register('lazy_context_ignore_invalid_ref', 'LazyContext') diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php index 80d3f69994..d59f315c2c 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php @@ -321,12 +321,9 @@ class ProjectServiceContainer extends Container protected function getLazyContextService() { return $this->services['lazy_context'] = new \LazyContext(new RewindableGenerator(function () { - yield 0 => 'foo'; - yield 1 => ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->get('foo.baz')) && false ?: '_'}; - yield 2 => array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo').'', 'foobar' => $this->getParameter('foo')); - yield 3 => true; - yield 4 => $this; - }, 5)); + yield 'k1' => ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->get('foo.baz')) && false ?: '_'}; + yield 'k2' => $this; + }, 2)); } /** diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php index 9ac0402f9d..f35ca89f03 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php @@ -324,12 +324,9 @@ class ProjectServiceContainer extends Container protected function getLazyContextService() { return $this->services['lazy_context'] = new \LazyContext(new RewindableGenerator(function () { - yield 0 => 'foo'; - yield 1 => ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->get('foo.baz')) && false ?: '_'}; - yield 2 => array('bar' => 'foo is bar', 'foobar' => 'bar'); - yield 3 => true; - yield 4 => $this; - }, 5)); + yield 'k1' => ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->get('foo.baz')) && false ?: '_'}; + yield 'k2' => $this; + }, 2)); } /** diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml index 60dd9a233b..792c515995 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml @@ -118,14 +118,8 @@ - foo - - - foo is %foo% - %foo% - - true - + + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml index 34402482ff..519567054e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml @@ -112,7 +112,7 @@ services: factory: ['@factory_simple', getInstance] lazy_context: class: LazyContext - arguments: [!iterator [foo, '@foo.baz', { '%foo%': 'foo is %foo%', foobar: '%foo%' }, true, '@service_container']] + arguments: [!iterator {'k1': '@foo.baz', 'k2': '@service_container'}] lazy_context_ignore_invalid_ref: class: LazyContext arguments: [!iterator ['@foo.baz', '@?invalid']] diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php index 5d97321195..3b6ddd7fcb 100644 --- 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('foo', new Reference('foo.baz'), array('%foo%' => 'foo is %foo%', 'foobar' => '%foo%'), true, 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')))), $lazyDefinition->getArguments(), '->load() parses lazy arguments'); } public function testParsesTags() diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index c0cf1e82cf..87a05eb2ab 100644 --- 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('foo', new Reference('foo.baz'), array('%foo%' => 'foo is %foo%', 'foobar' => '%foo%'), true, 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')))), $lazyDefinition->getArguments(), '->load() parses lazy arguments'); } public function testAutowire()