Fill in topic/@defgroup docs for Services/DIC

A few comments based on a read-through as a developer who's fairly new to the topic -- hence can't vouch for accuracy. (Though seems to match up with the little I know!)

1. +++ b/core/modules/system/core.api.php
   @@ -658,14 +658,118 @@
   + * Information about the Dependency Injection Container and Services.
   

   I don't think a topic needs to say it's 'information about...'? That's implicit, surely?

2. +++ b/core/modules/system/core.api.php
   @@ -658,14 +658,118 @@
   + * interface text) can be defined by Drupal Core or a module, along with a
   + * default class for providing that service. Other modules can offer alternative
   

   This seems to imply that providing a class is optional. Is it?

   If not, I would syggest:

   'A service [...] is provided by Drupal core of a module by defining the service [in a .yml file?] and a default class [...]'

3. +++ b/core/modules/system/core.api.php
   @@ -658,14 +658,118 @@
   + * class via the "dependency injection container" (also known simply as the
   

   I don't think we need the quotes here.

4. +++ b/core/modules/system/core.api.php
   @@ -658,14 +658,118 @@
   + * A typical service definition in a services.yml file looks like this:
   

   Maybe call it *.services.yml?

5. +++ b/core/modules/system/core.api.php
   @@ -658,14 +658,118 @@
   + * The first line gives the machine name of the service, and the class line
   + * gives the default class that provides the service. For some services, the
   

   Machine name: prefixed with module name? and by convention?

   Machine name is presumably what you use when you want to access the service. Could do with a mention here?

6. +++ b/core/modules/system/core.api.php
   @@ -658,14 +658,118 @@
   + * arguments line lists other services that are dependencies; the classes
   + * for each of these services is instantiated from the container and passed
   

   Some explanation of the syntax for the arguments is needed. What does the @ mean?

7. +++ b/core/modules/system/core.api.php
   @@ -658,14 +658,118 @@
   + *   @code $container->get('service.name') @endcode to instantiate a service.
   

   Should be made clear that service.name is the machine name mentioned earlier. Or is the 'service' part a special prefix?

8. +++ b/core/modules/system/core.api.php
   @@ -658,14 +658,118 @@
   + *   the above methods of "dependency injection", you can access the container
   

   I don't think we need quotes here.

1. +++ b/core/lib/Drupal/Core/DependencyInjection/ServiceProviderBase.php
   @@ -9,6 +9,8 @@
   + *
   + * @ingroup container
     */
    abstract class ServiceProviderBase implements ServiceProviderInterface, ServiceModifierInterface {
   

   Shouldn't this be added to ServiceProviderInterface then too?

   And CoreServiceProvider might be an interesting example to look at, so add that too?

2. +++ b/core/modules/system/core.api.php
   @@ -706,12 +706,121 @@
   + * The first line gives the unique machine name of the service (typically
   + * prefixed by the module name if provided by a module), and the class line
   

   There are common exceptions to the prefix by module rule, certain similar things share a common prefix, for example, all cache bins are registered as cache.name_of_the_bin, and most plugin managers are named plugin.manager.something.

3. +++ b/core/modules/system/core.api.php
   @@ -706,12 +706,121 @@
   + * gives the default class that provides the service. For some services, the
   

   Should probably at least mention that instead of defining a class, it's also possible to define a factory class or service, the cache bins are a good example for that, and then refer to some documentation?

4. +++ b/core/modules/system/core.api.php
   @@ -706,12 +706,121 @@
   + * - Plugin classes, controllers, and similar classes have create() methods,
   + *   whose first argument is
   + *   @code
   + *   \Symfony\Component\DependencyInjection\ContainerInterface $container
   

   There are multiple different create() methods and they only work when that class implements the corresponding interface. Different interfaces are needed because they have different arguments:

   ContainerInjectionInterface: just the container, for forms and controllers
   ContainerDerivativeInterface: for plugin derivatives, container + base plugin id
   ContainerFactoryPluginInterface: for plugins, receives container, $configuration, $plugin_id and $definition
   EntityControllerInterface: $container and $entity_type, named createInstance() due to a name clash with the create() method on entity storage classes.

   Mentioning them all here might be too long, but it should be mentioned that the right interface needs to be implemented, maybe they can be added to a documentation page?

5. +++ b/core/modules/system/core.api.php
   @@ -706,12 +706,121 @@
   + *   access any service. Examples:
   + *   @code
   + *   // Retrieve the entity.manager service object (special method exists).
   + *   $manager = \Drupal->entityManager();
   + *   // Retrieve the service object for machine name 'foo.bar'.
   + *   $foobar = \Drupal->service('foo.bar');
   + *   @endcode
   

   Might be worth mentioning here why we bother with the create() stuff and constructor injection if this is so easy: This makes your code dependent on the container itself which makes it harder to write unit tests for your code. The service() is also less useful for IDE autocomplete and self-documentation, constructor injection allows to enforce the right thing with interface type hints.

   It's technically also a service locator pattern when used like that and not dependency injection (nothing is injected, it's fetched). The difference to the create method is that calling that method is optional, in a unit test for example, you can directly call the constructor.

6. +++ b/core/modules/system/core.api.php
   @@ -706,12 +706,121 @@
   + * - Add an entry to a modulename.services.yml file for the service. See
   + *   @ref sec_discover above, or existing services.yml files in Core, for the
   + *   syntax; it will start with your machine name, refer to your default class,
   + *   and list the services that need to be passed into your constructor.
   

   Should this also mention that services can be defined dynamically in a service provider? See CoreServiceProvider.

   There are a few things that are only possible there, for example compiler passes.

   Speaking of those, a concept that is unclear to many is what tags do. I think a chapter that quicky explains what they are and commonly used tags would be very useful here.

   short version:

   Tagged services can be discovered/queried in a so called compiler pass while the container is built. See CoreServiceProvider::register(). They don't do anything on their own, it completely depends on the compiler pass that is doing something with them. Most of the time, tags come with a corresponding interface that such a service must implement.

   common:
   - event_subscriber: Needed by event subscribers, not sure if that should be a completely separate topic to explain in depth, as it's fairly important.
   - access_check: Used for route access checks, additionally contains the name that is then used in the route requirements definition.
   - cache.bin: Must be added to cache bin services, so that cache clear all and cache bin removal on module uninstallation works.
   - needs_destruction: Can be used if a service wants to be called at the end of a request if he was initialized. For example writing a cache that was collected during a request.

7. +++ b/core/modules/system/core.api.php
   @@ -706,12 +706,121 @@
   + * - Include a use statement for the class
   + *   \Drupal\Core\DependencyInjection\ContainerBuilder
   

   Do we really need to document this here? There's nothing special about that, you also need a use for ServiceModifierInterface and ContainerBuilder is in the alter() method, so not having it will fail with an error, just like every other missing use.

+++ b/core/modules/system/core.api.php
@@ -789,6 +824,29 @@
+ * - event_subscriber: Indicates an event subscriber service for routing; see
+ *   the @link menu Menu and routing system topic @endlink for more information.

event_subscriber is a more generic topic than just routing, that's just one using, like something else uses hooks to allow other modules to do something. Already mentioned above, this is big enough to deserve it's own @defgroup, maybe set that up as a @todo to be able to reference it?

All other changes look great, RTBC imho when that above is resolved.

Works for me. Maybe we will have useful information of our own at one point (there's discussion to move that to separate file) but then this will need to be updated anyway.

+++ b/core/modules/system/core.api.php
@@ -706,12 +708,180 @@
+ *   @ref sec_discover above, or existing services.yml files in Core, for the

Minor nit: The core file is core.services.yml. (If someone searches for services.yml as a file name they'll find nothing.)

Otherwise this looks solid. Well done!

Referencing core.services.yml explicitly would have been a valid option too, as that contains more examples than you'll ever need.

I'm happy, Crell is happy, everyone's happy!

> + * interface text) is defined (given an name and an interface or at least a

'an name'

Let's do this, we can "iterate" more later (i want the bike shed red).

This is great!!

Committed and pushed to 8.x. Thanks!

Committed 13f3582 and pushed to 8.x. Thanks!