[DependencyInjection] Do not add return in LazyClosure when retur…

…n type of closure is `void`
symfony · Aug 3, 2023 · 692704e · 692704e
1 parent 1a15b12
commit 692704e
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 10 deletions.
diff --git a/src/Symfony/Component/DependencyInjection/Argument/LazyClosure.php b/src/Symfony/Component/DependencyInjection/Argument/LazyClosure.php
@@ -78,7 +78,8 @@ public static function getCode(string $initializer, array $callable, Definition
             throw new RuntimeException("Cannot create lazy closure{$id} because its corresponding callable is invalid.");
         }
 
-        $code = ProxyHelper::exportSignature($r->getMethod($method), true, $args);
+        $methodReflector = $r->getMethod($method);
+        $code = ProxyHelper::exportSignature($methodReflector, true, $args);
 
         if ($asClosure) {
             $code = ' { '.preg_replace('/: static$/', ': \\'.$r->name, $code);
@@ -87,7 +88,7 @@ public static function getCode(string $initializer, array $callable, Definition
         }
 
         $code = 'new class('.$initializer.') extends \\'.self::class
-            .$code.' { return $this->service->'.$callable[1].'('.$args.'); } '
+            .$code.' { '.($methodReflector->hasReturnType() && 'void' === (string) $methodReflector->getReturnType() ? '' : 'return ').'$this->service->'.$callable[1].'('.$args.'); } '
             .'}';
 
         return $asClosure ? '('.$code.')->'.$method.'(...)' : $code;

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
@@ -48,6 +48,7 @@
 use Symfony\Component\DependencyInjection\Tests\Compiler\AInterface;
 use Symfony\Component\DependencyInjection\Tests\Compiler\Foo;
 use Symfony\Component\DependencyInjection\Tests\Compiler\FooAnnotation;
+use Symfony\Component\DependencyInjection\Tests\Compiler\FooVoid;
 use Symfony\Component\DependencyInjection\Tests\Compiler\IInterface;
 use Symfony\Component\DependencyInjection\Tests\Compiler\MyCallable;
 use Symfony\Component\DependencyInjection\Tests\Compiler\SingleMethodInterface;
@@ -1874,12 +1875,18 @@ public function testAutowireClosure()
     public function testLazyClosure()
     {
         $container = new ContainerBuilder();
-        $container->register('closure', 'Closure')
+        $container->register('closure1', 'Closure')
             ->setPublic('true')
             ->setFactory(['Closure', 'fromCallable'])
             ->setLazy(true)
             ->setArguments([[new Reference('foo'), 'cloneFoo']]);
+        $container->register('closure2', 'Closure')
+            ->setPublic('true')
+            ->setFactory(['Closure', 'fromCallable'])
+            ->setLazy(true)
+            ->setArguments([[new Reference('foo_void'), '__invoke']]);
         $container->register('foo', Foo::class);
+        $container->register('foo_void', FooVoid::class);
         $container->compile();
         $dumper = new PhpDumper($container);
 
@@ -1890,11 +1897,18 @@ public function testLazyClosure()
         $container = new \Symfony_DI_PhpDumper_Test_Lazy_Closure();
 
         $cloned = Foo::$counter;
-        $this->assertInstanceOf(\Closure::class, $container->get('closure'));
+        $this->assertInstanceOf(\Closure::class, $container->get('closure1'));
         $this->assertSame($cloned, Foo::$counter);
-        $this->assertInstanceOf(Foo::class, $container->get('closure')());
+        $this->assertInstanceOf(Foo::class, $container->get('closure1')());
         $this->assertSame(1 + $cloned, Foo::$counter);
-        $this->assertSame(1, (new \ReflectionFunction($container->get('closure')))->getNumberOfParameters());
+        $this->assertSame(1, (new \ReflectionFunction($container->get('closure1')))->getNumberOfParameters());
+
+        $counter = FooVoid::$counter;
+        $this->assertInstanceOf(\Closure::class, $container->get('closure2'));
+        $this->assertSame($counter, FooVoid::$counter);
+        $container->get('closure2')('Hello');
+        $this->assertSame(1 + $counter, FooVoid::$counter);
+        $this->assertSame(1, (new \ReflectionFunction($container->get('closure2')))->getNumberOfParameters());
     }
 
     public function testLazyAutowireAttribute()

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/autowiring_classes.php
@@ -38,6 +38,16 @@ public function cloneFoo(\stdClass $bar = null): static
     }
 }
 
+class FooVoid
+{
+    public static int $counter = 0;
+
+    public function __invoke(string $name): void
+    {
+        ++self::$counter;
+    }
+}
+
 class Bar
 {
     public function __construct(Foo $foo)

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/lazy_closure.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/lazy_closure.php
@@ -20,7 +20,8 @@ public function __construct()
     {
         $this->services = $this->privates = [];
         $this->methodMap = [
-            'closure' => 'getClosureService',
+            'closure1' => 'getClosure1Service',
+            'closure2' => 'getClosure2Service',
         ];
 
         $this->aliases = [];
@@ -40,6 +41,7 @@ public function getRemovedIds(): array
     {
         return [
             'foo' => true,
+            'foo_void' => true,
         ];
     }
 
@@ -49,12 +51,22 @@ protected function createProxy($class, \Closure $factory)
     }
 
     /**
-     * Gets the public 'closure' shared service.
+     * Gets the public 'closure1' shared service.
      *
      * @return \Closure
      */
-    protected static function getClosureService($container, $lazyLoad = true)
+    protected static function getClosure1Service($container, $lazyLoad = true)
     {
-        return $container->services['closure'] = (new class(fn () => new \Symfony\Component\DependencyInjection\Tests\Compiler\Foo()) extends \Symfony\Component\DependencyInjection\Argument\LazyClosure { public function cloneFoo(?\stdClass $bar = null): \Symfony\Component\DependencyInjection\Tests\Compiler\Foo { return $this->service->cloneFoo(...\func_get_args()); } })->cloneFoo(...);
+        return $container->services['closure1'] = (new class(fn () => new \Symfony\Component\DependencyInjection\Tests\Compiler\Foo()) extends \Symfony\Component\DependencyInjection\Argument\LazyClosure { public function cloneFoo(?\stdClass $bar = null): \Symfony\Component\DependencyInjection\Tests\Compiler\Foo { return $this->service->cloneFoo(...\func_get_args()); } })->cloneFoo(...);
+    }
+
+    /**
+     * Gets the public 'closure2' shared service.
+     *
+     * @return \Closure
+     */
+    protected static function getClosure2Service($container, $lazyLoad = true)
+    {
+        return $container->services['closure2'] = (new class(fn () => new \Symfony\Component\DependencyInjection\Tests\Compiler\FooVoid()) extends \Symfony\Component\DependencyInjection\Argument\LazyClosure { public function __invoke(string $name): void { $this->service->__invoke(...\func_get_args()); } })->__invoke(...);
     }
 }