Improve entity bundle info return value docs and consolidate them in one place

Nice, this improves the documentation!

  /**
   * Get the bundle info of all entity types.
   *
   * @return array
   *   An array of all bundle information.
   */
  public function getAllBundleInfo();

I dunno perhaps we should improve this at the same time because tbh this is not much better than no documentation.

@Xano imo this issue should just be merged into that one that. It's all the same scope to me.

Thanks @Xano for keeping patch sizes manageable and for fixing this documentation inconsistency.

In the case of these issues, I think a single patch is best because one is not really complete without the other -- they are all a part of fixing the same conceptual gap in the docs about the bundle info array. When we split patches up, it should be because they are different concepts, or because they are unmanageably large. https://www.drupal.org/core/scope#examples gives examples. In this case it's confusing because one of the issues removes documentation, another adds it back in a different place, and a third adds a reference to that documentation in a third place. This makes it so that reviewers cannot actually see the whole picture of what's being fixed.

Attached combines this with #2712367: Extend EntityTypeBundleInfoInterface's docblocks and make them more explicit and #2712373: Improve hook_entity_bundle_info()'s and hook_entity_bundle_info_alter()'s docblocks. The interdiff shows changes I did not include from the combination of all three issues, because those should just be part of #2571965: [meta] Fix PHP coding standards in core, stage 1. Let's review this one as a whole.

Heh except I did not actually exclude what I said I did. ;) Attached does.

Thanks for the patch! I have no opinion on what should and shouldn't be in this patch, but I do have opinions on the documentation in the patch (which is mostly great). A few small things to address:

1. +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleInfoInterface.php
   @@ -10,8 +10,12 @@
   +   * @return array[]
   +   *   Keys are entity type IDs. Values are arrays of which keys are bundle
   +   *   names, and values are arrays with the same structure as the return value
   +   *   of static::getBundleInfo().
   

   This is very clear, except that we normally want to start the @return docs with a description of what is returned, like what was in here before: "An array of all bundle information." Or better yet: "An associative array of all bundle information, where the keys are... and the values are...".

2. +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleInfoInterface.php
   @@ -19,10 +23,17 @@ public function getAllBundleInfo();
   +   *   Keys are bundle names, and values are arrays with the following keys:
   

   See note above.

3. +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleInfoInterface.php
   @@ -19,10 +23,17 @@ public function getAllBundleInfo();
   +   *   - uri_callback: The same as the 'uri_callback' key defined for the entity
   +   *     type in its definition, but for the bundle only. When determining the
   +   *     URI of an entity, if a 'uri_callback' is defined for both the entity
   +   *     type and the bundle, the one for the bundle is used.
   

   Isn't uri_callback deprecated?

4. +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleInfoInterface.php
   @@ -19,10 +23,17 @@ public function getAllBundleInfo();
   +   *   - translatable: (optional) A boolean value specifying whether this bundle
   

   boolean => Boolean

   Another (perhaps clearer) way to write this would be:

   TRUE if the bundle has translation support; FALSE (default) if not.

5. +++ b/core/lib/Drupal/Core/Entity/entity.api.php
   @@ -706,16 +706,9 @@ function hook_entity_view_mode_info_alter(&$view_modes) {
   + * @return array[]
   + *   The array has the same structure as the return value of
   + *   \Drupal\Core\Entity\EntityTypeBundleInfoInterface::getAllBundleInfo().
   

   See not above about the start of @return docs.

6. +++ b/core/lib/Drupal/Core/Entity/entity.api.php
   @@ -728,8 +721,9 @@ function hook_entity_bundle_info() {
   + *   The array has the same structure as the return value of
   + *   \Drupal\Core\Entity\EntityTypeBundleInfoInterface::getAllBundleInfo().
   

   And here too.

Our documentation is normally written in English, and the description of the return value normally starts by stating what the function returns. Why is this so much of a problem to ask that the way it was already documented is not removed, but added to?

Thanks! This looks pretty good. I have a few suggestions for improvement:

1. +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleInfo.php
   @@ -19,7 +19,9 @@ class EntityTypeBundleInfo implements EntityTypeBundleInfoInterface {
   +   * @var array[]
   +   *   The array has the same structure as the return value of
   +   *   \Drupal\Core\Entity\EntityTypeBundleInfoInterface::getAllBundleInfo().
   

   @var doesn't have a description, so this text should go above.

2. +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleInfoInterface.php
   @@ -10,8 +10,12 @@
   +   *   An array of all bundle information, of which keys are entity type IDs and
   +   *   values are arrays of which keys are bundle names, and values are arrays
   +   *   with the same structure as the return value of static::getBundleInfo().
   

   This is correct, but a little bit hard to read.

   Suggest something like:

   An associative array of bundle information. In the outer array, the keys are ... and the values are arrays whose keys are ... and whose values are ...

3. +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleInfoInterface.php
   @@ -19,10 +23,18 @@ public function getAllBundleInfo();
   +   *   An array of bundle information of which keys are bundle names and values
   +   *   are arrays with the following keys:
   

   of which -> whose

4. +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleInfoInterface.php
   @@ -19,10 +23,18 @@ public function getAllBundleInfo();
   +   *   - uri_callback: The same as the 'uri_callback' key defined for the entity
   

   So, uri_callback is deprecated for entities. Is this normally even in here?

Ah. Issue not done yet, apparently:
#2667040: Deprecate uri_callback in routes for entities

Re-rolled for 9.2

I am working on it.

Fixed the error from #27, attaching the patch.

Looks like a good docs clean-up!

Just a couple of points:

1. +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleInfoInterface.php
   @@ -24,11 +26,16 @@ public function getAllBundleInfo();
   +   * @return mixed[]
   

   Should be array[], since it's always an array of arrays?

2. +++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleInfoInterface.php
   @@ -24,11 +26,16 @@ public function getAllBundleInfo();
   +   *   - translatable: (optional) TRUE if the bundle has translation support,
   

   What does it mean for a return value to be optional? Does it mean that the 'translatable' key might sometimes not be there at all?

   If that is the case, then the docs should say what it means if it's not there. Presumably it's the same as FALSE?

I will work on this.

Thanks for the feedback @joachim.

Point 1- Has been addressed and uploading the patch.

Point 2: This has been already addressed:

+++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleInfoInterface.php
@@ -24,11 +26,16 @@ public function getAllBundleInfo();
+   *   - translatable: (optional) TRUE if the bundle has translation support,
+   *     FALSE (default) if not.

Please review.

Thanks

Thanks!

+++ b/core/lib/Drupal/Core/Entity/EntityTypeBundleInfoInterface.php
@@ -24,11 +26,16 @@ public function getAllBundleInfo();
+   *   - translatable: (optional) TRUE if the bundle has translation support,

However, this still says 'optional' in a return description.

What does it mean to say part of a return array is optional?

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

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

Attached patch against drupal 10.1.x

@joachim translatable key is set, only if the language modules are enabled and setup. Maybe that's the reason its marked optional.

Did a small review but don't think the MR was reviewed before being put into review as it contains tons of out of scope changes. Also this was a documentation change with code changes now.