Autowire core services that do not require explicit configuration

It turns out that we can just start using the #[Autowire] attribute now, and it works out of the box with Symfony 6.2. If autowiring fails the error message is unhelpful, but that is fixed in Symfony 6.3.

This looks really cool and I use this feature frequently in laravel/symfony projects so I am a big fan.
Since this also already converts a lot of services this would be really helpful to get in but I'm not sure how to review this. If this would be broken in some way this would lead to a lot of broken tests, so seeing that doesn't happen gives a lot of confidence.

We should probably add a change record as well?

I agree with the change record.

As someone who hasn't done any symfony project I didn't know what this was so there are sure to be others.

📌 Enable more service autowiring by adding interface aliases to core modules Fixed landed!

AFAICT that means this issue can now really move forward!

Hi I added a basic CR https://www.drupal.org/node/3366757 →

Opened a new MR for 11.x, following 📌 Enable more service autowiring by adding interface aliases to core modules Fixed hopefully everyone can see how this can simplify services going forwards. One day I hope we won't even need to write services.yml files in most cases.

Seems 11.x caused some failures.

I filed 📌 [policy] Service autowiring - decide approach for multiple service implementations based on the same class and different arguments Needs work . That would help here, by allowing to avoid the need to use #[Autowire] for services that have multiple implementations for the same interface (cache bins, loggers, etc).

Fixed test failures and autowired some things that were previously missed.

This doesn't yet autowire core.services.yml, perhaps that can be done in a followup.

Should this be postponed on 📌 [policy] Service autowiring - decide approach for multiple service implementations based on the same class and different arguments Needs work ?

#22 no I do not think so. If it's finally agreed that 📌 [policy] Service autowiring - decide approach for multiple service implementations based on the same class and different arguments Needs work is worth doing, then it's probably worth closing it and do what it preaches here, so that all things go together.

Think the open question on the other ticket needs to be answered first before closing, and I don't think I can make that call. But the question

Given that each module needs its own logger, should we automatically register a logger service for each module, and then autowire LoggerInterface to use the module specific logger with no extra configuration?

Don't want to hold this up on 📌 [policy] Service autowiring - decide approach for multiple service implementations based on the same class and different arguments Needs work if it's not needed. So lets see what the committers think.

The issue summary is clear here. Reading the comments I don't see any unanswered question.

#21 suggests a followup. Does that need to be done or not?

The title of the CR is a directive. It reads like custom code must use autowiring. I think that needs a tweak. I read the CR and it seems vague to me. It says to 'remove as many explicit arguments as possible' without direction on what 'possible' means here. I also see in the MR the use of

  _defaults:
    autowire: true

Maybe an example should be added that uses that?

I am setting to NW because the MR needs a rebase.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

This is getting unwieldy to repeatedly fix merge conflicts; I wonder if we need to rescope this into smaller issues to get them in more easily.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

+1 — what kind of split do you think would work well? 2 parts or more still?

One way forward might be to write a test to discover which services could be autowired simply by removing their arguments section (ie. all arguments in the constructor can be autowired without specify attributes), which solves the simple cases, then in a followup we handle the more complicated cases where we need cache backends or loggers to be specified.

Added a test case for #32.

Rescoping this issue to cover core.services.yml only; the changes are minimal (almost all deletions) except for a new test that ensures we don't specify arguments in services.yml where autowiring can suffice.

In followups, we can deal with autowiring more of core.services.yml (by either adding more interface aliases, or specifying the #[Autowire] attribute), and autowiring core modules as well.

Updated the change record to match the rescoped approach.

The rescope seems good and definitely simpler to read.

Note for reviewers: the changes in LamguageServiceProvider imply that this is not backward compatible for modules that might implement similar service providers to extend or alter core services. I am not sure what we can do about this.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

re: #38 what is the incompatibility? You no longer have to add an argument that can be autowired, but if you do, it still works, right?

  language_manager:
    class: Drupal\Core\Language\LanguageManager
    arguments: ['@language.default']

  public function alter(ContainerBuilder $container) {
    $definition = $container->getDefinition('language_manager');
    $definition->setClass('Drupal\language\ConfigurableLanguageManager')
      ->addArgument(new Reference('config.factory'))
      ->addArgument(new Reference('module_handler'))
      ->addArgument(new Reference('language.config_factory_override'))
      ->addArgument(new Reference('request_stack'));

IIRC the issue was that if we switch language_manager to autowiring but leave the alter hook unchanged, config.factory becomes the first argument of the new constructor, which is incorrect.