Remove hook_field_[CRUD]_() in favor of hook_ENTITY_TYPE_[CRUD]() calls

+++ b/core/modules/entity_reference/entity_reference.module
@@ -192,33 +192,35 @@ function entity_reference_field_settings_form($field, $instance, $has_data) {
+function entity_reference_entity_update_update(EntityInterface $entity) {

This doesn't look right :)

Thanks for this !

+++ b/core/modules/entity_reference/entity_reference.moduleundefined
@@ -192,33 +192,35 @@ function entity_reference_field_settings_form($field, $instance, $has_data) {
-function entity_reference_field_update_field($field, $prior_field, $has_data) {
-  if ($field['type'] != 'entity_reference') {
-    // Not an entity reference field.
-    return;
-  }
+function entity_reference_entity_update(EntityInterface $entity) {

For performance, I guess it's recommended to rather implement hook_[ENTITY_TYPE]_[OP]() ?

The "generic" hook_entity_[OP]() are there for the cases where you want to react for all config entities, or all fieldable entities...

(also applies to the other hook migrations in the patch ?)

+++ b/core/modules/entity_reference/entity_reference.moduleundefined
@@ -192,33 +192,35 @@ function entity_reference_field_settings_form($field, $instance, $has_data) {
+  if ($entity->entityType() == 'field_entity' && $entity->type == 'entity_reference') {
+    if ($entity['type'] != 'entity_reference') {

Something looks weird here. $entity->type is checked twice with BC/array and object syntax ?

Since this code is de facto new code, I'd say it should not continue using the BC array syntax. $entity['foo'] definitely looks weird :-)
(same for the new image_entity_[op]() implementations)

+++ b/core/modules/entity_reference/entity_reference.moduleundefined
@@ -192,33 +192,35 @@ function entity_reference_field_settings_form($field, $instance, $has_data) {
-  $field_name = $field['field_name'];
+    $field_name = $$entity['field_name'];

double $$
(and switch to ->id() ?)

+++ b/core/modules/field/lib/Drupal/field/Plugin/Core/Entity/Field.phpundefined
@@ -372,6 +384,7 @@ public function save() {
       $has_data = field_has_data($this);
+      $this->hasData = $has_data;

That's a property that only exists at a specific point in the object lifecycle (during save() if updating an existing field entity), but is documented as always being present. We should move field_has_data() to a method on the Field entity IMO.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Core/Entity/Field.phpundefined
@@ -347,13 +361,11 @@ public function save() {
+      $this->originalField = $original;

Not great either, for the same reasons. Less sure about how we can make that better, though.

This looks like a more general issue with hook_entity_update().
I opened #2017809: how do hook_entity_update() implementations access the "previous" state of the entity ? for this.

No better suggestion for now, let's leave it that way here, I guess...
(same for FieldInstance::$originalInstance - maybe at least name both properties just "$original" ?)

Nitpick: I think core tends to avoid starting function bodies with an empty line.

hook_[ENTITY_TYPE]_[OP]() :
see ConfigStorageController::invokeHook()

Nope, /me won't shoot @swentel

OK, let's go with a followup for hasData.

IMO we could write:

function entity_reference_field_entity_update(EntityInterface $field) {

which would make code in the func body much easier to understand.

Maybe even :

function entity_reference_field_entity_update(FieldInterface $field) {

but I'm less sure about that.

Looks good :-)
RTBC if green.

"Argument 1 passed to field_test_field_instance_delete() must be an instance of FieldInstanceInterface, instance of Drupal\field\Plugin\Core\Entity\FieldInstance given"

This looks like a missing "use" statement for FieldInstanceInterface in field_test.something :-)

... if green

Related : #2020895: Move save() / delete() logic in Field / FieldInstance to [pre|post]Save(), [pre|post]Delete()...
(this one is still RTBC, though)

Rerolled.

Needed some more updates for #1818568: Convert files to the new Entity Field API.

Needs a reroll

curl https://drupal.org/files/2013939-31.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 26745  100 26745    0     0  11895      0  0:00:02  0:00:02 --:--:-- 16053
error: patch failed: core/modules/entity_reference/entity_reference.module:169
error: core/modules/entity_reference/entity_reference.module: patch does not apply

Actually entity_reference_field_update_field is the conflicting function and there's no "has data" check there, so here's a re-roll #31

Removing has data reference mentioned in #35

#36: 2013939-36.patch queued for re-testing.

#36: 2013939-36.patch queued for re-testing.

Back to RTBC. Sorry for the noise. Just wanted to make sure it still passed after other Field API commits in the last 12 hours.

Committed 5ff3e98 and pushed to 8.x. Thanks!

Change notification added at https://drupal.org/node/2054619

More accurate title for posterity