Existing situation

Currently, services provided by a package (module) are registered in a services.yml file shipped with the module.
This yml file also controls the dependencies of each service.

Pros:
- One central place (per module) to see a list of all provided services.

Cons:
- To create/write a service, you need to edit two places.

There are also some things that need services from the container, but do not really need to be exposed as services themselves:
Plugins, page controllers. And hook implementations - see #2237831: Allow module services to specify hooks.

Proposal

Allow services and/or service consumers to declare their dependencies with annotations.

We could come up with a new annotation syntax, but for now I want to keep it simple and propose the same Doctrine annotation syntax as we already use for plugins.


namespace Drupal\forum;

/**
 * @Service(
 *   id = "forum.uninstall_validator",
 *   arguments = (
 *     "@entity.manager",
 *     "@entity.query",
 *     "@config.factory",
 *     "@string_translation"
 *   ),
 *   lazy = true
 * )
 */
class ForumUninstallValidator {
  [..]
}

Annotations can be on classes, or on (static) factory methods.

We can then convert these annotations into service definitions.

Consequences for services

For services, this is mostly *equivalent* with the existing services.yml declaration:
In either case, the definitions are controlled/owned by the package/module that provides the service, and in either case they can be altered by other modules.

The only difference is the pro/con mentioned above, which is now turned around. And added time/cost to scan the class files.

Pros:
- Only one file to edit, when creating a service.

Cons:
- No more central place (per module) to see a list of available services.
- Added time/cost to scan class files and look for annotations.
- Some people think that annotations are bad and/or ugly.

Annotated service consumers

This can be a solution to avoid dealing with the container directly.

namespace Drupal\node\Controller;

/**
 * Returns responses for Node routes.
 *
 * @Arguments(
 *   '@date.formatter',
 *   '@renderer',
 * )
 */
class NodeController extends ControllerBase implements ContainerInjectionInterface {
  [..]
  public function __construct(DateFormatterInterface $date_formatter, RendererInterface $renderer) {
    [..]
  }
  [..]
}

Annotated plugins

Since plugins already have annotations, it would not be a big change to also annotate the service dependencies - instead of dealing with the container in a static method.

Parameter currying

We could also mix service arguments into other signatures..

/**
 * Implements hook_foo_alter()
 *
 * @Arguments(
 *   1 = '@renderer',
 * )
 */
function mymodule_foo_alter(&$foo, RendererInterface $renderer) {
  [..]
}

This would fix the second parameter, and leave a curried callable with only one free parameter left.

One problem with this is that mymodule_foo_alter() no longer matches the generic signature of e.g. hook_foo_alter(). So we could no longer use stuff like the proposed @implements or @signature tag .. https://github.com/phpDocumentor/phpDocumentor2/issues/1689

Comments

donquixote created an issue. See original summary.

dawehner’s picture

So mh, this would be another way of doing things. Basically all places which would leverage that functionality currently uses the static factory pattern. There is at least some form of standard already.

bojanz’s picture

Please no. It's time we stopped abusing annotations.

Many classes using an annotation for discovery + static factory pattern could actually move to being registered in services.yml. I'd like that direction to happen more than the opposite.

donquixote’s picture

@bojanz: What is your personal reason to dislike annotations?
I have read different opinions about this in the past. Some people don't like the syntax, or the discovery process. Some don't like the entire concept of having stuff i the doc comment that is used for something other than documentation.
Personally I see some pros and cons, but currently don't have a strong opinion either way.

And, would you prefer annotation with annotated dependencies instead of annotation + static method with container parameter?

E.g. if you have the choice between this..

/**
 * @Annotation(..)
 */
class C {
  static function create(Container $container) {
    return new self($container->get('a'), $container->get('b'));
  }
  function __construct(A $a, B $b) {..}
}

.. and this ..

/**
 * @Annotation(
 *   arguments = (
 *     a = "@a",
 *     b = "@b"
 *   )
 * )
 */
class C {
  function __construct(A $a, B $b) {..}
}
dawehner’s picture

My personal reasons why annotations are a bad thing here:

* It is yet another pattern
* If you need to debug stuff its potentially one more level of things you need to deal with
* You get potentially easier autocompletion support, at least container services autocompletion works for me inside create(), not sure about annotaitons.

Crell’s picture

-1. One of the key reasons for doing proper DI is to decouple code from the framework. Setting up your dependencies with an annotation presumes the framework, the DI Container, and the specific service names. Yes, you could futz with configuration to change that elsewhere but now your annotation is lying to you.

Are we really that scared of 2 files? :-)

donquixote’s picture

@Crell:
The annotation is as much a part of the package/module as the services.yml file.

Both of them presume the service names.
Neither of them explicitly presumes the framework or the container.
They simply presume that there is some mechanism that will provide a service for a given service id.
If you need to alter the info from the annotation, you would also need to alter the info from the services.yml.

Of course it is still legitimate to dislike this idea, but imo this is more a stylistic and DX question than anything else. And performance and complexity of the container mechanism.

dawehner’s picture

Of course it is still legitimate to dislike this idea, but imo this is more a stylistic question than anything else. And performance and complexity of the container mechanism.

These are mostly compiler time costs, so potentially it doesn't matter much. Annotations might be okay for custom code but its serious smell IMHO for generic stuff.

benjy’s picture

I'm not a huge fan of annotations, they're harder to write than YML for me and I really have no issues with the two files, the consolidated list for a module is actually quite handy.

As bojanz said, I think moving the other way would be better. We could have also standardised core on yml discovery/definition for plugins for further consistency.

donquixote’s picture

I had some situations where I was creating, renaming and deleting classes a lot, until I was finally happy.
In these cases, the separate files would have been annoying to keep in sync.

Otherwise, maybe they are not such a big deal.

dawehner’s picture

Well at least IDEs like storm should help you with that.

twistor’s picture

One of the key reasons for doing proper DI is to decouple code from the framework. Setting up your dependencies with an annotation presumes the framework, the DI Container, and the specific service names. Yes, you could futz with configuration to change that elsewhere but now your annotation is lying to you.

How is this any different from a factory method?

We've been using something like this for Feeds plugins and it works quite well. The only trouble is that subclasses need to copy the dependency annotation. This can probably be solved by introspection, but it's tricky and I haven't cared to yet.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

donquixote’s picture

Yes, you could futz with configuration to change that elsewhere but now your annotation is lying to you.

Or in case of a module-provided services.yml file (for a module-provided service), the services.yml would be lying to you.
Or in case of a static factory (for a module-provided plugin) the only way is a container adapter. So now the container exposed to the static method is lying to you.

If we sell the annotation or services.yml dependency setting as a suggestion only, I see no problem with "lying".

xano’s picture

Guilty as charged!

This excellent quote from @bojanz got me thinking:

Many classes using an annotation for discovery + static factory pattern could actually move to being registered in services.yml. I'd like that direction to happen more than the opposite.

Symfony's service container isn't just a service container. It contains its own factory as well. While not all classes in our code base are services, having non-PHP instructions on how to instantiate them is useful. I wrote https://www.drupal.org/project/retriever with the idea of annotations in mind (as bad as they sometimes are, sticking metadata to the class in question is such a useful thing), but the codebase is written in such a way that it could also take DI suggestions from a YAML file, for instance. All it needs is a different implementation of the Finder interface.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

andypost’s picture

Is this issue still makes sense?

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bradjones1’s picture

Given the age of this issue and more general efforts such as #3021900: Bring new features from Symfony 3/4/5/6/7 into our container, I think #3221128: Enable service autoconfiguration for core event subscribers is probably a better way to get at this type of functionality.

bradjones1’s picture

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

Status: Active » Closed (duplicate)
Related issues: +#3295751: Autowire core services that do not require explicit configuration

Symfony now supports this with the #[Autowire] attribute which is undoubtedly a nicer syntax than annotations, closing this as a duplicate of #3295751: Autowire core services that do not require explicit configuration

donquixote’s picture

True, autowire is the way to go!
Although that alone does not give us full discovery of services.