8000
DI generates an invalid code. 'Undefined variable $containerRef' #50257
Labels
Comments
|
Here is the fix I think: --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
+++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@@ -945,8 +945,9 @@ EOF;
if (!$isProxyCandidate && !$definition->isShared()) {
$c = implode("\n", array_map(fn ($line) => $line ? ' '.$line : $line, explode("\n", $c)));
$lazyloadInitialization = $definition->isLazy() ? ', $lazyLoad = true' : '';
+ $useContainerRef = $this->addContainerRef ? ' use ($containerRef)' : '';
- $c = sprintf(" %s = function (\$container%s) {\n%s };\n\n return %1\$s(\$container);\n", $factory, $lazyloadInitialization, $c);
+ $c = sprintf(" %s = function (\$container%s)%s {\n%s };\n\n return %1\$s(\$container);\n", $factory, $lazyloadInitialization, $useContainerRef, $c);
}
$code .= $c;Can you come up with a test case? If yes, can you submit a PR with the fix + the test case 🙏 ? |
|
I will try. It will be a pleasure. |
|
Hi @nicolas-grekas, just letting you know that the PR is ready for your review. Let me know if you have any concerns or suggestions. |
fabpot
added a commit
that referenced
this issue
May 12, 2023
…h TaggedIteratorArgument (marphi) This PR was squashed before being merged into the 6.3 branch. Discussion ---------- [DependencyInjection] Fix dumping non-shared factories with TaggedIteratorArgument Fix dumping non-shared factories which have a configured `methodCall` with `TaggedIteratorArgument`. | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #50257 | License | MIT I've updated my project to the fresh 6.3-beta versions and discovered that DI generates an invalid code. I've reported issue #50257, where you can find more info and context. The PHPDumper class generated code where use the `$containerRef` variable located in into anonymous function where this variable is not accessible. `@nicolas`-grekas quickly hinted to me where probably the bug is located. It works! I've prepared PR with this fix, and a PHPUnit test covers this case. Before the fix, my test case generated this code: ``` protected static function getFooService($container) { $containerRef = $container->ref; $container->factories['foo'] = function ($container) { $instance = new \Symfony\Component\DependencyInjection\Tests\Dumper\FooClass(); $instance->setOtherInstances(new RewindableGenerator(function () use ($containerRef) { $container = $containerRef->get(); yield 0 => ($container->privates['Bar'] ??= new \Bar()); yield 1 => ($container->privates['stdClass'] ??= new \stdClass()); }, 2)); return $instance; }; return $container->factories['foo']($container); } ``` Which threw an error: `Warning: Undefined variable $containerRef` at this line: ``` $instance->setOtherInstances(new RewindableGenerator(function () use ($containerRef) { ``` After the fix, looks good and works. ``` protected static function getFooService($container) { $containerRef = $container->ref; $container->factories['foo'] = function ($container) use ($containerRef) { $instance = new \Symfony\Component\DependencyInjection\Tests\Dumper\FooClass(); $instance->setOtherInstances(new RewindableGenerator(function () use ($containerRef) { $container = $containerRef->get(); yield 0 => ($container->privates['Bar'] ??= new \Bar()); yield 1 => ($container->privates['stdClass'] ??= new \stdClass()); }, 2)); return $instance; }; return $container->factories['foo']($container); } ``` Thx `@nicolas`-grekas for your help! Commits ------- a60dfcf [DependencyInjection] Fix dumping non-shared factories with TaggedIteratorArgument
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Symfony version(s) affected
6.3.0-BETA2
Description
I tested the latest 6.3.0-BETA2 release on my project and found something worth to report.
I've got this warning during
cache:clearWarning: Undefined variable $containerRefI suspect the issue is related to
!tagged_iteratorand factories.How to reproduce
Example DI configuration in this case part of my SonataAdmin configuration.
Generates invalid code:
Take a look at line:
$containerRefthis variable exists, but the code is located in into anonymous function where origin variable$containerRefis not accessible.Possible Solution
I'm not sure and not have enough knowledge about internal DI matters. I suppose PhpDumper have an issue and needs to rewrite:
from:
to:
But will it always work?
Additional Context
No response
The text was updated successfully, but these errors were encountered: