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

…n type of closure is `void`
symfony · nicolas-grekas · Aug 3, 2023 · Aug 3, 2023 · Aug 3, 2023 · 692704e133726ba9ff8ecf12f845cac1ed96c19a
commit 692704e133726ba9ff8ecf12f845cac1ed96c19a
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(...);
    }
 }