Add EntityContextDefinition for the 80% use case

This looks GREAT.
I think it could use some dedicated tests, and it needs docblocks and such.
Only one point below

1. +++ b/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php
   @@ -348,14 +348,6 @@ protected function getSampleValues() {
   -    // @todo Move the entity specific logic out of this class in
   -    //   https://www.drupal.org/node/2932462.
   -    // If the data type is an entity, manually add one to the constraints array.
   -    if (strpos($this->getDataType(), 'entity:') === 0) {
   -      $entity_type_id = substr($this->getDataType(), 7);
   -      $constraint_definitions['EntityType'] = ['type' => $entity_type_id];
   -    }
   -
   

   <3

2. +++ b/core/lib/Drupal/Core/Plugin/Context/EntityContextDefinition.php
   @@ -0,0 +1,39 @@
   +    return new static($entity->getEntityType());
   

   Shouldn't this be return static::fromEntityType($entity->getEntityType());?

3. +++ b/core/modules/layout_builder/src/Entity/LayoutBuilderEntityViewDisplay.php
   @@ -150,9 +149,7 @@ public function buildMultiple(array $entities) {
   -        $contexts['layout_builder.entity'] = new Context(new ContextDefinition("entity:{$entity->getEntityTypeId()}", new TranslatableMarkup('@entity being viewed', ['@entity' => $entity->getEntityType()->getLabel()])), $entity);
   +        $contexts['layout_builder.entity'] = EntityContext::fromEntity($entity);
   

   <3

1. +++ b/core/lib/Drupal/Core/Plugin/Context/EntityContext.php
   @@ -0,0 +1,39 @@
   +   * Get a context definition from an entity type.
   ...
   +   * Get a context object from an entity.
   

   Gets

2. +++ b/core/lib/Drupal/Core/Plugin/Context/EntityContext.php
   @@ -0,0 +1,39 @@
   +   * @param $entity_type
   +   *   Entity type in which a definition will be derived.
   ...
   +  public static function fromEntityType($entity_type) {
   

   Is this a string or an object? Or both?

3. +++ b/core/lib/Drupal/Core/Plugin/Context/EntityContextDefinition.php
   @@ -0,0 +1,58 @@
   +    if (!($entity_type instanceof EntityTypeInterface)) {
   +      $entity_type = \Drupal::entityTypeManager()->getDefinition($entity_type);
   +    }
   

   Oh, both. Hmm. This might be a bit of overkill.

   If this switches to the object, please typehint EntityTypeInterface

1. +++ b/core/lib/Drupal/Core/Plugin/Context/EntityContextDefinition.php
   @@ -0,0 +1,90 @@
   +    $this->addConstraint('EntityType', [
   +      'type' => $this->getEntityTypeId(),
   +    ]);
   

   Should this check if (!$this->getConstraint('EntityType')) first?

2. +++ b/core/lib/Drupal/Core/Plugin/Context/EntityContextDefinition.php
   @@ -0,0 +1,90 @@
   +    return new EntityAdapter($definition);
   

   This needs to be a yield, this method has no return value

3. +++ b/core/lib/Drupal/Core/Plugin/Context/EntityContextDefinition.php
   @@ -0,0 +1,90 @@
   +   *   The entity type providing the context
   

   Missing trailing .

4. +++ b/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php
   --- a/core/tests/Drupal/Tests/Core/Plugin/Context/ContextDefinitionIsSatisfiedTest.php
   +++ b/core/tests/Drupal/Tests/Core/Plugin/Context/ContextDefinitionIsSatisfiedTest.php
   

   This test is covering \Drupal\Core\Plugin\Context\ContextDefinition, but all the entity-specific stuff has moved to a new class. So IMO a new test is needed, and anything entity-related should be removed from this test

Also, I think this deserves a CR.

Looks great! Thanks.
Just need the CR now.

The BC looks good.

1. +++ b/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php
   @@ -105,6 +121,18 @@ public static function create($data_type = 'any') {
   +    if (strpos($data_type, 'entity:') === 0) {
   

   I think this needs a !instanceof check for EntityContextDefinition, otherwise, when the new class is used, this code will still fire.

2. +++ b/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php
   @@ -309,33 +337,10 @@ public function isSatisfiedBy(ContextInterface $context) {
   -      return;
   +    if ($this->entityContextDefinition) {
   +      yield $this->entityContextDefinition->getSampleValues();
        }
   

   There is a missing return after the first yield.

3. +++ b/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php
   @@ -72,6 +67,27 @@ class ContextDefinition implements ContextDefinitionInterface {
   +   * This property should be kept private so that it is only accessible to this
   +   * class for backwards compatibility reasons. It will be removed in Drupal 9.
   

   I think this needs @deprecated or something, so that we remember to actually remove it

4. +++ b/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php
   @@ -72,6 +67,27 @@ class ContextDefinition implements ContextDefinitionInterface {
   +   * @see https://www.drupal.org/project/drupal/issues/2932462
   +   * @see https://www.drupal.org/node/2976400
   

   Please pick one format for the d.o links

+++ b/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php
@@ -302,7 +303,12 @@
-      $values = $definition->getSampleValues();
+      if ($this->entityContextDefinition) {
+        $values = $this->entityContextDefinition->getSampleValues();
+      }
+      else {
+        $values = $definition->getSampleValues();
+      }

@@ -337,10 +343,6 @@
   protected function getSampleValues() {
-    if ($this->entityContextDefinition) {
-      yield $this->entityContextDefinition->getSampleValues();
-    }
-
     yield $this->getTypedDataManager()->create($this->getDataDefinition());

This change feels wrong to me. Because now every call to getSampleValues would need this...

What's the reason we can't use the old way? Or yield from if that's needed?

I think the implementation here is great.
I have a couple questions/suggestions for DX (since that's the whole point of the issue).

1. +++ b/core/lib/Drupal/Core/Plugin/Context/ContextProviderInterface.php
   @@ -62,7 +62,7 @@ public function getRuntimeContexts(array $unqualified_context_ids);
   -   *   $context = new Context(new ContextDefinition('entity:node'));
   +   *   $context = new Context(new EntityContextDefinition('entity:node'));
   

   Shouldn't this change to EntityContext::fromEntityTypeId('node')?

   EDIT: we don't have that. But that seems useful, no?

2. +++ b/core/modules/layout_builder/src/Plugin/Derivative/FieldBlockDeriver.php
   @@ -118,9 +118,7 @@ public function getDerivativeDefinitions($base_plugin_definition) {
   -          $context_definition = new ContextDefinition('entity:' . $entity_type_id, $entity_type_labels[$entity_type_id], TRUE);
   +          $context_definition = new EntityContextDefinition('entity:' . $entity_type_id, $entity_type_labels[$entity_type_id], TRUE);
   
   +++ b/core/modules/layout_builder/tests/src/Kernel/FieldBlockTest.php
   @@ -207,7 +207,7 @@ protected function getTestBlock(ProphecyInterface $entity_prophecy, array $confi
   -        'entity' => new ContextDefinition('entity:entity_test', 'Test', TRUE),
   +        'entity' => new EntityContextDefinition('entity:entity_test', 'Test', TRUE),
   

   I would think this could be new EntityContextDefinition($entity_type_id, ...)
   without requiring the entity: prefix.

3. +++ b/core/modules/node/src/ContextProvider/NodeRouteContext.php
   @@ -65,7 +66,7 @@ public function getRuntimeContexts(array $unqualified_context_ids) {
   -    $context = new Context(new ContextDefinition('entity:node', $this->t('Node from URL')));
   +    $context = new EntityContext(new EntityContextDefinition('entity:node', $this->t('Node from URL')));
   

   This as well, ideally you could do EntityContext::fromEntityTypeId('node', $this->t('Node from URL'));

Overall my point is that we should look at the usages of this and be sure we really nailed the new class to provide good DX for the majority of use cases.

1. +++ b/core/lib/Drupal/Core/Plugin/Context/EntityContextDefinition.php
   @@ -16,6 +16,18 @@
   +    if (!preg_match('/^entity:/', $data_type)) {
   

   strpos($data_type, 'entity:') !== 0
   and skip out on the slower regex

2. +++ b/core/modules/block/tests/modules/block_test/src/ContextProvider/MultipleStaticContext.php
   @@ -46,11 +46,8 @@
   -    $context2 = EntityContext::fromEntity($current_user);
   -    $context2->getContextDefinition()->setLabel('User B');
   ...
   +    $context2 = EntityContext::fromEntity($current_user, 'User B');
   

   👍

3. +++ b/core/modules/layout_builder/tests/src/Kernel/FieldBlockTest.php
   @@ -207,7 +207,7 @@
   +        'entity' => new EntityContextDefinition('entity_test', 'Test', TRUE),
   
   +++ b/core/modules/node/src/ContextProvider/NodeRouteContext.php
   @@ -40,7 +40,7 @@
   +    $context_definition = EntityContextDefinition::create('node')->setRequired(FALSE);
   

   This is a confusing difference, IMO.
   Wait, is ::create() even a method?

Thanks for those changes.
This is really close!

1. +++ b/core/lib/Drupal/Core/Annotation/ContextDefinition.php
   @@ -116,7 +116,16 @@ public function __construct(array $values) {
   +    if (isset($values['class'])) {
   +      $class = $values['class'];
   +    }
   +    elseif (strpos($values['value'], 'entity:') === 0) {
   +      $class = 'Drupal\Core\Plugin\Context\EntityContextDefinition';
   +    }
   +    else {
   +      $class = 'Drupal\Core\Plugin\Context\ContextDefinition';
   +    }
   

   I think this deserves some code comments.
   Also could switch to the ::class versions

2. +++ b/core/lib/Drupal/Core/Plugin/Context/ContextDefinition.php
   @@ -72,6 +70,32 @@ class ContextDefinition implements ContextDefinitionInterface {
   +   * @deprecated
   
   @@ -111,6 +135,11 @@ public function __construct($data_type = 'any', $label = NULL, $required = TRUE,
   +      @trigger_error('Constructing a ContextDefinition object for an entity type is deprecated in Drupal 8.6.0. Use ' . __NAMESPACE__ . '\EntityContextDefinition instead. See https://www.drupal.org/node/2976400 for more information.', E_USER_DEPRECATED);
   

   The @deprecated needs to be expanded

   A @deprecated PHPdoc tag that indicates when the code was deprecated, when it will be removed, and what to use instead, and an @see link to the change record for the change.

3. +++ b/core/lib/Drupal/Core/Plugin/Context/EntityContextDefinition.php
   @@ -0,0 +1,117 @@
   +    return substr($this->getDataType(), 7);
   

   This could use a comment about the 7 corresponding to the length of 'entity:'

4. +++ b/core/modules/layout_builder/tests/src/Kernel/FieldBlockTest.php
   @@ -207,7 +207,7 @@ protected function getTestBlock(ProphecyInterface $entity_prophecy, array $confi
   -        'entity' => new ContextDefinition('entity:entity_test', 'Test', TRUE),
   +        'entity' => EntityContextDefinition::fromEntityTypeId('entity_test')->setLabel('Test'),
   

   Thought we lost something here, but that , TRUE matches the default value so I think it's fine.
   Just calling out in case someone else notices.

Could have aliased the import, but not really worth it in the long-run.
I think this is good to go!

This is being copied from \Drupal\Tests\Core\Plugin\Context\ContextDefinitionIsSatisfiedTest.
It'd be a shame to hold up this on something that unrelated.

Also in

\Drupal\Tests\Core\Render\Element\MachineNameTest
\Drupal\Tests\views\Unit\EntityViewsDataTest

And the !function_exists pattern in tests is in 17 other tests as well.

Rerolled without function_exists.

1. +++ b/core/lib/Drupal/Core/Annotation/ContextDefinition.php
   @@ -116,10 +116,36 @@ public function __construct(array $values) {
   +    elseif (strpos($values['value'], 'entity:') === 0) {
   

   Deja vu, but this can be an if. (see #57.1)

2. +++ b/core/modules/layout_builder/src/Entity/LayoutBuilderEntityViewDisplay.php
   @@ -150,9 +149,7 @@ public function buildMultiple(array $entities) {
   -        $contexts['layout_builder.entity'] = new Context(new ContextDefinition("entity:{$entity->getEntityTypeId()}", new TranslatableMarkup('@entity being viewed', ['@entity' => $entity->getEntityType()->getLabel()])), $entity);
   +        $contexts['layout_builder.entity'] = EntityContext::fromEntity($entity);
   

   This is now a UX regression for Layout Builder, as we lose the " being viewed" portion that was all that allowed users to differentiate between other contexts of the same entity type.

Also, here's an interdiff for the above. No idea how you got it to be 28k

BS selenium failure.

Opened #2986033: [regression] The BC layer for EntityContextDefinition in ContextDefinition is incomplete

#3126747: ContextDefinition::create() can no longer be used with an entitytype-specific datatype (like entity:user) was opened recently. Bringing this to the attention of those following this issue.