Apply defaults to definition objects

Having definitions be the go-to objects for asking about information sounds like a nice idea in principle. That's basically what FieldDefinitionInterface::getSchema() is doing.

Some thoughts though:

- "Smart logic" like "merging from an outside source" raises the question of statically caching the results, meaning we then need to be cautious about serialization.

- Getters that are not symmetrical with setters feel a bit slippery API-wise. Means get(set()) is not stricly idempotent. You can only "get()" the merged version, not "what the caller of set() just placed in there". Shaky for stuff that ends up saved in Field / Instance config entities (you save the merged version, not strictly "the definition").
getSchema() escapes that, since that's purely derived data, with no associated setter.

- Having multiple potential sources for the info is indeed confusing. "Who should I ask then ? Who has which version ?" - e.g for constraints:
$data->getConstraints() ?
$definition->getConstraints() ?
$manager->getConstraints($data) ?
It's probably a bit clearer if $data::getConstraints() is a static - at least shows you're not supposed to call it through a $data object (although I guess it still shows up in IDEs autocomplete for "$data->" ?)

But right, settling on a pattern of :
static FieldItem::schema($field_definition)
static FieldItem::getPropertyDefinitions($field_definition)
static FieldItem::getConstraints($field_definition)
would make sense - although, er, would that mean schema() should be getSchema() ?

- We probably can't apply that pattern everywhere.
FieldDefinition::getDisplayOptions(), added in #2144919: Allow widgets and formatters for base fields to be configured in Field UI, for example: it only returns "striclty the defaults that were defined in setDisplayOptions()", not the actual resolved runtime display options, those are held in an EntityDisplay, we probably want to add EntityDisplays to the mix here :-)

Would it help if we made a list of "things" we'd want to apply that pattern to:
- class
- constraints
- schema
- ?

See #2272161: Entity-based conditions should use type => entity:$entitytype instead of constraints for a), I think that can go in on its own, and we can handle b) here...?

Okay, b) should work for my needs.

Here we are. This test should illustrate why I had to postpone #2272161: Entity-based conditions should use type => entity:$entitytype instead of constraints.

That's wrong. DataDefinition is not the right class for all kind of types, in particular not for entities. use $typed_data_manager->createDataDefinition() and "entity:node" will give you all the wanted constraints.

That's fine, but we need some form of test here, so that we can safely make the change in #2272161: Entity-based conditions should use type => entity:$entitytype instead of constraints

Okay, sounds good.

I avoided making the default TRUE because I didn't want to track down all of those instances, but its definitely better.

Coming here from #2236903: setSettings() on field definitions can unintentionally clear default values.

The issue summary does not at all clarify the use-case for setting $apply_defaults to FALSE. I.e. why this is needed in the first place. The boilerplate documentation in the patch does not reveal any information either, as to when I would want to use which. Marking needs work for that at least.

The confusing-ness of the different getConstraints() methods is a thing, but I'm not sure this is the solution. I.e. when I have a data definition and I want to get a list of constraints why would I only want those specifically set up for this particular field definition. I want all constraints that are applicable to this field definition. If I don't want the defaults to apply, I can remove them with setConstraints() (or if that doesn't work that's a separate bug, at least), but I don't see a use-case for this separation.

For getting a single constraint I find this even stranger, as a consumer, as there shouldn't be a difference between a constraint that is default or not default.

The places where this is changed in code are not commented, so that is not very helpful either.

Can't really formulate whether it's connected to this needs for "with defaults / without defaults", but I have a vague feeling that we're using the same API for two things here:
- get a list of "constraint definitions" (arrays) - either those that are implicitly enforced by the TypedDataManager, and / or those that are additionally explicitly set by the data definition at hand.
- get a list of actual, runtime Constraint objects, always instantiated from *all* of above, in order to feed to the Validation API.

The fact that the @return phpdoc for some xxConstraint() methods mentions "Constraint objects", and others "arrays ready to create Constraint objects" feels a bit like an API smell.

Again, sorry, a bit vague, maybe it's not related at all, feel free to ignore.

I think the points of #21 are valid, but I don't think this issue makes the situation that much worse.
I think switching according to #18/#19 was the right way to go, thanks @fago!

Straight PSR-4 reroll

26: typeddata-2116341-26.patch queued for re-testing.

Good looking out.

Committed/pushed to 8.x, thanks!

there's still 2 @todo left in code

$ git grep 2116341
core/lib/Drupal/Core/Field/BaseFieldDefinition.php:63:    // @todo Cleanup in https://drupal.org/node/2116341.
core/lib/Drupal/Core/Field/BaseFieldDefinition.php:476:   * https://drupal.org/node/2116341.

This was marked fixed but BaseFieldDefinition::create() still has a "@todo cleanup in [this issue]" ?

// Create a definition for the items, and initialize it with the default
// settings for the field type.
// @todo Cleanup in https://drupal.org/node/2116341.
$field_type_manager = \Drupal::service('plugin.manager.field.field_type');
$default_settings = $field_type_manager->getDefaultStorageSettings($type) + $field_type_manager->getDefaultFieldSettings($type);
$field_definition->itemDefinition->setSettings($default_settings);

Also in BaseFiedDefinition (although that one is more "@todo cleanup after [this issue] is fixed"

/**
   * Helper to retrieve the field item class.
   *
   * @todo: Remove once getClass() adds in defaults. See
   * https://drupal.org/node/2116341.
   */
  protected function getFieldItemClass() {
    if ($class = $this->getItemDefinition()->getClass()) {
      return $class;
    }
    else {
      $type_definition = \Drupal::typedDataManager()
        ->getDefinition($this->getItemDefinition()->getDataType());
      return $type_definition['class'];
    }
  }

Still not completed still see @todo in the code

I opened a follow-up for what reported in the last comment.