Allow plugin service wiring via constructor parameter attributes

@larowlan I added a test that is similar to your trait setup but it passed first time, so not sure what I have done wrong - any ideas?

I'll see if I can still replicate it - might have been a PHP version issue?

For full backward compatibility we might need to provide an autowiring trait, which perhaps means we can simplify some of this.

class MenuLinkDefault extends MenuLinkBase implements ContainerFactoryPluginInterface {

  public function __construct(array $configuration, $plugin_id, $plugin_definition, StaticMenuLinkOverridesInterface $static_override) {
    ...
  }

  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
    ...
  }
}

In the long run we probably want to drop the create method here and use autowiring instead, but then we are no longer implementing ContainerFactoryPluginInterface, and any subclasses will no longer have their create() method called.

So maybe the answer is to autowire the constructor, and use a trait to replace create() that handles the autowiring? Then any subclasses will work with no changes - we could also find a way to notify downstream users that they should switch to autowiring, or maybe we just live with the trait forever until we deprecate ContainerFactoryPluginInterface entirely?

Sounds like this should be NW for the trait mentioned in #19?

Marked 📌 Allow typed data plugins to receive injected dependencies Closed: duplicate as a duplicate as well. Would be great to make some traction on this by answering the questions raised in #19, or perhaps this just needs an IS update to reflect the current status after the recent MR changes.

@duadua Some great ideas there.

Following these issues:
📌 Enable service autowiring by adding interface aliases to core service definitions Fixed
📌 Enable more service autowiring by adding interface aliases to core modules Fixed
it is possible to infer the correct service in many cases without needing the #[Autowire] attribute. Therefore we can implement a trait that autowires based on the type where possible, or the attribute where it isn't possible, and this seems fairly trivial. I don't think we have to worry about inheritance even? Subclassed plugins will continue to provide their own create() method, parent or child classes can switch to the trait any time, as far as I can see - this might need a bit of testing though.

Proof of concept patch attached.

If a new approach is being taken could the IS be updated?

Will new approach still require the change record? Leaving tag just in case.

Can this be expanded to all classes that have the container factory pattern -- forms and controllers IIRC.

Yes, but probably not in this issue; I picked plugins first as I was getting annoyed keeping create() methods in sync with the constructor but those are valid candidates as well.

Forms have exactly the same pattern of create() and __construct(), just use Drupal\Core\DependencyInjection\ContainerInjectionInterface instead.

But the plugin factory method and constructors have three additional arguments - array $configuration, $plugin_id, $plugin_definition - so I don't think we can use the same code/trait.

We have the controller issue

So, the trait in the patch does the job. +1.

Is there any benefit of doing it in ContainerFactory instead of using the trait?

Actually, migrate plugins get an additional parameter passed to the ::create() method.
In MigratePluginManager:

      $plugin = $plugin_class::create(\Drupal::getContainer(), $configuration, $plugin_id, $plugin_definition, $migration);

The positional mapping with 3 parameters would lose this 4th parameter.

But technically there is no guarantee that calling code will always pass the `$migration` parameter.
The 4th parameter is optional, to comply with LSP.

I think such cases need a custom trait with 4 parameters instead of 3.
The trait can be chosen per class, according to the needs of the constructor.
E.g. some migrate plugins might not need the `$migration`, then they can use the regular trait.

> But the plugin factory method and constructors have three additional arguments - array $configuration, $plugin_id, $plugin_definition

Field formatter plugins have more base constructor arguments too. I know ALL about this problem from Module Builder.... :/

Created ✨ Add an Autowire trait for plugins Needs review , as an interim DX measure for all existing (legacy) plugins.

As a step forward, would it be better to push the AutowirePluginTrait [1] in the lead up to D11, as it is so alike the services Drupal\Core\DependencyInjection\AutowireTrait ?

[1] Drupal\Core\DependencyInjection\AutowirePluginTrait
✨ Add an Autowire trait for plugins Needs review

[2] Drupal\Core\DependencyInjection\AutowireTrait
AutowireTrait allows ContainerInjectionInterface classes to be autowired →

This would seem to be in keeping with the current thrust of development. There would also need to be a specialist Trait for the migration plugins which require 4 parameters.

What would be the next steps needed? Core tests: one that shows it working the old way, and one that shows it working the new?

Agree that the Trait would be awesome

Urgh, the field formatters case is pretty horrible.

  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
    return new static(
      $plugin_id,
      $plugin_definition,
      $configuration['field_definition'],
      $configuration['settings'],
      $configuration['label'],
      $configuration['view_mode'],
      $configuration['third_party_settings'],
      $container->get('entity_type.manager'),
      $container->get('language_manager')
    );
  }

Shouldn't that extraction work be done in the constructor instead? But changing all existing constructors and providing BC is going to be no fun at all.

Shouldn't that extraction work be done in the constructor instead?

Agreed. Just pass the $configuration to the constructor.

I did a quick count and there are 92 sub-classes of FormatterBase in core alone.