Properly deprecate entity_create() and remove all occurrences

Looks everything covered

Bot failure

Is there an issue to add the trigger_error to entity_create?

Then we know it is gone (for good)?

1. +++ b/core/includes/entity.inc
   @@ -306,6 +306,7 @@ function entity_delete_multiple($entity_type, array $ids) {
     */
    function entity_create($entity_type, array $values = []) {
   +  @trigger_error('entity_create() is deprecated in Drupal 8.6.0 and will be removed before Drupal 9.0.0. Use needed entity type class directly or \Drupal::entityTypeManager()->getStorage($entity_type)->create($values) instead.', E_USER_DEPRECATED);
      return \Drupal::entityManager()
        ->getStorage($entity_type)
   

   I'm not sure what the rule now is. Obviously this will trigger a lot of deprecations in contrib, do we now no longer care about that since they are not reported anymore by testbot (for now)?

2. +++ b/core/modules/field/src/Tests/reEnableModuleFieldTest.php
   @@ -95,7 +95,7 @@ public function testReEnabledField() {
        // uninstallation of a module.
   -    $field_storage2 = entity_create('field_storage_config', [
   +    $field_storage2 = \Drupal::entityTypeManager()->getStorage('field_storage_config')->create([
          'field_name' => 'field_telephone_2',
   

   this could also use FieldStorageConfig::create() but either is fine.

Looks ok to me too.

Obviously this will trigger a lot of deprecations in contrib

- yep that's the point and it will power #2822727: [policy, no patch] Adopt a continuous API upgrade path for major version changes

+++ b/core/includes/entity.inc
@@ -306,6 +306,7 @@ function entity_delete_multiple($entity_type, array $ids) {
+  @trigger_error('entity_create() is deprecated in Drupal 8.6.0 and will be removed before Drupal 9.0.0. Use needed entity type class directly or \Drupal::entityTypeManager()->getStorage($entity_type)->create($values) instead.', E_USER_DEPRECATED);

This needs to reference the change record in which we deprecated it. See the 'how we deprecate' section in https://www.drupal.org/core/deprecation

@larowlan,

Looks like there is no explicit change of record for the same, https://www.drupal.org/project/drupal/issues/2490966 , Do we need to add one ?

@rakesh.gectcr yes, it needs new one like we dong for database.inc and others from #2999721: [META] Deprecate the legacy include files before Drupal 9

Working on related I think better to file new CR to notify contrib that now deprecation will throw

There's existing CR which I did update with new references https://www.drupal.org/node/2266845
This method is deprecated it in #2346261: Deprecate entity_create() in favor of a <EntityType>::create($values) or \Drupal::entityManager()->getStorage($entity_type)->create($values)

Meanwhile closed #2932317: Replace stray documentation references to entity_create() as there's only few leftovers in docs which added to patch

Here's reroll and clean-up of doc blocks, also added deprecation test
No interdiff because it's bigger then patch

+++ b/core/scripts/generate-d7-content.sh
@@ -140,7 +140,7 @@
-    $term = entity_create('taxonomy_term', array(
+    $term = \Drupal::entityTypeManager()->getStorage('taxonomy_term')->create([

this is d7 script which should not be changed

fix missed coding standard

+++ b/core/modules/content_translation/tests/src/Functional/ContentTranslationSyncImageTest.php
@@ -123,9 +123,8 @@ public function testImageFieldSync() {
-    $entity = entity_create($this->entityTypeId, $values);
+    $entity = $this->createEntity($values, $default_langcode);

This was wrong, cos it returns ID instead of entity

1. +++ b/core/modules/block_content/src/Tests/Views/BlockContentTestBase.php
   @@ -58,7 +58,7 @@ protected function setUp($import_test_views = TRUE) {
       * @param array $settings
       *   (optional) An associative array of settings for the block_content, as
   -   *   used in entity_create().
   +   *   used to create entity.
   
   +++ b/core/modules/block_content/tests/src/Functional/Views/BlockContentTestBase.php
   @@ -53,7 +53,7 @@ protected function setUp($import_test_views = TRUE) {
       *
       * @param array $settings
       *   (optional) An associative array of settings for the block_content, as
   -   *   used in entity_create().
   +   *   used to create entity.
   

   I think the second part here doesn't really make sense anymore. A whole lot about this seems very strange, the variable name is completely bogus.

   Not sure if it is out of scope, but I'd suggest to just rename it to $values and just say "The values for the block_content entity"

2. +++ b/core/modules/node/tests/src/Traits/NodeCreationTrait.php
   @@ -41,7 +41,7 @@ public function getNodeByTitle($title, $reset = FALSE) {
       *
       * @param array $settings
       *   (optional) An associative array of settings for the node, as used in
   -   *   entity_create(). Override the defaults by specifying the key and value
   +   *   creation of entity. Override the defaults by specifying the key and value
       *   in the array, for example:
       *   @code
   

   Ah, I guess this settings stuff in block_content was copied from node, equally weird here.

3. +++ b/core/modules/user/tests/src/Traits/UserCreationTrait.php
   @@ -89,7 +89,7 @@ protected function createUser(array $permissions = [], $name = NULL, $admin = FA
       * @param int $weight
   -   *   (optional) The weight for the role. Defaults NULL so that entity_create()
   +   *   (optional) The weight for the role. Defaults NULL so that created entity
       *   sets the weight to maximum + 1.
   
   @@ -116,7 +116,7 @@ protected function createAdminRole($rid = NULL, $name = NULL, $weight = NULL) {
       * @param int $weight
   -   *   (optional) The weight for the role. Defaults NULL so that entity_create()
   +   *   (optional) The weight for the role. Defaults NULL so that created entity
       *   sets the weight to maximum + 1.
   

   should be "so that *the* created entity", but it's still weird, because the entity was then already created and isn't able to "set" something. Maybe just:

   Defaults to NULL, which sets the weight to maximum + 1?

Addressed Berdir's feedback and re-rolled. I went ahead and changed the variable names from $settings to $values in all three instances for clarity. Interdiff might be off slightly, it's between this patch and the initial reroll.

+++ b/core/modules/user/tests/src/Traits/UserCreationTrait.php
@@ -212,8 +212,8 @@ protected function createUser(array $permissions = [], $name = NULL, $admin = FA
    *   (optional) The label for the role. Defaults to a random string.
    * @param int $weight
-   *   (optional) The weight for the role. Defaults NULL so that entity_create()
-   *   sets the weight to maximum + 1.
+   *   (optional) The weight for the role. Defaults NULL which sets the weight
+   *   to maximum + 1.
    *
    * @return string

@@ -239,7 +239,7 @@ protected function createAdminRole($rid = NULL, $name = NULL, $weight = NULL) {
    * @param int $weight
-   *   (optional) The weight for the role. Defaults NULL so that entity_create()
+   *   (optional) The weight for the role. Defaults NULL so that created entity
    *   sets the weight to maximum + 1.
    *

The new variables are so much better IMHO. Looks you missed on of the two cases here, though. Also, Defaults *to* NULL I think?

Yep, I missed that.

Thanks, looks good now I think.

1. +++ b/core/includes/entity.inc
   @@ -291,10 +291,12 @@ function entity_delete_multiple($entity_type, array $ids) {
   +  @trigger_error('entity_create() is deprecated in Drupal 8.0.0 and will be removed before Drupal 9.0.0. Use create() method of the entity type class directly or \Drupal::entityTypeManager()->getStorage($entity_type)->create($values) instead. See https://www.drupal.org/node/2266845', E_USER_DEPRECATED);
   

   Should this be 'Use the create method'?

2. +++ b/core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php
   @@ -87,8 +87,6 @@ public static function createFromBaseFieldDefinition(BaseFieldDefinition $base_f
   -   * @see entity_create()
   

   should we reference the replacement method? not fussed

2. I was thinking about that too, not sure. You can clearly see that the constructors of those 3-4 cases have been copied from each other, ContentLanguageSettings even still references FieldConfig::create(). The base class doesn't have anything like that, there also seem other things that are outdated, like mentions of a BC layer but also mismatches on field_name vs name vs id. Since it does reference the create method above (even if wrongly sometimes), I think removing that is fine. IMHO the only one that is a bit special is FieldConfig::__construct() because that allows different versions (field storage or field name + entity type) some aren't real properties, so there isn't really a good place to document that. For the others, I'd even consider just replacing the whole thing with an {@inheritdoc}.

Conflicted on EntityLegacyTest, reroll of that, just two methods being added in the same place. Also added the *the*.

I only did a reroll and added a word to the deprecation message so I think it is OK if I set it back to RTBC. We'll need someone to decide on #30.2, I provided background in #31.

Committed 9b1690c and pushed to 8.8.x. Thanks!