From 2612f810525fbd1e8be183a796146be7ad85044f Mon Sep 17 00:00:00 2001 From: Gary PEGEOT Date: Tue, 3 Apr 2018 14:45:35 +0200 Subject: [PATCH 1/4] Allow autoconfigured calls in PHP. --- .../ResolveInstanceofConditionalsPass.php | 13 ++++++++++--- .../ResolveInstanceofConditionalsPassTest.php | 16 +++++++++++----- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php index 15110261a2..f91cfd0667 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php @@ -33,9 +33,6 @@ class ResolveInstanceofConditionalsPass implements CompilerPassInterface if ($definition->getArguments()) { throw new InvalidArgumentException(sprintf('Autoconfigured instanceof for type "%s" defines arguments but these are not supported and should be removed.', $interface)); } - if ($definition->getMethodCalls()) { - throw new InvalidArgumentException(sprintf('Autoconfigured instanceof for type "%s" defines method calls but these are not supported and should be removed.', $interface)); - } } foreach ($container->getDefinitions() as $id => $definition) { @@ -64,6 +61,7 @@ class ResolveInstanceofConditionalsPass implements CompilerPassInterface $definition->setInstanceofConditionals(array()); $parent = $shared = null; $instanceofTags = array(); + $instanceofCalls = array(); foreach ($conditionals as $interface => $instanceofDefs) { if ($interface !== $class && (!$container->getReflectionClass($class, false))) { @@ -81,7 +79,13 @@ class ResolveInstanceofConditionalsPass implements CompilerPassInterface $parent = 'instanceof.'.$interface.'.'.$key.'.'.$id; $container->setDefinition($parent, $instanceofDef); $instanceofTags[] = $instanceofDef->getTags(); + + foreach ($instanceofDef->getMethodCalls() as $methodCall) { + $instanceofCalls[] = $methodCall; + } + $instanceofDef->setTags(array()); + $instanceofDef->setMethodCalls(array()); if (isset($instanceofDef->getChanges()['shared'])) { $shared = $instanceofDef->isShared(); @@ -98,6 +102,7 @@ class ResolveInstanceofConditionalsPass implements CompilerPassInterface $definition = serialize($definition); $definition = substr_replace($definition, '53', 2, 2); $definition = substr_replace($definition, 'Child', 44, 0); + /** @var ChildDefinition $definition */ $definition = unserialize($definition); $definition->setParent($parent); @@ -117,6 +122,8 @@ class ResolveInstanceofConditionalsPass implements CompilerPassInterface } } + $definition->setMethodCalls(array_merge($instanceofCalls, $definition->getMethodCalls())); + // reset fields with "merge" behavior $abstract ->setBindings($bindings) diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php index 21a2810578..a7167a3b31 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php @@ -200,16 +200,22 @@ class ResolveInstanceofConditionalsPassTest extends TestCase } /** - * @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException - * @expectedExceptionMessage Autoconfigured instanceof for type "PHPUnit\Framework\TestCase" defines method calls but these are not supported and should be removed. + * Test that autoconfigured calls are handled gracefully. */ - public function testProcessThrowsExceptionForAutoconfiguredCalls() + public function testProcessForAutoconfiguredCalls() { $container = new ContainerBuilder(); - $container->registerForAutoconfiguration(parent::class) - ->addMethodCall('setFoo'); + $container->registerForAutoconfiguration(parent::class)->addMethodCall('setLogger'); + + $def = $container->register('foo', self::class)->setAutoconfigured(true); + $this->assertFalse($def->hasMethodCall('setLogger'), 'Definition shouldn\'t have method call yet.'); (new ResolveInstanceofConditionalsPass())->process($container); + + $this->assertTrue( + $container->findDefinition('foo')->hasMethodCall('setLogger'), + 'Definition should have "setLogger" method call.' + ); } /** From 15c45ee40a8062726580edee81a742682a4edac7 Mon Sep 17 00:00:00 2001 From: Gary PEGEOT Date: Wed, 4 Apr 2018 00:54:08 +0200 Subject: [PATCH 2/4] Add more test-cases --- .../ResolveInstanceofConditionalsPassTest.php | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php index a7167a3b31..82774d2a49 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php @@ -205,17 +205,29 @@ class ResolveInstanceofConditionalsPassTest extends TestCase public function testProcessForAutoconfiguredCalls() { $container = new ContainerBuilder(); - $container->registerForAutoconfiguration(parent::class)->addMethodCall('setLogger'); - $def = $container->register('foo', self::class)->setAutoconfigured(true); - $this->assertFalse($def->hasMethodCall('setLogger'), 'Definition shouldn\'t have method call yet.'); + $expected = array( + array('setFoo', array( + 'plain_value', + '%some_parameter%' + )), + array('callBar', array()), + array('isBaz', array()), + ); + + $container->registerForAutoconfiguration(parent::class)->addMethodCall('setFoo', $expected[0][1]); + $container->registerForAutoconfiguration(self::class)->addMethodCall('callBar'); + + $def = $container->register('foo', self::class)->setAutoconfigured(true)->addMethodCall('isBaz'); + $this->assertEquals( + array(array('isBaz', array())), + $def->getMethodCalls(), + 'Definition shouldn\'t have only one method call.' + ); (new ResolveInstanceofConditionalsPass())->process($container); - $this->assertTrue( - $container->findDefinition('foo')->hasMethodCall('setLogger'), - 'Definition should have "setLogger" method call.' - ); + $this->assertEquals($expected, $container->findDefinition('foo')->getMethodCalls()); } /** From 71bf3ced74673d9e2035cea8bd5767492626d22d Mon Sep 17 00:00:00 2001 From: Gary PEGEOT Date: Wed, 4 Apr 2018 00:56:28 +0200 Subject: [PATCH 3/4] CS fix --- .../Tests/Compiler/ResolveInstanceofConditionalsPassTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php index 82774d2a49..45022a4117 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php @@ -209,7 +209,7 @@ class ResolveInstanceofConditionalsPassTest extends TestCase $expected = array( array('setFoo', array( 'plain_value', - '%some_parameter%' + '%some_parameter%', )), array('callBar', array()), array('isBaz', array()), From 1d811cb3f78871637931511ba8836a9872c1fd43 Mon Sep 17 00:00:00 2001 From: Gary PEGEOT Date: Tue, 10 Apr 2018 11:14:32 +0200 Subject: [PATCH 4/4] Add test for both _intanceof and manual method setting. --- .../Tests/Compiler/IntegrationTest.php | 10 ++++++++++ .../yaml/integration/instanceof_and_calls/expected.yml | 10 ++++++++++ .../yaml/integration/instanceof_and_calls/main.yml | 9 +++++++++ 3 files changed, 29 insertions(+) create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/instanceof_and_calls/expected.yml create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/instanceof_and_calls/main.yml diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php index e7fdb3593a..3dbec91eb8 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php @@ -207,6 +207,16 @@ class IntegrationTest extends TestCase 'child_service', 'child_service_expected', ); + + $container = new ContainerBuilder(); + $container->registerForAutoconfiguration(IntegrationTestStub::class) + ->addMethodCall('setSunshine', array('supernova')); + yield array( + 'instanceof_and_calls', + 'main_service', + 'main_service_expected', + $container, + ); } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/instanceof_and_calls/expected.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/instanceof_and_calls/expected.yml new file mode 100644 index 0000000000..2d485a7a24 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/instanceof_and_calls/expected.yml @@ -0,0 +1,10 @@ +services: + # main_service should look like this in the end + main_service_expected: + class: Symfony\Component\DependencyInjection\Tests\Compiler\IntegrationTestStub + public: true + autoconfigure: true + calls: + - [setSunshine, [supernova]] + - [setSunshine, [warm]] + diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/instanceof_and_calls/main.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/instanceof_and_calls/main.yml new file mode 100644 index 0000000000..aa0924e107 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/integration/instanceof_and_calls/main.yml @@ -0,0 +1,9 @@ +services: + _instanceof: + Symfony\Component\DependencyInjection\Tests\Compiler\IntegrationTestStubParent: + calls: + - [setSunshine, [warm]] + main_service: + class: Symfony\Component\DependencyInjection\Tests\Compiler\IntegrationTestStub + autoconfigure: true + public: true