bug #21436 [DependencyInjection] check for circular refs caused by method calls (xabbuh)

fabpot · fabpot · commit 3441b1586f3e · 2017-02-16T05:23:11.000-08:00
This PR was merged into the 2.7 branch. Discussion ---------- [DependencyInjection] check for circular refs caused by method calls | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #19362 | License | MIT | Doc PR | Before we check for circular references, dependencies coming from method calls are not part of the dependency graph. That why the pass is not able to detect circular references like the one described in #19362 during compilation of the container. If we add another check after all the optimisation passes have been processed, we should be able to detect these circular references too. Commits ------- fe4f7ec check for circular refs caused by method calls
diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php
@@ -63,6 +63,7 @@ public function __construct()
                 new RemoveUnusedDefinitionsPass(),
             )),
             new CheckExceptionOnInvalidReferenceBehaviorPass(),
+            new CheckCircularReferencesPass(),
         );
     }
 
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php
@@ -113,4 +113,30 @@ public function testProcessInlinesWhenThereAreMultipleReferencesButFromTheSameDe
         $this->assertFalse($container->hasDefinition('b'));
         $this->assertFalse($container->hasDefinition('c'), 'Service C was not inlined.');
     }
+
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException
+     */
+    public function testCircularReferencesCausedByMethodCallsAreDetectedDuringCompilation()
+    {
+        $container = new ContainerBuilder();
+        $container->setResourceTracking(false);
+
+        $container
+            ->register('foobar', '\stdClass')
+            ->addArgument(new Reference('foo'))
+        ;
+
+        $container
+            ->register('foo', '\stdClass')
+            ->addArgument(new Reference('bar'))
+        ;
+
+        $container
+            ->register('foo', '\stdClass')
+            ->addMethodCall('addFoobar', array(new Reference('foobar')))
+        ;
+
+        $container->compile();
+    }
 }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container9.php
@@ -64,7 +64,6 @@
 ;
 $container
     ->register('baz', 'Baz')
-    ->addMethodCall('setFoo', array(new Reference('foo_with_inline')))
 ;
 $container
     ->register('request', 'Request')
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/graphviz/services9.dot b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/graphviz/services9.dot
@@ -36,6 +36,5 @@ digraph sc {
   node_method_call1 -> node_foobaz [label="setBar()" style="dashed"];
   node_foo_with_inline -> node_inlined [label="setBar()" style="dashed"];
   node_inlined -> node_baz [label="setBaz()" style="dashed"];
-  node_baz -> node_foo_with_inline [label="setFoo()" style="dashed"];
   node_configurator_service -> node_baz [label="setFoo()" style="dashed"];
 }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php
@@ -80,11 +80,7 @@ protected function getBarService()
      */
     protected function getBazService()
     {
-        $this->services['baz'] = $instance = new \Baz();
-
-        $instance->setFoo($this->get('foo_with_inline'));
-
-        return $instance;
+        return $this->services['baz'] = new \Baz();
     }
 
     /**
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php
@@ -99,11 +99,7 @@ protected function getBarService()
      */
     protected function getBazService()
     {
-        $this->services['baz'] = $instance = new \Baz();
-
-        $instance->setFoo($this->get('foo_with_inline'));
-
-        return $instance;
+        return $this->services['baz'] = new \Baz();
     }
 
     /**
@@ -227,12 +223,11 @@ protected function getFooBarService()
     protected function getFooWithInlineService()
     {
         $a = new \Bar();
-
-        $this->services['foo_with_inline'] = $instance = new \Foo();
-
         $a->pub = 'pub';
         $a->setBaz($this->get('baz'));
 
+        $this->services['foo_with_inline'] = $instance = new \Foo();
+
         $instance->setBar($a);
 
         return $instance;
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml
@@ -70,11 +70,7 @@
         <argument type="service" id="baz"/>
       </call>
     </service>
-    <service id="baz" class="Baz">
-      <call method="setFoo">
-        <argument type="service" id="foo_with_inline"/>
-      </call>
-    </service>
+    <service id="baz" class="Baz"/>
     <service id="request" class="Request" synthetic="true"/>
     <service id="configurator_service" class="ConfClass" public="false">
       <call method="setFoo">
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/services9.yml
@@ -52,8 +52,6 @@ services:
 
     baz:
         class: Baz
-        calls:
-            - [setFoo, ['@foo_with_inline']]
 
     request:
         class: Request

Original file line number	Diff line number	Diff line change
`@@ -63,6 +63,7 @@ public function __construct()`
`63`	`63`	`new RemoveUnusedDefinitionsPass(),`
`64`	`64`	`)),`
`65`	`65`	`new CheckExceptionOnInvalidReferenceBehaviorPass(),`
	`66`	`+ new CheckCircularReferencesPass(),`
`66`	`67`	`);`
`67`	`68`	`}`
`68`	`69`
Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,6 @@`
`64`	`64`	`;`
`65`	`65`	`$container`
`66`	`66`	`->register('baz', 'Baz')`
`67`		`- ->addMethodCall('setFoo', array(new Reference('foo_with_inline')))`
`68`	`67`	`;`
`69`	`68`	`$container`
`70`	`69`	`->register('request', 'Request')`
Original file line number	Diff line number	Diff line change
`@@ -36,6 +36,5 @@ digraph sc {`
`36`	`36`	`node_method_call1 -> node_foobaz [label="setBar()" style="dashed"];`
`37`	`37`	`node_foo_with_inline -> node_inlined [label="setBar()" style="dashed"];`
`38`	`38`	`node_inlined -> node_baz [label="setBaz()" style="dashed"];`
`39`		`- node_baz -> node_foo_with_inline [label="setFoo()" style="dashed"];`
`40`	`39`	`node_configurator_service -> node_baz [label="setFoo()" style="dashed"];`
`41`	`40`	`}`
Original file line number	Diff line number	Diff line change
`@@ -80,11 +80,7 @@ protected function getBarService()`
`80`	`80`	`*/`
`81`	`81`	`protected function getBazService()`
`82`	`82`	`{`
`83`		`- $this->services['baz'] = $instance = new \Baz();`
`84`		`-`
`85`		`- $instance->setFoo($this->get('foo_with_inline'));`
`86`		`-`
`87`		`- return $instance;`
	`83`	`+ return $this->services['baz'] = new \Baz();`
`88`	`84`	`}`
`89`	`85`
`90`	`86`	`/**`