Skip to content

[DependencyInjection] fixes #9815 Syntax error in PHP dumper #9816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 20, 2013
Merged

[DependencyInjection] fixes #9815 Syntax error in PHP dumper #9816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ private function addServiceConfigurator($id, $definition, $variableName = 'insta
$class = $this->dumpValue($callable[0]);
// If the class is a string we can optimize call_user_func away
if (strpos($class, "'") === 0) {
return sprintf(" \%s::%s(\$%s);\n", substr($class, 1, -1), $callable[1], $variableName);
return sprintf(" %s::%s(\$%s);\n", $this->dumpLiteralClass($class), $callable[1], $variableName);
}

return sprintf(" call_user_func(array(%s, '%s'), \$%s);\n", $this->dumpValue($callable[0]), $callable[1], $variableName);
Expand Down Expand Up @@ -701,7 +701,7 @@ private function addNewInstance($id, Definition $definition, $return, $instantia

// If the class is a string we can optimize call_user_func away
if (strpos($class, "'") === 0) {
return sprintf(" $return{$instantiation}\%s::%s(%s);\n", substr($class, 1, -1), $definition->getFactoryMethod(), $arguments ? implode(', ', $arguments) : '');
return sprintf(" $return{$instantiation}%s::%s(%s);\n", $this->dumpLiteralClass($class), $definition->getFactoryMethod(), $arguments ? implode(', ', $arguments) : '');
}

return sprintf(" $return{$instantiation}call_user_func(array(%s, '%s')%s);\n", $this->dumpValue($definition->getFactoryClass()), $definition->getFactoryMethod(), $arguments ? ', '.implode(', ', $arguments) : '');
Expand All @@ -718,7 +718,7 @@ private function addNewInstance($id, Definition $definition, $return, $instantia
return sprintf(" \$class = %s;\n\n $return{$instantiation}new \$class(%s);\n", $class, implode(', ', $arguments));
}

return sprintf(" $return{$instantiation}new \\%s(%s);\n", substr(str_replace('\\\\', '\\', $class), 1, -1), implode(', ', $arguments));
return sprintf(" $return{$instantiation}new %s(%s);\n", $this->dumpLiteralClass($class), implode(', ', $arguments));
}

/**
Expand Down Expand Up @@ -1248,6 +1248,18 @@ private function dumpValue($value, $interpolate = true)
}
}

/**
* Dumps a string to a literal (aka PHP Code) class value.
*
* @param string $class
*
* @return string
*/
private function dumpLiteralClass($class)
{
return '\\'.substr(str_replace('\\\\', '\\', $class), 1, -1);
}

/**
* Dumps a parameter
*
Expand Down
48 changes: 24 additions & 24 deletions src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public function testDefinitions()
{
$builder = new ContainerBuilder();
$definitions = array(
'foo' => new Definition('FooClass'),
'foo' => new Definition('Bar\FooClass'),
'bar' => new Definition('BarClass'),
);
$builder->setDefinitions($definitions);
Expand Down Expand Up @@ -69,7 +69,7 @@ public function testDefinitions()
public function testRegister()
{
$builder = new ContainerBuilder();
$builder->register('foo', 'FooClass');
$builder->register('foo', 'Bar\FooClass');
$this->assertTrue($builder->hasDefinition('foo'), '->register() registers a new service definition');
$this->assertInstanceOf('Symfony\Component\DependencyInjection\Definition', $builder->getDefinition('foo'), '->register() returns the newly created Definition instance');
}
Expand All @@ -81,7 +81,7 @@ public function testHas()
{
$builder = new ContainerBuilder();
$this->assertFalse($builder->has('foo'), '->has() returns false if the service does not exist');
$builder->register('foo', 'FooClass');
$builder->register('foo', 'Bar\FooClass');
$this->assertTrue($builder->has('foo'), '->has() returns true if a service definition exists');
$builder->set('bar', new \stdClass());
$this->assertTrue($builder->has('bar'), '->has() returns true if a service exists');
Expand Down Expand Up @@ -259,11 +259,11 @@ public function testAddGetCompilerPass()
public function testCreateService()
{
$builder = new ContainerBuilder();
$builder->register('foo1', 'FooClass')->setFile(__DIR__.'/Fixtures/includes/foo.php');
$this->assertInstanceOf('\FooClass', $builder->get('foo1'), '->createService() requires the file defined by the service definition');
$builder->register('foo2', 'FooClass')->setFile(__DIR__.'/Fixtures/includes/%file%.php');
$builder->register('foo1', 'Bar\FooClass')->setFile(__DIR__.'/Fixtures/includes/foo.php');
$this->assertInstanceOf('\Bar\FooClass', $builder->get('foo1'), '->createService() requires the file defined by the service definition');
$builder->register('foo2', 'Bar\FooClass')->setFile(__DIR__.'/Fixtures/includes/%file%.php');
$builder->setParameter('file', 'foo');
$this->assertInstanceOf('\FooClass', $builder->get('foo2'), '->createService() replaces parameters in the file provided by the service definition');
$this->assertInstanceOf('\Bar\FooClass', $builder->get('foo2'), '->createService() replaces parameters in the file provided by the service definition');
}

/**
Expand All @@ -273,13 +273,13 @@ public function testCreateProxyWithRealServiceInstantiator()
{
$builder = new ContainerBuilder();

$builder->register('foo1', 'FooClass')->setFile(__DIR__.'/Fixtures/includes/foo.php');
$builder->register('foo1', 'Bar\FooClass')->setFile(__DIR__.'/Fixtures/includes/foo.php');
$builder->getDefinition('foo1')->setLazy(true);

$foo1 = $builder->get('foo1');

$this->assertSame($foo1, $builder->get('foo1'), 'The same proxy is retrieved on multiple subsequent calls');
$this->assertSame('FooClass', get_class($foo1));
$this->assertSame('Bar\FooClass', get_class($foo1));
}

/**
Expand All @@ -300,7 +300,7 @@ public function testCreateServiceArguments()
{
$builder = new ContainerBuilder();
$builder->register('bar', 'stdClass');
$builder->register('foo1', 'FooClass')->addArgument(array('foo' => '%value%', '%value%' => 'foo', new Reference('bar'), '%%unescape_it%%'));
$builder->register('foo1', 'Bar\FooClass')->addArgument(array('foo' => '%value%', '%value%' => 'foo', new Reference('bar'), '%%unescape_it%%'));
$builder->setParameter('value', 'bar');
$this->assertEquals(array('foo' => 'bar', 'bar' => 'foo', $builder->get('bar'), '%unescape_it%'), $builder->get('foo1')->arguments, '->createService() replaces parameters and service references in the arguments provided by the service definition');
}
Expand All @@ -312,7 +312,7 @@ public function testCreateServiceFactoryMethod()
{
$builder = new ContainerBuilder();
$builder->register('bar', 'stdClass');
$builder->register('foo1', 'FooClass')->setFactoryClass('FooClass')->setFactoryMethod('getInstance')->addArgument(array('foo' => '%value%', '%value%' => 'foo', new Reference('bar')));
$builder->register('foo1', 'Bar\FooClass')->setFactoryClass('Bar\FooClass')->setFactoryMethod('getInstance')->addArgument(array('foo' => '%value%', '%value%' => 'foo', new Reference('bar')));
$builder->setParameter('value', 'bar');
$this->assertTrue($builder->get('foo1')->called, '->createService() calls the factory method to create the service instance');
$this->assertEquals(array('foo' => 'bar', 'bar' => 'foo', $builder->get('bar')), $builder->get('foo1')->arguments, '->createService() passes the arguments to the factory method');
Expand All @@ -337,7 +337,7 @@ public function testCreateServiceMethodCalls()
{
$builder = new ContainerBuilder();
$builder->register('bar', 'stdClass');
$builder->register('foo1', 'FooClass')->addMethodCall('setBar', array(array('%value%', new Reference('bar'))));
$builder->register('foo1', 'Bar\FooClass')->addMethodCall('setBar', array(array('%value%', new Reference('bar'))));
$builder->setParameter('value', 'bar');
$this->assertEquals(array('bar', $builder->get('bar')), $builder->get('foo1')->bar, '->createService() replaces the values in the method calls arguments');
}
Expand All @@ -348,23 +348,23 @@ public function testCreateServiceMethodCalls()
public function testCreateServiceConfigurator()
{
$builder = new ContainerBuilder();
$builder->register('foo1', 'FooClass')->setConfigurator('sc_configure');
$builder->register('foo1', 'Bar\FooClass')->setConfigurator('sc_configure');
$this->assertTrue($builder->get('foo1')->configured, '->createService() calls the configurator');

$builder->register('foo2', 'FooClass')->setConfigurator(array('%class%', 'configureStatic'));
$builder->register('foo2', 'Bar\FooClass')->setConfigurator(array('%class%', 'configureStatic'));
$builder->setParameter('class', 'BazClass');
$this->assertTrue($builder->get('foo2')->configured, '->createService() calls the configurator');

$builder->register('baz', 'BazClass');
$builder->register('foo3', 'FooClass')->setConfigurator(array(new Reference('baz'), 'configure'));
$builder->register('foo3', 'Bar\FooClass')->setConfigurator(array(new Reference('baz'), 'configure'));
$this->assertTrue($builder->get('foo3')->configured, '->createService() calls the configurator');

$builder->register('foo4', 'FooClass')->setConfigurator('foo');
$builder->register('foo4', 'Bar\FooClass')->setConfigurator('foo');
try {
$builder->get('foo4');
$this->fail('->createService() throws an InvalidArgumentException if the configure callable is not a valid callable');
} catch (\InvalidArgumentException $e) {
$this->assertEquals('The configure callable for class "FooClass" is not a callable.', $e->getMessage(), '->createService() throws an InvalidArgumentException if the configure callable is not a valid callable');
$this->assertEquals('The configure callable for class "Bar\FooClass" is not a callable.', $e->getMessage(), '->createService() throws an InvalidArgumentException if the configure callable is not a valid callable');
}
}

Expand All @@ -375,7 +375,7 @@ public function testCreateServiceConfigurator()
public function testCreateSyntheticService()
{
$builder = new ContainerBuilder();
$builder->register('foo', 'FooClass')->setSynthetic(true);
$builder->register('foo', 'Bar\FooClass')->setSynthetic(true);
$builder->get('foo');
}

Expand All @@ -384,7 +384,7 @@ public function testCreateServiceWithExpression()
$builder = new ContainerBuilder();
$builder->setParameter('bar', 'bar');
$builder->register('bar', 'BarClass');
$builder->register('foo', 'FooClass')->addArgument(array('foo' => new Expression('service("bar").foo ~ parameter("bar")')));
$builder->register('foo', 'Bar\FooClass')->addArgument(array('foo' => new Expression('service("bar").foo ~ parameter("bar")')));
$this->assertEquals('foobar', $builder->get('foo')->arguments['foo']);
}

Expand All @@ -394,7 +394,7 @@ public function testCreateServiceWithExpression()
public function testResolveServices()
{
$builder = new ContainerBuilder();
$builder->register('foo', 'FooClass');
$builder->register('foo', 'Bar\FooClass');
$this->assertEquals($builder->get('foo'), $builder->resolveServices(new Reference('foo')), '->resolveServices() resolves service references to service instances');
$this->assertEquals(array('foo' => array('foo', $builder->get('foo'))), $builder->resolveServices(array('foo' => array('foo', new Reference('foo')))), '->resolveServices() resolves service references to service instances in nested arrays');
$this->assertEquals($builder->get('foo'), $builder->resolveServices(new Expression('service("foo")')), '->resolveServices() resolves expressions');
Expand Down Expand Up @@ -427,7 +427,7 @@ public function testMerge()

$container = new ContainerBuilder();
$container->setResourceTracking(false);
$container->register('foo', 'FooClass');
$container->register('foo', 'Bar\FooClass');
$container->register('bar', 'BarClass');
$config = new ContainerBuilder();
$config->setDefinition('baz', new Definition('BazClass'));
Expand All @@ -441,7 +441,7 @@ public function testMerge()

$container = new ContainerBuilder();
$container->setResourceTracking(false);
$container->register('foo', 'FooClass');
$container->register('foo', 'Bar\FooClass');
$config->setDefinition('foo', new Definition('BazClass'));
$container->merge($config);
$this->assertEquals('BazClass', $container->getDefinition('foo')->getClass(), '->merge() overrides already defined services');
Expand All @@ -466,7 +466,7 @@ public function testfindTaggedServiceIds()
{
$builder = new ContainerBuilder();
$builder
->register('foo', 'FooClass')
->register('foo', 'Bar\FooClass')
->addTag('foo', array('foo' => 'foo'))
->addTag('bar', array('bar' => 'bar'))
->addTag('foo', array('foofoo' => 'foofoo'))
Expand All @@ -486,7 +486,7 @@ public function testfindTaggedServiceIds()
public function testFindDefinition()
{
$container = new ContainerBuilder();
$container->setDefinition('foo', $definition = new Definition('FooClass'));
$container->setDefinition('foo', $definition = new Definition('Bar\FooClass'));
$container->setAlias('bar', 'foo');
$container->setAlias('foobar', 'bar');
$this->assertEquals($definition, $container->findDefinition('foobar'), '->findDefinition() returns a Definition');
Expand Down
10 changes: 5 additions & 5 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@

$container = new ContainerBuilder();
$container->
register('foo', 'FooClass')->
register('foo', 'Bar\FooClass')->
addTag('foo', array('foo' => 'foo'))->
addTag('foo', array('bar' => 'bar'))->
setFactoryClass('FooClass')->
setFactoryClass('Bar\\FooClass')->
setFactoryMethod('getInstance')->
setArguments(array('foo', new Reference('foo.baz'), array('%foo%' => 'foo is %foo%', 'foobar' => '%foo%'), true, new Reference('service_container')))->
setProperties(array('foo' => 'bar', 'moo' => new Reference('foo.baz')))->
Expand All @@ -22,7 +22,7 @@
setConfigurator('sc_configure')
;
$container->
register('bar', 'FooClass')->
register('bar', 'Bar\FooClass')->
setArguments(array('foo', new Reference('foo.baz'), new Parameter('foo_bar')))->
setScope('container')->
setConfigurator(array(new Reference('foo.baz'), 'configure'))
Expand All @@ -40,13 +40,13 @@
$container->getParameterBag()->clear();
$container->getParameterBag()->add(array(
'baz_class' => 'BazClass',
'foo_class' => 'FooClass',
'foo_class' => 'Bar\FooClass',
'foo' => 'bar',
));
$container->setAlias('alias_for_foo', 'foo');
$container->setAlias('alias_for_alias', 'alias_for_foo');
$container->
register('method_call1', 'FooClass')->
register('method_call1', 'Bar\FooClass')->
setFile(realpath(__DIR__.'/../includes/foo.php'))->
addMethodCall('setBar', array(new Reference('foo')))->
addMethodCall('setBar', array(new Reference('foo2', ContainerInterface::NULL_ON_INVALID_REFERENCE)))->
Expand Down
8 changes: 4 additions & 4 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/graphviz/services9.dot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ digraph sc {
node [fontsize="11" fontname="Arial" shape="record"];
edge [fontsize="9" fontname="Arial" color="grey" arrowhead="open" arrowsize="0.5"];

node_foo [label="foo (alias_for_foo)\nFooClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
node_bar [label="bar\nFooClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
node_foo [label="foo (alias_for_foo)\nBar\\FooClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
node_bar [label="bar\nBar\\FooClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
node_foo_baz [label="foo.baz\nBazClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
node_foo_bar [label="foo_bar\nFooClass\n", shape=record, fillcolor="#eeeeee", style="dotted"];
node_method_call1 [label="method_call1\nFooClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
node_foo_bar [label="foo_bar\nBar\\FooClass\n", shape=record, fillcolor="#eeeeee", style="dotted"];
node_method_call1 [label="method_call1\nBar\\FooClass\n", shape=record, fillcolor="#eeeeee", style="filled"];
node_factory_service [label="factory_service\nBar\n", shape=record, fillcolor="#eeeeee", style="filled"];
node_foo_with_inline [label="foo_with_inline\nFoo\n", shape=record, fillcolor="#eeeeee", style="filled"];
node_inlined [label="inlined\nBar\n", shape=record, fillcolor="#eeeeee", style="filled"];
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/foo.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<?php

namespace Bar;

class FooClass
{
public $foo, $moo;
Expand Down
14 changes: 7 additions & 7 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@ public function __construct()
* This service is shared.
* This method always returns the same instance of the service.
*
* @return FooClass A FooClass instance.
* @return Bar\FooClass A Bar\FooClass instance.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here we want to do \Bar\FooClass\

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh nevermind i see a healthy container with

* @return Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntityValidator A Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntityValidator instance.

so no need i guess

*/
protected function getBarService()
{
$a = $this->get('foo.baz');

$this->services['bar'] = $instance = new \FooClass('foo', $a, $this->getParameter('foo_bar'));
$this->services['bar'] = $instance = new \Bar\FooClass('foo', $a, $this->getParameter('foo_bar'));

$a->configure($instance);

Expand Down Expand Up @@ -132,13 +132,13 @@ protected function getFactoryServiceService()
* This service is shared.
* This method always returns the same instance of the service.
*
* @return FooClass A FooClass instance.
* @return Bar\FooClass A Bar\FooClass instance.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

*/
protected function getFooService()
{
$a = $this->get('foo.baz');

$this->services['foo'] = $instance = \FooClass::getInstance('foo', $a, array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo').'', 'foobar' => $this->getParameter('foo')), true, $this);
$this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo').'', 'foobar' => $this->getParameter('foo')), true, $this);

$instance->setBar($this->get('bar'));
$instance->initialize();
Expand Down Expand Up @@ -201,13 +201,13 @@ protected function getFooWithInlineService()
* This service is shared.
* This method always returns the same instance of the service.
*
* @return FooClass A FooClass instance.
* @return Bar\FooClass A Bar\FooClass instance.
*/
protected function getMethodCall1Service()
{
require_once '%path%foo.php';

$this->services['method_call1'] = $instance = new \FooClass();
$this->services['method_call1'] = $instance = new \Bar\FooClass();

$instance->setBar($this->get('foo'));
$instance->setBar($this->get('foo2', ContainerInterface::NULL_ON_INVALID_REFERENCE));
Expand Down Expand Up @@ -297,7 +297,7 @@ protected function getDefaultParameters()
{
return array(
'baz_class' => 'BazClass',
'foo_class' => 'FooClass',
'foo_class' => 'Bar\\FooClass',
'foo' => 'bar',
);
}
Expand Down
Loading