[DependencyInjection] Avoid generating call_user_func in more cases #18682
Conversation
bb94287 to
9db2a9c
Compare
|
Not sure what do with the fabbot.io failures as the offending code is in the dumped containers. |
| @@ -542,6 +542,10 @@ private function addServiceConfigurator($id, $definition, $variableName = 'insta | |||
| return sprintf(" %s::%s(\$%s);\n", $this->dumpLiteralClass($class), $callable[1], $variableName); | |||
| } | |||
|
|
|||
| if (strpos($class, 'new') === 0) { | |||
There was a problem hiding this comment.
Shouldn't this check for new (note the trailing space)?
There was a problem hiding this comment.
I agree, should be 0 === strpos($class, 'new ')
|
👍 after @xabbuh's comment is fixed (fabbot failures should be ignored for failures). |
| @@ -723,6 +727,10 @@ private function addNewInstance(Definition $definition, $return, $instantiation, | |||
| return sprintf(" $return{$instantiation}%s::%s(%s);\n", $this->dumpLiteralClass($class), $callable[1], $arguments ? implode(', ', $arguments) : ''); | |||
| } | |||
|
|
|||
| if (strpos($class, 'new') === 0) { | |||
There was a problem hiding this comment.
0 === strpos($class, 'new ')
|
ping @realityking |
|
@realityking will you have some time to finish this nice pull request adding the minor changes asked by @nicolas-grekas? If not, we can make those minor changes ourselves. Thank you! |
There are some circumstances where a Factory or a Configurator causes code using call_user_func is dumped but this hasn't been tested.
9db2a9c to
62411f3
Compare
|
Sorry, I was distracted and then forgot all about this. Comments addressed, hope Travis runs succeeds this time. |
|
Remaning failures are in the generated code. |
|
Thank you @realityking. |
… more cases (realityking) This PR was submitted for the master branch but it was merged into the 3.0 branch instead (closes #18682). Discussion ---------- [DependencyInjection] Avoid generating call_user_func in more cases | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | This is a follow up to #9807 to add the PHP 5.4 only coded I excluded when I split it off #9432. Commits ------- 2718a6c [DependencyInjection] Avoid generating call_user_func in more cases
This is a follow up to #9807 to add the PHP 5.4 only coded I excluded when I split it off #9432.