Support ServiceClosureArgument in \Drupal\Component\DependencyInjection\Dumper\OptimizedPhpArrayDumper::dumpValue

The provided patch does not handle ServiceClosureArgument correctly. The class that will be using this argument, ServiceLocatorTrait, expects it to be a callable and passes itself as an argument when calling it. The return value should be the actual service. See ServiceLocatorTrait::get():

return $this->factories[$id]($this);

Instead, with the patch the service itself is dumped thus $this->factories[$id] is already the service.

I tried to modify OptimizedPhpArrayDumper a bit and use a closure for the factory instead, however dumping closures is not supported. So eventually I had to use an additional class for the factory. Unfortunately I had to use the static \Drupal::getContainer() in this class. Maybe better solutions for this problem exist, but I'm out of ideas at the moment. Symfony's PhpDumper has a lot more freedom and flexibility in this regards, as it can just just any possible PHP code.

Changes in OptimizedPhpArrayDumper:

elseif ($value instanceof ServiceClosureArgument) {
      $reference = $value->getValues();
      /** @var Reference $reference */
      $reference = reset($reference);

      // We use a special Invokable class here, as closures can not be
      // serialized.
      // @see \Symfony\Component\DependencyInjection\Dumper::PhpDumper::dumpValue().
      return new DumpedServiceClosureArgument((string) $reference);
    }

The additional class:

namespace Drupal\Component\DependencyInjection\Dumper;

use Psr\Container\ContainerInterface;

class DumpedServiceClosureArgument {

  public function __construct(protected string $serviceId) {

  }

  public function __invoke(ContainerInterface $serviceLocator) {
    $container = \Drupal::getContainer();
    return $container->get($this->serviceId);
  }

}

I tought another approach, based on how 'private_service' is done. ServiceClosureArgument is dumped now to a stdClass and then in the container itself it's converted to a closure.

Attached a patch based on the 9.5.x branch.

I am not sure when / under which conditions Drupal\Component\DependencyInjection\PhpArrayContainer is used though, so I could not verify my changes in there.

Patch in #27 contained a mistake, attaching a new one.

#2909185: Replace ContainerAwareEventDispatcher with Symfony EventDispatcher also has a patch that handles this.

Thanks! I will have a closer look at the other patch, it seems pretty close to my last attempt but might contain some stuff I missed.
I am preparing unit tests as well.

Thanks for adding the tests. This looks OK to me and will simplify swapping out the event dispatcher for Symfony's one.

I think we need a CR record here to tell people that we now support this. The patch looks good.

Added a draft change record, we should add an example in the change record of how this looks in a services file. I don't understand this enough to add a good example.

Adapted the example from https://symfony.com/doc/current/service_container/service_closures.html for the CR.

#31 fails to apply, needs a reroll.

Rerolled patch on #31

Reuploading the patch after fixing issues.

No substantial changes since #34, hence RTBC.

I removed the service declaration from the change record, since that isn't working with the custom YamlFileLoader Drupal is currently using. But for service definitions built programmatically, this patch still brings some benefits.

(also added a change record)

Committed and pushed 92e703a69a to 11.x. Thanks!

Let's continue in #2909185: Replace ContainerAwareEventDispatcher with Symfony EventDispatcher which should become almost trivial now.

Just created a new issue for following up on ServiceClosureArgument with private services: #3391860: ServiceClosureArgument with private service references original non-existent service