bug #40167 [DependencyInjection] Definition::removeMethodCall should remove all matching calls (ruudk)

fabpot · fabpot · commit aad1d094a5a1 · 2021-02-14T12:22:38.000+01:00
This PR was submitted for the 5.x branch but it was merged into the 4.4 branch instead. Discussion ---------- [DependencyInjection] Definition::removeMethodCall should remove all matching calls It would only remove the first match, leaving the other method call(s) there to exist. This leads to unexpected situations. | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | None | License | MIT | Doc PR | I found out that `Definition::removeMethodCall` only removes the first method call and stops. It should remove all method calls named `$method`. Commits ------- 944ba23 Definition::removeMethodCall should remove all matching calls
diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php
@@ -392,7 +392,6 @@ public function removeMethodCall($method)
         foreach ($this->calls as $i => $call) {
             if ($call[0] === $method) {
                 unset($this->calls[$i]);
-                break;
             }
         }
 
diff --git a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php
@@ -425,4 +425,20 @@ public function testAddError()
         $def->addError('Second error');
         $this->assertSame(['First error', 'Second error'], $def->getErrors());
     }
+
+    public function testMultipleMethodCalls()
+    {
+        $def = new Definition('stdClass');
+
+        $def->addMethodCall('configure', ['arg1']);
+        $this->assertTrue($def->hasMethodCall('configure'));
+        $this->assertCount(1, $def->getMethodCalls());
+
+        $def->addMethodCall('configure', ['arg2']);
+        $this->assertTrue($def->hasMethodCall('configure'));
+        $this->assertCount(2, $def->getMethodCalls());
+
+        $def->removeMethodCall('configure');
+        $this->assertFalse($def->hasMethodCall('configure'));
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -392,7 +392,6 @@ public function removeMethodCall($method)`
`392`	`392`	`foreach ($this->calls as $i => $call) {`
`393`	`393`	`if ($call[0] === $method) {`
`394`	`394`	`unset($this->calls[$i]);`
`395`		`- break;`
`396`	`395`	`}`
`397`	`396`	`}`
`398`	`397`