Remove usage of language_list()

Still we have some places where this function used near about 20 files on fresh D8 instance.

Thanks
Sidhartha

I will work on this @ DUG Belgium.

Remove function and other lines to used language_list function.

Missing a function.

Hello @keopx
Thank you for your patch, it is good and tests are passing.

Here you have some nitpicks that will help to improve the final code:

1. +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php
   @@ -1161,7 +1161,7 @@ protected function loadFieldItems(array $entities) {
   -    $langcodes = array_keys(language_list(LanguageInterface::STATE_ALL));
   +    $langcodes = array_keys(\Drupal::languageManager()->getLanguages(LanguageInterface::STATE_ALL));
   

   You can inject \Drupal::LanguageManager as a dependency. You can see Drupal\Core\Config\Entity\ConfigEntityStorage as an example.

2. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -100,7 +100,7 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En
   -    $languages = language_list();
   
   @@ -133,7 +133,7 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En
   -      foreach (language_list(LanguageInterface::STATE_CONFIGURABLE) as $language) {
   +      foreach (\Drupal::languageManager()->getLanguages(LanguageInterface::STATE_CONFIGURABLE) as $language) {
   
   @@ -146,7 +146,7 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En
   -      foreach (language_list(LanguageInterface::STATE_CONFIGURABLE) as $language) {
   +      foreach (\Drupal::languageManager()->getLanguages(LanguageInterface::STATE_CONFIGURABLE) as $language) {
   

   You are calling \Drupal::languageManager()->getLanguages(); three times in the same method. You can extract it to a variable and reuse it.

3. +++ b/core/modules/language/src/Form/LanguageDeleteForm.php
   @@ -96,7 +96,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   -    $languages = language_list();
   +    $languages = \Drupal::languageManager()->getLanguages();
   

   You can inject $this->languageManager as a dependency.

4. +++ b/core/modules/language/src/Form/LanguageEditForm.php
   @@ -49,7 +49,7 @@ public function actions(array $form, FormStateInterface $form_state) {
   -    $languages = language_list();
   +    $languages = \Drupal::languageManager()->getLanguages();
   

   You can use $this->languageManager->getLanguages(); here.

5. +++ b/core/modules/language/src/Form/NegotiationBrowserForm.php
   @@ -60,7 +60,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   -    $languages = language_list();
   +    $languages = \Drupal::languageManager()->getLanguages();
   

   Same as above.

6. +++ b/core/modules/language/src/Form/NegotiationUrlForm.php
   @@ -69,7 +69,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   -    $languages = language_list();
   +    $languages = \Drupal::languageManager()->getLanguages();
   

   You can inject $this->languageManager as a dependency.

7. +++ b/core/modules/language/src/Plugin/Condition/Language.php
   @@ -31,7 +31,7 @@ class Language extends ConditionPluginBase {
        if (\Drupal::languageManager()->isMultilingual()) {
          // Fetch languages.
   -      $languages = language_list(LanguageInterface::STATE_CONFIGURABLE);
   +      $languages = \Drupal::languageManager()->getLanguages(LanguageInterface::STATE_CONFIGURABLE);
   

   You are calling \Drupal::languageManager() twice in the same function. You can extract it to a variable and reuse it.

8. +++ b/core/modules/locale/src/Form/TranslateEditForm.php
   @@ -32,7 +32,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   -    $languages = language_list();
   +    $languages = \Drupal::languageManager()->getLanguages();
   

   You can use $this->languageManager here.

9. +++ b/core/modules/locale/src/Form/TranslateFormBase.php
   @@ -161,7 +161,7 @@ protected function translateFilters() {
   -    $languages = language_list();
   +    $languages = \Drupal::languageManager()->getLanguages();
   

   Same as above.

Thank you again for your time.

reroll patch

reroll

Add LanguageManagerInterface and this variable.

reroll

I don't understand correctly problem...

Hi Keopx,

Thank you for your patch.

I did a review of your patch and I think i found the error :

1. In Core/modules/comment/src/CommentStorage.php
This class also extends from the SqlContentEntityManager, so you should also pass the language_manager in the parent::__construct function.

2. The same for core/modules/user/src/UserStorage.php

3.

+++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php
@@ -128,7 +136,8 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
       $entity_type,
       $container->get('database'),
       $container->get('entity.manager'),
-      $container->get('cache.entity')
+      $container->get('cache.entity'),
+      \Drupal::languageManager()        
     );
   }

In the createInstance function in files userStorage, CommentStorage and SqlContentEntityManager instead of using \Drupal::languageManager() you should use $container->get('language_manager').

4.There are some whitespaces issues in your patch :

/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityManager.php line 124 and 141

/core/lib/Drupal/Core/Session/UserSession.php line 105

For more information, visit https://www.drupal.org/coding-standards#indenting

Hi,

Thanks for review patch.

This file doesn't exist /core/lib/Drupal/Core/Entity/Sql/SqlContentEntityManager.php

Yes, I've configured IDE with coding standards, but I think didn't do some actions when format.

The last patch works fine, but doesn't pass test.

So, I do interdiff between patch #2328293-9: Remove usage of language_list() and #2328293-25: Remove usage of language_list().

Here my new patch.

Hi keopx, we are closer now :)
Just some coding standards and nitpicks.

1. +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php
   @@ -119,7 +120,13 @@ class SqlContentEntityStorage extends ContentEntityStorageBase implements SqlEnt
   +  ¶
   +  /**
   

   Minor: trailing whitespace.

2. +++ b/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php
   @@ -155,12 +163,13 @@ public function getFieldStorageDefinitions() {
   -  public function __construct(EntityTypeInterface $entity_type, Connection $database, EntityManagerInterface $entity_manager, CacheBackendInterface $cache) {
   +  public function __construct(EntityTypeInterface $entity_type, Connection $database, EntityManagerInterface $entity_manager, CacheBackendInterface $cache, LanguageManagerInterface $language_manager) {
   

   You need to add the $language_manager param to the docblock.

3. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -100,7 +100,8 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En
   +    $language_manager = \Drupal::languageManager();       ¶
   

   Trailing whitespace

4. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -133,7 +134,7 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En
   -      foreach (language_list(LanguageInterface::STATE_CONFIGURABLE) as $language) {
   
   @@ -146,7 +147,7 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En
   -      foreach (language_list(LanguageInterface::STATE_CONFIGURABLE) as $language) {
   +      foreach ($language_manager->getLanguages(LanguageInterface::STATE_CONFIGURABLE) as $language) {
   

   LanguageInterface::STATE_CONFIGURABLE is the default value for $flags parameter, so you can call to $language_manager->getLanguages()

5. +++ b/core/modules/language/src/Form/LanguageDeleteForm.php
   @@ -27,6 +28,13 @@ class LanguageDeleteForm extends EntityConfirmFormBase {
   +  ¶
   

   Trailing whitespace

6. +++ b/core/modules/language/src/Form/NegotiationUrlForm.php
   @@ -69,7 +69,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   -    $languages = language_list();
   +    $languages = \Drupal::languageManager()->getLanguages();
   
   @@ -98,7 +98,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   -    $languages = language_list();
   +    $languages = \Drupal::languageManager()->getLanguages();
   

   You can inject LanguageManager here.

7. +++ b/core/modules/language/src/Plugin/Condition/Language.php
   @@ -29,9 +29,10 @@ class Language extends ConditionPluginBase {
   +      $languages = $language_manager->getLanguages(LanguageInterface::STATE_CONFIGURABLE);
   

   You can remove the LanguageInterface::STATE_CONFIGURABLE parameter

Apart from that, your last patch is failing because you have to take into account that Drupal\comment\CommentStorage and Drupal\user\UserStorage extends \Drupal\Core\Entity\Sql\SqlContentEntityStorage and overrides the constructor. So you have to include the language_manager in those classes.
Here you have an example for CommentStorage:

/**
   * Constructs a CommentStorage object.
   *
   * @param \Drupal\Core\Entity\EntityTypeInterface $entity_info
   *   An array of entity info for the entity type.
   * @param \Drupal\Core\Database\Connection $database
   *   The database connection to be used.
   * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
   *   The entity manager.
   * @param \Drupal\Core\Session\AccountInterface $current_user
   *   The current user.
   * @param \Drupal\Core\Cache\CacheBackendInterface $cache_backend
   *   Cache backend instance to use.
   * @param \Drupal\Core\Language\LanguageManagerInterface $language_manager
   *   The language manager.
   */
  public function __construct(EntityTypeInterface $entity_info, Connection $database, EntityManagerInterface $entity_manager, AccountInterface $current_user, CacheBackendInterface $cache, LanguageManagerInterface $language_manager) {
    parent::__construct($entity_info, $database, $entity_manager, $cache, $language_manager);
    $this->currentUser = $current_user;
  }

  /**
   * {@inheritdoc}
   */
  public static function createInstance(ContainerInterface $container, EntityTypeInterface $entity_info) {
    return new static(
      $entity_info,
      $container->get('database'),
      $container->get('entity.manager'),
      $container->get('current_user'),
      $container->get('cache.entity'),
      $container->get('language_manager')
    );
  }

Hope this helps. Thank you for your effort!

Reroll

I think that is not possible.
6.

+++ b/core/modules/language/src/Form/NegotiationUrlForm.php
@@ -69,7 +69,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
-    $languages = language_list();
+    $languages = \Drupal::languageManager()->getLanguages();
@@ -98,7 +98,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
-    $languages = language_list();
+    $languages = \Drupal::languageManager()->getLanguages();

You can inject LanguageManager here.

I need to change unit test to add new variable languageManager.

Changes works fine, but construct need this variable and unit test doesn't have.

Congrats keopx, your patch looks really good :)

To get all the things sorted out, I think you should inject LanguageManager as a dependency in \Drupal\language\Form\NegotiationUrlForm. It is injected in all the other forms.

I will take a look at the test changes later.

PS: An interdiff would be very appreciated to make the patch revision easier.

Thank you again!

Here interdiff and new patch.

Hello, your improvements in this patch look good.
However, I think you have to do a couple of changes in the Depencency Injection implementation in \Drupal\language\Form\NegotiationUrlForm.

+++ b/core/modules/language/src/Form/NegotiationUrlForm.php
@@ -17,6 +19,32 @@
+  public function __construct(LanguageManagerInterface $language_manager) {
+    $this->languageManager = $language_manager;
+  }
...
+  public static function create(ContainerInterface $container) {
+    return new static(
+      $container->get('language_manager')
+    );
+  }

Given that extends from ConfigFormBase and you should maintain the $configFactory in the class constructor, as it is done in \Drupal\language\Form\NegotiationBrowserForm. This property is necessary for config() method

Also in line 129 of NegotiationUrlForm you have a call to the static \Drupal::languageManager()->getLanguages(); that should be replaced too.

Finally, patch does not apply now, so we need a re-roll here.

Thank you!

I do changes and reroll.

mmm the last patch passed test...

Well, I do new version, with reroll, install correctly and pass test.

:-/

Thank you @keopx.
Patch looks really good now.

You only missed the second comment in #35.

public function validateForm(array &$form, FormStateInterface $form_state) {
    $languages = \Drupal::languageManager()->getLanguages();

Should use the class property $this->languageManager instead of the static method in NegotiationUrlForm.

Apart of that, I think this patch could be RTBC'ed.

Thank you again @keopx for your time and persistence :)

correct mistake

And thank @plopesc :D

I think this is OK now
Let's see if someone else has any objection.

1. +++ b/core/includes/bootstrap.inc
   @@ -1286,25 +1286,6 @@ function drupal_installation_attempted() {
    /**
   - * Returns a list of languages set up on the site.
   - *
   - * @param $flags
   - *   (optional) Specifies the state of the languages that have to be returned.
   - *   It can be: LanguageInterface::STATE_CONFIGURABLE,
   - *   LanguageInterface::STATE_LOCKED, LanguageInterface::STATE_ALL.
   - *
   - * @return array
   - *   An associative array of languages, keyed by the language code, ordered by
   - *   weight ascending and name ascending.
   - *
   - * @deprecated in Drupal 8.x-dev, will be removed before Drupal 8.0.
   - *   Use \Drupal::languageManager()->getLanguages().
   - */
   -function language_list($flags = LanguageInterface::STATE_CONFIGURABLE) {
   -  return \Drupal::languageManager()->getLanguages($flags);
   -}
   

   Can we remove the usages before removing the function. Also we need to link this to the change record https://www.drupal.org/node/2174591

   We will then remove the function in a separate issue. This makes patch conflict resolution much easier as HEAD is less likely to break.

2. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -103,7 +103,8 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En
   +    $language_manager = \Drupal::languageManager();
   

   It'd be nice to inject this. Which means ContentTranslationHandler needs to extend Drupal\Core\Entity\EntityHandlerInterface and we should just refactor content_translation_controller() out of existence and use the entity manager getHandler() instead.

3. +++ b/core/modules/language/src/Plugin/Condition/Language.php
   @@ -29,9 +29,10 @@ class Language extends ConditionPluginBase {
   -    if (\Drupal::languageManager()->isMultilingual()) {
   +    $language_manager = \Drupal::languageManager();
   +    if ($language_manager->isMultilingual()) {
   

   This can be injected see \Drupal\node\Plugin\Condition\NodeType

I am working on this issue

I updated points 2 and 3 in comment #43. For point 1, there are too many places that call "function language_list"

For example:

// Load field data.
    $langcodes = array_keys(language_list(LanguageInterface::STATE_ALL));
    foreach ($storage_definitions as $field_name => $storage_definition) {
      $table = $load_current ? $table_mapping->getDedicatedDataTableName($storage_definition) : $table_mapping->getDedicatedRevisionTableName($storage_definition);

      // Ensure that only values having valid languages are retrieved. Since we
      // are loading values for multiple entities, we cannot limit the query to
      // the available translations.
      $results = $this->database->select($table, 't')
        ->fields('t')
        ->condition($load_current ? 'entity_id' : 'revision_id', $ids, 'IN')
        ->condition('deleted', 0)
        ->condition('langcode', $langcodes, 'IN')
        ->orderBy('delta')
        ->execute();

In file: \core\lib\Drupal\Core\Entity\Sql\SqlContentEntityStorage.php (line 1220)

If I delete language_list function above, what will happen to $langcodes ??

Created straight reroll of patch in comment 41. I haven't addressed the changes as suggested by alexpott in comment 43.

So I addressed point 1 and 3 of comment 43.

I was not quite sure how to do point 2, but I will try it later today again. Patch attached.

Created follow-up issue: #2354681: Remove language_list() from bootstrap.inc

All right I think i got it. Patch attached!

1. +++ b/core/lib/Drupal/Core/Session/UserSession.php
   @@ -183,7 +183,7 @@ public function isAnonymous() {
   -    $language_list = language_list();
   +    $language_list = \Drupal::languageManager()->getLanguages();
   

   This should use an injected language managed (it did in #22), as can getPreferredAdminLangcode.

2. +++ b/core/modules/content_translation/content_translation.module
   @@ -322,24 +322,6 @@ function content_translation_enabled($entity_type, $bundle = NULL) {
   - * Content translation controller factory.
   - *
   - * @param string $entity_type_id
   - *   The type of the entity being translated.
   - *
   - * @return \Drupal\content_translation\ContentTranslationHandlerInterface
   - *   An instance of the content translation controller interface.
   - *
   - * @todo Move to \Drupal\content_translation\ContentTranslationManager.
   - */
   -function content_translation_controller($entity_type_id) {
   -  $entity_type = \Drupal::entityManager()->getDefinition($entity_type_id);
   -  // @todo Throw an exception if the key is missing.
   -  $class = $entity_type->getHandlerClass('translation');
   -  return new $class($entity_type);
   -}
   -
   -/**
     * Checks whether a content translation is accessible.
     *
     * @param \Drupal\Core\Entity\EntityInterface $entity
   @@ -357,7 +339,7 @@ function content_translation_controller($entity_type_id) {
   
   @@ -357,7 +339,7 @@ function content_translation_controller($entity_type_id) {
     * @todo Move to \Drupal\content_translation\ContentTranslationManager.
     */
    function content_translation_access(EntityInterface $entity, $op) {
   -  return content_translation_controller($entity->getEntityTypeId())->getTranslationAccess($entity, $op) ;
   +  return \Drupal::entityManager()->getHandler($entity->getEntityTypeId(), 'translation')->getTranslationAccess($entity, $op);
   

   There are several changes to content_translation_controller that seem out of scope for this issue.

3. +++ b/core/modules/content_translation/src/ContentTranslationHandler.php
   @@ -104,7 +127,8 @@ public function entityFormAlter(array &$form, FormStateInterface $form_state, En
   +    $language_manager = $this->languageManager;
   +    $languages = $language_manager->getLanguages();
   

   There's no need for a $language_manager local variable, just use $this->language_manager each time, it isn't much longer. (this probably dates from code that didn't just an injected manager)

4. +++ b/core/modules/node/src/Plugin/views/field/Node.php
   @@ -73,7 +73,7 @@ protected function renderLink($data, ResultRow $values) {
   -          $languages = language_list();
   +          $languages = \Drupal::languageManager()->getLanguages();
   

   This should be injected

I've not picked out everything I spotted, but the above comments should cover the general themes. As a rule, if you're in a class and not in a static method, you should be calling $this->languageManager. Any changes other obviously necessary to replace language_list() should be explained in the issue summary. If you're not sure how to change something, there's an earlier patch at #2225427: Remove remaining uses of deprecated language functions (mostly in object oriented code) that might help.

It looks like we need to create a UserSession before we can get a language manager, so I guess we'll need to stick with \Drupal for now in that class.

Test fixes

1. +++ b/core/lib/Drupal/Core/Session/UserSession.php
   @@ -6,6 +6,7 @@
   +use Drupal\Core\Language\LanguageManagerInterface;
   

   No longer needed

2. +++ b/core/modules/node/src/Plugin/views/field/Node.php
   @@ -25,6 +27,43 @@
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
   +    return new static(
   +      $configuration,
   +      $plugin_id,
   +      $plugin_definition,
   +      $container->get('language_manager')
   +    );
   +  }
   

   It doesn't look like anything will call create(). I'm not sure if we can inject services into field plugins (none of the other ones do).

Attached patch changes the above and fixes some comments. There are no other calls to language_list().

Created re-roll.

Patch attached.

Reroll patch and interdiff with changes, but not realy interdiff. Please see it.

Clear my patch, works fine @JeroenT

Works fine @JeroenT patch

Patch still applies.

Updated issue summary, tagging for D8MI.

1. +++ b/core/lib/Drupal/Core/Session/UserSession.php
   @@ -183,7 +183,8 @@ public function isAnonymous() {
   -    $language_list = language_list();
   +    $language_manager = \Drupal::languageManager();
   +    $language_list = $language_manager->getLanguages();
   
   @@ -196,7 +197,8 @@ function getPreferredLangcode($fallback_to_default = TRUE) {
   -    $language_list = language_list();
   +    $language_manager = \Drupal::languageManager();
   +    $language_list = $language_manager->getLanguages();
   

   The variable assignment to $language_manager here is a waste. Just do $language_list = \Drupal::languageManager()->getLanguages().

2. +++ b/core/modules/content_translation/content_translation.module
   @@ -243,9 +243,10 @@ function content_translation_translate_access(EntityInterface $entity) {
    function content_translation_controller($entity_type_id) {
      $entity_type = \Drupal::entityManager()->getDefinition($entity_type_id);
   +  $language_manager = \Drupal::languageManager();
      // @todo Throw an exception if the key is missing.
      $class = $entity_type->getHandlerClass('translation');
   -  return new $class($entity_type);
   +  return new $class($entity_type, $language_manager);
    }
   

   I think we should replace all the code in content_translation_controller with

     return \Drupal::entityManager()->getHandler($entity_type_id, 'translation');
   

   This wrapper should be removed because it is pointless and only adds maintenance overhead but that is separate issue.

asdsa

#74 Changes

@ashutoshsngh please, the interdiff file is good to see changes.

Do you use https://www.drupal.org/node/2328293#comment-9478605 patch base?

Interdiff attached.
I have used https://www.drupal.org/node/2328293#comment-9478611 as batch base.

@ashutoshsngh

Use #64 issue patch, because I did not add this lines.

diff -u b/core/modules/content_translation/src/ContentTranslationHandler.php b/core/modules/content_translation/src/ContentTranslationHandler.php
--- b/core/modules/content_translation/src/ContentTranslationHandler.php
+++ b/core/modules/content_translation/src/ContentTranslationHandler.php
@@ -181,7 +181,7 @@
       $language_select = &$language_widget['widget'][0]['value'];
       if ($language_select['#type'] == 'language_select') {
         $options = array();
-        foreach ($this->languageManager->getLanguages() as $language) {
+        foreach (\Drupal::languageManager()->getLanguages() as $language) {
           // Show the current language, and the languages for which no
           // translation already exists.
           if (empty($translations[$language->getId()]) || $language->getId() == $entity_langcode) {

And another thing.

Your add some non coding standars lines. Review your interdiff file. Example:

   db_delete('content_translation')
-    ->condition('entity_type', $entity->getEntityTypeId())
-    ->condition('entity_id', $entity->id())
-    ->execute();
+      ->condition('entity_type', $entity->getEntityTypeId())
+      ->condition('entity_id', $entity->id())
+      ->execute();
 }

Fixed all spaces issues according to #79.

+++ b/core/lib/Drupal/Core/Session/UserSession.php
@@ -196,7 +196,8 @@ function getPreferredLangcode($fallback_to_default = TRUE) {
-    $language_list = language_list();
+    $language_manager = \Drupal::languageManager();
+    $language_list = $language_manager->getLanguages();

Still need to fix getPreferredAdminLangcode()

I compared #64 and #82. Comments from #74 have been addressed, but #64 replaced a call to \Drupal in ContentTranslationHandler line 161 that has been lost.

Other than I would have set to RTBC.

Done

#84 fixes that one remaining issue, thanks.

Beta evaluation is in the meta issue. Committed 338e966 and pushed to 8.0.x. Thanks!

Amazing, thanks!

Marked #2328463: Language condition plugin needs LanguageManager injected, unit tests added as duplicate of this issue.