Improve ConfigDependencyManager::getDependentEntities() documentation

  /**
   * Gets dependencies.
   *
   * @param string $type
   *   The type of dependency
   * @param string $name
   *  …
   *
   * @return \Drupal\Core\Config\Entity\ConfigEntityDependency[]
   *   An array of config entity dependency objects that are dependent.
   */
  public function getDependentEntities($type, $name) {

We see:

1. dependencies
2. dependency
3. dependency objects that are dependent
4. dependent entities

This is indeed very confusing at best.

Furthermore:

1. the CR at https://www.drupal.org/node/2220437 doesn't cover this at all.
2. the extensive documentation on \Drupal\Core\Config\Entity\ConfigDependencyManager does not even mention this public API method

I'm working on this in Drupal North code sprint

Thank you to check my first patch

Assigning to @alexpott since he's both a committer and maintainer of this subsystem, so he's best able to review this.

(There are whitespace problems in this patch, but that's less important for now — what matters most is whether the content is accurate.)

1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigDependencyManager.php
   @@ -136,18 +136,30 @@ class ConfigDependencyManager {
   +   * Given a configuration name and type, returns a graph of its dependencies.
   

   It doesn't return the graph anymore. It returns a list of ConfigEntityDependency objects in the order of the nearest to the thing you passed in. So if you pass in Node module you get node_type -> field_storage -> field -> entity view displays.

2. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigDependencyManager.php
   @@ -136,18 +136,30 @@ class ConfigDependencyManager {
   +   * If the given entity is a configuration entity, this function simply calls
   +   * $this->createGraphConfigEntityDependencies() on it.
   +   * ¶
   +   * If given entity is a module, theme or content entity, this function looks
   +   * through this configDependencyManager's configuration entities and calls
   +   * $this->createGraphConfigEntityDependencies() on each of them.
   +   * ¶
   +   * If the given entity isn't a configuration, module, theme or content entity
   +   * then this function returns just an empty array.
   

   This is too much about implementation and not what is being calculated. See the issue summary of #2754477: Unexpected config entity delete due to dependency calculations which has some pictures that explain what is going on here.

3. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigDependencyManager.php
   @@ -136,18 +136,30 @@ class ConfigDependencyManager {
   +   * ¶
   ...
   +   * ¶
   

   Lines have spaces.

4. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigDependencyManager.php
   @@ -136,18 +136,30 @@ class ConfigDependencyManager {
   +   *   The specific configuration name to check. If $type equals 'module' or
   +   *   'theme' then it should be a module name or theme name. Otherwise it
   +   *   should be the full configuration object name.
   

   We should probably add an @see to \Drupal\Core\Entity\EntityInterface::getConfigDependencyName()

5. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigDependencyManager.php
   @@ -136,18 +136,30 @@ class ConfigDependencyManager {
   +   *   An array of forward and reverse config entity dependency objects in a
   +   *   graph.
   

   I'm not sure this is better.

As per @Alexpott review in comment #10
An Interdiff for patch 2724835-ConfigDependencyManager-documentation-7

Sorry about #11 without files.

As per @Alexpott review in comment #10
An Interdiff-2724835-ConfigDependencyManager-documentation-7-12

The patch in #12 seems to address all of @alexpott's comments; and contains no whitespace errors. +1 to RTBC.

1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigDependencyManager.php
   @@ -136,18 +136,29 @@ class ConfigDependencyManager {
   +   * return [node_type, field_storage, field, entity view displays, ...].
   

   I'm not sure about this formatting. Also, if we use an ellipsis we should use the proper unicode character.

2. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigDependencyManager.php
   @@ -136,18 +136,29 @@ class ConfigDependencyManager {
   -   *   or 'content'.
   +   *   or 'content'; anything else will return an empty array.
   

   I don't think this change is necessary. We could add documentation to the return to say if nothing is dependent it'll be empty.

Address the points in #14

Way more helpful than the current docs. :-)

Patch applies to 8.1.x, 8.2.x, 8.3.x.

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigDependencyManager.php
@@ -136,18 +136,30 @@ class ConfigDependencyManager {
+   *   If nothing is dependent it'll be empty.

'Returns an empty array If there are no ancestors.'

Sounds good.

Pretty sure the 8.1.x fail is unrelated, since this is a documentation change.

Looks good, thanks.

1. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigDependencyManager.php
   @@ -136,18 +136,30 @@ class ConfigDependencyManager {
   +   * ConfigEntityDependency objects in the order of the nearest to the entity
   +   * passed in. For example, if passed ('module', 'node'), this function will
   +   * return [node_type, field_storage, field, entity view displays].
   

   Isn't this hardcoding documentation of something that can change? I think we need to add a bit more context, or a simpler, made-up example.

2. +++ b/core/lib/Drupal/Core/Config/Entity/ConfigDependencyManager.php
   @@ -136,18 +136,30 @@ class ConfigDependencyManager {
       * @param string $type
   -   *   The type of dependency being checked. Either 'module', 'theme', 'config'
   -   *   or 'content'.
   +   *   The type of dependency being checked. Either 'module', 'theme', 'config'.
   

   AFAIK we still have content dependencies, so why is that word being removed?

Marking issue for Drupal Mumbai Code Sprint

Here is a reroll of the patch in #18. As the diff (not interdiff) shows, the only difference is a line of context. The patch in #18 was made before we switched to short array syntax.

I am removing the "rc eleigible" issue tag. I think it is out of date. I am also removing DrupalMumbaiCodeSprint, since it looks as though this issue was not touched at that event.

Oops, I forgot to revert the change I made in order to apply the patch from #18. Ignore the patch, diff attached to #30: new ones attached now.