Rename hook_entity_info/alter() to hook_entity_type_build/alter()

+++ b/core/modules/field/tests/modules/field_test/field_test.entity.incundefined
@@ -9,9 +9,9 @@
+function field_test_entity_info_build() {

Just for consistency this maybe should take the &$entity_info as parameter.

+++ b/core/modules/translation_entity/lib/Drupal/translation_entity/EntityTranslationControllerInterface.phpundefined
@@ -16,8 +16,8 @@
  * The entity translation UI relies on the entity info to provide its features.
- * See the documentation of hook_entity_info() in the Entity API documentation
- * for more details on all the entity info keys that may be defined.
+ * See the documentation of \Drupal\Core\Entity\Annotation\EntityType for more
+ * details on all the entity info keys that may be defined.

I guess on the long run we want to remove this extra translation parameters from EntityType to the info_build hook?

Great, so let's fix this small bit and RTBC it.

Weird. I really don't understand this at all.

I was told the reason we introduced the horrifically convoluted annotations system in the first place was the DX benefit of keeping the declarations for things above the things they're defining. Killing hook_entity_info() was supposed to be part of this. Regardless of what it's ultimately called, if we keep the concept of hook_entity_info() around, isn't that working against the entire point of doing annotations in the first place? If I still have to look in at least two places anyway to figure out all of the "info," I'd much rather just kill annotations altogether (in favour of YAML or something) and thus one of the 6-odd ways there are of declaring things in D8.

The more logical (albeit, I'm hand-waving here) thing to do would be to create a ViewsUI interface or something, that has these defaults declared on it as annotations, like all the other plugins that have defaults defined, and then somehow pull that onto Views structure (I assumed that'd be through a hook_entity_info_alter(), and don't understand why doing it that way would prevent contrib from doing things too).

Existence of hook_entity_info() in D8 is very confusing, since it's meaning has significantly changed from D7; so I second proposal to rename hook_entity_info() for better DX

Tagging

Please don't add the "needs change notification" tag as long the change is not committed yet.

Re-rerolled.

+++ b/core/lib/Drupal/Core/Entity/EntityManager.php
@@ -143,7 +143,7 @@ public function __construct(\Traversable $namespaces, ContainerInterface $contai
+    $this->discovery = new InfoHookDecorator($this->discovery, 'entity_info_build');
...
-    $this->discovery = new InfoHookDecorator($this->discovery, 'entity_info');

In the new world order shouldn't this be hook_entity_type_build()?

Yep, hook_entity_type_build($entity_types) and hook_entity_type_alter(&$entity_types) I'd say.

Here's a patch for this.

Want to get #2165155: Change $entity_type to $entity_type_id and $entity_info to $entity_type/$entity_types in first and there's also a entity url related patch somewhere conflicting with this, but I wanted to get started.

Completely new patch, that now does the following:

- Rename hook_entity_info($entity_info) to hook_entity_type_build($entity_types)
- Rename hook_entity_info_alter() to hook_entity_type_alter($entity_types)
- Drop the unused (!) entity_get_info()
- Also drop the unfortunately still used entity_info_clear_cache(). Maybe that should be a separate issue (to check if some calls are really still needed) but it was close to entity_get_info() and I wanted to see if this works.

16: core-1981858-16-rename_entity_info.patch queued for re-testing.

Ok, reverted the cache stuff, unit tests are failing due to the drupal_static() call. Let's fix that first.

The test result looks like this because fatal errors in unit tests result in this :(

Manually grepped the test output for other fails, update.php was broken because the special handling for entity_info in UpdateModuleHandler didn't apply anymore. Fixed that.

Ok, re-roll and fixed content_translation, used the wrong hook name in the alter.

Updating title.

1. +++ b/core/modules/action/action.module
   @@ -71,11 +71,11 @@ function action_menu_link_defaults() {
   +function action_entity_type_build(&$entity_types) {
   
   +++ b/core/modules/block/custom_block/custom_block.module
   @@ -136,15 +136,15 @@ function custom_block_load($id) {
   +function custom_block_entity_type_alter(&$entity_types) {
   
   +++ b/core/modules/book/book.module
   @@ -112,11 +112,11 @@ function book_permission() {
   +function book_entity_type_build(&$entity_types) {
   

   While we are here, can we explicit typehint them with array but also keep the @var ?

2. +++ b/core/modules/system/entity.api.php
   @@ -99,18 +99,18 @@ function hook_ENTITY_TYPE_create_access(\Drupal\Core\Session\AccountInterface $a
   +function hook_entity_type_build(array &$entity_types) {
   
   @@ -122,20 +122,20 @@ function hook_entity_info(&$entity_info) {
   +function hook_entity_type_alter(&$entity_types) {
   

   Interesting we typehint here but not there.

Yes, I started doing that but didn't complete.

Weird, I didn't mean to set this to RTBC, sorry :)

Thank you!

core-1981858-26-rename_entity_info.patch no longer applies.

error: patch failed: core/modules/config_translation/config_translation.module:85
error: core/modules/config_translation/config_translation.module: patch does not apply

Re-roll, also noticed that I didn't rename $entity_info before for config and content translation.

30: core-1981858-30-rename_entity_info.patch queued for re-testing.

Missed a use of $entity_type as string there.

Created https://drupal.org/node/2196275, this should be ready again :)

Looks good to me.

35: core-1981858-35-rename_entity_info.patch queued for re-testing.

core-1981858-35-rename_entity_info.patch no longer applies.

error: patch failed: core/modules/field_ui/field_ui.module:130
error: core/modules/field_ui/field_ui.module: patch does not apply

Rerolled.

Re-roll looks good, thanks!

Committed c210e86 and pushed to 8.x. Thanks!

Made a little fix during commit...

diff --git a/core/modules/field/lib/Drupal/field/Tests/FieldInfoTest.php b/core/modules/field/lib/Drupal/field/Tests/FieldInfoTest.php
index dbaad54..32ffe10 100644
--- a/core/modules/field/lib/Drupal/field/Tests/FieldInfoTest.php
+++ b/core/modules/field/lib/Drupal/field/Tests/FieldInfoTest.php
@@ -317,7 +317,7 @@ function testFieldInfoCache() {
     // field_test_entity_type_build(). Ensure the test field is still in the returned
     // array.
     field_info_cache_clear();
-    \Drupal::state()->set('field_test.clear_info_cache_in_hook_entity_info', TRUE);
+    \Drupal::state()->set('field_test.clear_info_cache_in_hook_entity_type_build', TRUE);
     $fields = field_info_fields();
     $this->assertTrue(isset($fields[$field->uuid]), 'The test field is found in the array returned by field_info_fields() even if its cache is cleared while being rebuilt.');
   }
diff --git a/core/modules/field/tests/modules/field_test/field_test.entity.inc b/core/modules/field/tests/modules/field_test/field_test.entity.inc
index 81044de..2d6a96b 100644
--- a/core/modules/field/tests/modules/field_test/field_test.entity.inc
+++ b/core/modules/field/tests/modules/field_test/field_test.entity.inc
@@ -13,7 +13,7 @@ function field_test_entity_type_build() {
   /** @var $entity_info \Drupal\Core\Entity\EntityTypeInterface[] */
   // If requested, clear the field cache while this is being called. See
   // Drupal\field\Tests\FieldInfoTest::testFieldInfoCache().
-  if (\Drupal::state()->get('field_test.clear_info_cache_in_hook_entity_info')) {
+  if (\Drupal::state()->get('field_test.clear_info_cache_in_hook_entity_type_build')) {
     field_info_cache_clear();
   }
 }