Add @trigger_error() to deprecated EntityManager->EntityRepository methods

1. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -368,6 +368,7 @@ public function getEntityTypeLabels($group = FALSE) {
   +    @trigger_error('EntityManagerInterface::getTranslationFromContext() is deprecated in 8.0.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Entity\EntityRepository::getTranslationFromContext() instead. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
   
   @@ -498,6 +499,7 @@ public function clearDisplayModeInfo() {
   +    @trigger_error('EntityManagerInterface::loadEntityByUuid() is deprecated in 8.0.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Entity\EntityRepository::loadEntityByUuid() instead. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
   
   @@ -511,6 +513,7 @@ public function loadEntityByUuid($entity_type_id, $uuid) {
   +    @trigger_error('EntityManagerInterface::loadEntityByConfigTarget() is deprecated in 8.0.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\Entity\EntityRepository::loadEntityByConfigTarget() instead. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
   
   +++ b/core/modules/views/src/Plugin/views/field/EntityOperations.php
   @@ -45,16 +67,24 @@ class EntityOperations extends FieldPluginBase {
    
   

   Shouldn't we be adding new deprecation tests for these?

   And the constructor 'you need to pass blah' ones too?

2. +++ b/core/lib/Drupal/Core/Entity/EntityReferenceSelection/SelectionTrait.php
   @@ -7,6 +7,8 @@
   +@trigger_error(__NAMESPACE__ . '\SelectionTrait is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0.', E_USER_DEPRECATED);
   

   We need a change record link in this message

3. +++ b/core/modules/aggregator/src/FeedViewBuilder.php
   @@ -15,21 +18,42 @@
   +  public function __construct(EntityTypeInterface $entity_type, EntityRepositoryInterface $entity_repository, LanguageManagerInterface $language_manager, Config $config, Registry $theme_registry, EntityDisplayRepositoryInterface $entity_display_repository, EntityTypeManagerInterface $entity_type_manager) {
   +    parent::__construct($entity_type, $entity_repository, $language_manager, $theme_registry, $entity_display_repository);
   

   Do you think we should do a big Change record listing all the constructor changes? Feels like too much to do one for each instance.

4. +++ b/core/modules/block/src/BlockViewBuilder.php
   @@ -6,58 +6,18 @@
   -   * The module handler.
   -   *
   -   * @var \Drupal\Core\Extension\ModuleHandlerInterface
   -   */
   -  protected $moduleHandler;
   -
   -  /**
   -   * Constructs a new BlockViewBuilder.
   -   *
   -   * @param \Drupal\Core\Entity\EntityTypeInterface $entity_type
   -   *   The entity type definition.
   -   * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
   -   *   The entity manager service.
   -   * @param \Drupal\Core\Language\LanguageManagerInterface $language_manager
   -   *   The language manager.
   -   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
   -   *   The module handler.
   -   */
   -  public function __construct(EntityTypeInterface $entity_type, EntityManagerInterface $entity_manager, LanguageManagerInterface $language_manager, ModuleHandlerInterface $module_handler) {
   -    parent::__construct($entity_type, $entity_manager, $language_manager);
   -    $this->moduleHandler = $module_handler;
   -  }
   -
   -  /**
   -   * {@inheritdoc}
   -   */
   -  public static function createInstance(ContainerInterface $container, EntityTypeInterface $entity_type) {
   -    return new static(
   -      $entity_type,
   -      $container->get('entity.manager'),
   

   note for further reviews:

   BlockViewBuilder extends from EntityHandlerBase, which has setModuleHandler method called on it from \Drupal\Core\Entity\EntityTypeManager::createHandlerInstance

   there is some technical debt there we have to pay down anyway - see #2471663: Undeprecate EntityHandlerBase

5. +++ b/core/modules/taxonomy/src/Form/OverviewTerms.php
   @@ -79,7 +101,8 @@ public static function create(ContainerInterface $container) {
          $container->get('entity.manager'),
   

   shouldn't this update to use entity type manager now?

6. +++ b/core/modules/user/src/Plugin/EntityReferenceSelection/UserSelection.php
   @@ -33,13 +36,6 @@ class UserSelection extends DefaultSelection {
   -  protected $userStorage;
   

   note to other reviewers: not used

7. +++ b/core/modules/views/src/Entity/Render/EntityTranslationRendererBase.php
   @@ -31,7 +31,7 @@ public function query(QueryPluginBase $query, $relationship = NULL) {
   -    $view_builder = $this->view->rowPlugin->entityManager->getViewBuilder($this->entityType->id());
   
   +++ b/core/modules/views/src/Entity/Render/TranslationLanguageRenderer.php
   @@ -80,7 +80,7 @@ protected function getLangcodeTable(QueryPluginBase $query, $relationship) {
   -    $view_builder = $this->view->rowPlugin->entityManager->getViewBuilder($this->entityType->id());
   

   ouch, talking to your neighbour's neighbour's neighbour's neighbour

1. +++ b/core/lib/Drupal/Core/Entity/EntityReferenceSelection/SelectionTrait.php
   @@ -7,7 +7,7 @@
   +@trigger_error(__NAMESPACE__ . '\SelectionTrait is deprecated in drupal:8.7.0 and will be removed before drupal:9.0.0.', E_USER_DEPRECATED);
   

   I think we need a CR link in this message

2. +++ b/core/modules/menu_ui/src/Tests/MenuWebTestBase.php
   @@ -46,7 +46,7 @@ public function assertMenuLink($menu_plugin_id, array $expected_item) {
   -      $entity = \Drupal::service('entity.repository')->loadEntityByUuid('menu_link_content', $uuid);
   +      $entity = \Drupal::entityManager()->loadEntityByUuid('menu_link_content', $uuid);
   

   is this intended?

Don't we need deprecation tests for the constructor arg trigger_errors?

Fair enough, will chat with other committers to confirm

We discussed the CR and agreed that we should have a CR summary of the overall constructor change pattern and what it applies to, with an example, rather than listing them all.

I think its time for second set of committer eyes here

I agree with not testing the constructor deprecations (unless it's for a value object or similar) because we're already going over and above adding those deprecations in the first place for @internal code. tbh I'm also not sure about a change record, because the trigger_error() documents the change as well as a change record can, and this is code we don't expect people to have to change - so it can make important change records harder to find. But consolidating in one is a reasonable compromise.

Patch looks right to me.

+++ b/core/modules/editor/src/Plugin/Filter/EditorFileReference.php
@@ -25,11 +25,11 @@
   /**
-   * An entity manager object.
+   * The entity repository.
    *
-   * @var \Drupal\Core\Entity\EntityManagerInterface
+   * @var \Drupal\Core\Entity\EntityRepositoryInterface
    */
-  protected $entityManager;
+  protected $entityRepository;

I think this should use DeprecatedServicePropertyTrait

Couldn't spot anything else.

Given all the PHP 5.5 trait issues triggered a test against PHP 5.5

Committed e69e249 and pushed to 8.7.x. Thanks!

This has not been pushed yet.