Add first(), get($index) and possibly other methods to ListInterface

1. +++ b/core/lib/Drupal/Core/Entity/Query/Sql/Tables.php
   @@ -151,7 +151,7 @@ public function addField($field, $type, $langcode) {
   -            if (!$column && isset($propertyDefinitions[$relationship_specifier]) && $entity->{$field->getFieldName()}->get('entity') instanceof EntityReference) {
   +            if (!$column && isset($propertyDefinitions[$relationship_specifier]) && $entity->{$field->getFieldName()}->first()->get('entity') instanceof EntityReference) {
   

   This means we limit the "default reroute to the 1st item" sugar syntax strictly to $items->property_name ?
   Why not, just pointing that out.

   Also, this code here could then just be $entity->{$field_name}->entity ?

2. +++ b/core/lib/Drupal/Core/Entity/Query/Sql/Tables.php
   @@ -202,7 +202,7 @@ public function addField($field, $type, $langcode) {
   -        if (isset($propertyDefinitions[$relationship_specifier]) && $entity->{$specifier}->get('entity') instanceof EntityReference) {
   +        if (isset($propertyDefinitions[$relationship_specifier]) && $entity->get($specifier)->first()->get('entity') instanceof EntityReference) {
   

   Same here ?
   (and possibly in other places, didn't check all)

I think I personally could live with ItemList->get() being just about retrieving a list item by its key, and have the "implicit route to delta 0" DX sugar restricted to just $list->property.

But I'm not sure what the impact would be on the TypedData side ? I'm thinking of "generic code" like validation or normalization, that uses the $item as a TypedData and maybe relies on the current get() behavior ?

Oh - then yes, I'm +1 on that.
This is still for @fago to sign off, though. Assigning to him.

And for generic code, the new behavior IMHO makes more sense, because you don't want to special case the single value case anyway, you should foreach over each level taht is complexData or ListInterface.

Generic code would have no knowledge of get() on a list. Anyway I like the consistency brought to get method. If the little extra-magic within the magic doesn't bother you, it should be fine to others also. I'm not feeling strong about it either, so let's do it so then!

1. +++ b/core/lib/Drupal/Core/TypedData/ListInterface.php
   @@ -43,4 +43,34 @@ public function getItemDefinition();
   +   * @return
   +   *   The item at the specified position in this list.
   +   */
   +  public function get($index);
   

   Misses the return doc of returning a typed data object. Also, what does it return if the item does not exist?

   For field items we chose to always instantiate empty items, thus I guess we should document it that way here. If it's not set, then the returned object is empty (or contains NULL).

2. +++ b/core/lib/Drupal/Core/TypedData/ListInterface.php
   @@ -43,4 +43,34 @@ public function getItemDefinition();
   +  public function set($index, $item);
   

   Usually we do setters which return "static", so we can chain through. I'd suggest doing that here as well.

3. +++ b/core/lib/Drupal/Core/TypedData/ListInterface.php
   @@ -43,4 +43,34 @@ public function getItemDefinition();
   +   * @return
   

   Misses the interface, it's always a typed data object.

1. +++ b/core/lib/Drupal/Core/TypedData/ListInterface.php
   @@ -43,4 +43,38 @@ public function getItemDefinition();
   +   * @return self
   

   according to amateescu's experiences that should be static

2. +++ b/core/lib/Drupal/Core/Config/Schema/Sequence.php
   @@ -49,4 +50,26 @@ public function onChange($delta) {
   +    $this->offsetSet($index, $value);
   

   violates the interface as it misses the return.

Yep, thanks. Setting RTBC, assuming bot stays green.

28: list-methods-2110467-28.patch queued for re-testing.

Sequences seem to be the same as lists. I've no idea how the get() is intended to be used though, but given that it's just a list I'd assume it should map to the proposed meaning already.

all the typed config objects do that parsing, e.g. see the Mapping class. Not sure why it is required, but it seems to do nothing else than creating contained typed-data objects but just via the typed-config factory.

only semi-related, but shameless plug : #2164601: Stop auto-creating FieldItems on mere reading of $entity->field[N]

+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityFieldTest.php
@@ -616,3 +616,6 @@ protected function assertComputedProperties($entity_type) {
 }
+    $definition = FieldDefinition::create('entity_reference')
+      ->setLabel('Test entity')
+      ->setSetting('target_type', 'entity_test');
 

Is adding this here intentional?

Else, the patch looks good to me.

Needs a reroll

git ac https://drupal.org/files/issues/list-methods-2110467-41.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 30008  100 30008    0     0  21261      0  0:00:01  0:00:01 --:--:-- 24258
error: patch failed: core/lib/Drupal/Core/Config/Schema/Sequence.php:51
error: core/lib/Drupal/Core/Config/Schema/Sequence.php: patch does not apply

Plus...

+++ b/core/lib/Drupal/Core/Config/Schema/Sequence.php
@@ -8,6 +8,7 @@
+use Drupal\Core\TypedData\TypedDataInterface;

Unused use.

Hopefully the reroll is trivial again...

git ac https://drupal.org/files/issues/list-methods-2110467-44.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 29744  100 29744    0     0  15011      0  0:00:01  0:00:01 --:--:-- 16977
error: patch failed: core/lib/Drupal/Core/Field/FieldItemListInterface.php:77
error: core/lib/Drupal/Core/Field/FieldItemListInterface.php: patch does not apply
error: core/lib/Drupal/Core/Field/Plugin/Field/FieldType/LegacyConfigFieldItemList.php: does not exist in index

Committed 19e8cf3 and pushed to 8.x. Thanks!