Get rid of field_get_items()

Totally :)

Agreed - totally ;-)

+++ b/core/modules/node/node.tokens.inc
@@ -137,17 +137,18 @@ function node_tokens($type, $tokens, array $data = array(), array $options = arr
+          if (($items = $node->getTranslation($langcode)->body) && !$items->isEmpty()) {

Currently this is correct. The end goal is passing around the correct translation object, so we don't have to instantiate it here.

Well, in this case (node_token()), both the $node and the $langcode are received as entries in the $options param.

I think we should simply get rid of the $langcode options entry: if you can specify it, you can even pass the correct translation object. Otherwise things will apply to the default language which is the intended behavior.

Anyway, as you say, this seems the right code for the current state of HEAD ?

Well, now that I look more closely I think there would be a problem if $langcode is NULL (and right now it can): we can either replace the NULL value in line 101 with Language::LANGCODE_DEFAULT or make EntityNG::getTranslation() interpret a NULL value as Language::LANGCODE_DEFAULT. Actually I am not sure we want to encourage passing unchecked langcode values.

crosspost

Hmm. I'm not changing the way tokens work in this patch :-)

No, sure, I was just thinking loud :)

+++ b/core/modules/file/file.module
@@ -1975,8 +1975,8 @@ function file_get_file_references(File $file, $field = NULL, $age = FIELD_LOAD_R
+          if (!$match && ($items = $entity->get($field_name))) {

+++ b/core/modules/node/node.tokens.inc
@@ -137,7 +137,7 @@ function node_tokens($type, $tokens, array $data = array(), array $options = arr
+          if (($items = $node->getTranslation($langcode)->get('body')) && !$items->isEmpty()) {

Overall this looks good to me, can you explain why these changes were needed? Aren't __get() and get() functionally the same when we come to regular fields?

I find this quite confusing, BTW ($entity->$field_name returns plain arrays, $entity->get($field_name) returns objects) :-)

I think the BC decorator is the culprit (as usual), EntityNG::get($field_name) and EntityNG::__get($field_name) should return exactly the same value (a field item list object).

I think we should mark this @deprecated now we're in API freeze. It's not a one-line wrapper so it'll need to go before release but we should give people some warning.

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/views/argument_default/Tid.phpundefined
@@ -139,8 +139,8 @@ public function getArgument() {
-            foreach (field_get_items($node, $name) as $item) {
-              $taxonomy[$item['tid']] = $field_info['settings']['allowed_values'][0]['vocabulary'];
+            foreach ($node->{$name} as $item) {
+              $taxonomy[$item->tid] = $field_info['settings']['allowed_values'][0]['vocabulary'];

I wouldn't be surprised if this would be broken too but we just don't have any test coverage...

#18: field_get_items-2051721-18.patch queued for re-testing.

@yched we don't have a way (yet). I opened #2042165: Add a 'deprecated' module that includes deprecated functions only when enabled to discuss stuff that's left in.

Back to RTBC?

Committed/pushed to 8.x, thanks!

Needs change notice for the deprecation.

Looks good :)