Replace drupal_container() with Drupal::service() in the field module

Sorry, first timer here. Previous patch only updated the .module file, this one should handle all references to drupal_container in the field module.

Sure, remove the empty line.

Sweet

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.phpundefined
@@ -197,7 +197,7 @@ public function query($use_groupby = FALSE) {
-                                array(drupal_container()->get(Language::TYPE_CONTENT)->langcode, $default_langcode),
+                                array(\Drupal::service(Language::TYPE_CONTENT)->langcode, $default_langcode),

Well this is just plain weird... because drupal_container()->get(Language::TYPE_CONTENT) ain't going to return anything AFAICS as there is not service called 'language_content'

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.phpundefined
@@ -409,7 +409,7 @@ public function buildOptionsForm(&$form, &$form_state) {
-    if ($formatter = drupal_container()->get('plugin.manager.field.formatter')->getInstance($options)) {
+    if ($formatter = \Drupal::service('plugin.manager.field.formatter')->getInstance($options)) {

This is already injected into the plugin so $this->formatterPluginManager can be used...

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.phpundefined
@@ -838,7 +838,7 @@ function field_langcode(EntityInterface $entity) {
-                              array(drupal_container()->get(Language::TYPE_CONTENT)->langcode, $default_langcode),
+                              array(\Drupal::service(Language::TYPE_CONTENT)->langcode, $default_langcode),

as above... no 'language_content' service!

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/relationship/EntityReverse.phpundefined
@@ -62,7 +62,7 @@ public function query() {
-    $first_join = drupal_container()->get('plugin.manager.views.join')->createInstance($id, $first);
+    $first_join = \Drupal::service('plugin.manager.views.join')->createInstance($id, $first);

@@ -87,7 +87,7 @@ public function query() {
-    $second_join = drupal_container()->get('plugin.manager.views.join')->createInstance($id, $second);
+    $second_join = \Drupal::service('plugin.manager.views.join')->createInstance($id, $second);

As this is a views plugin I'm pretty sure we can and a static create method and a constructor to inject this...

Working on the issue.

Patch didn't apply. Straight reroll attached to the issue. Fixes from field.info.inc were moved to /core/modules/field/field.deprecated.inc.

function field_info_widget_types($widget_type = NULL) {
  if ($widget_type) {
    return drupal_container()->get('plugin.manager.field.widget')->getDefinition($widget_type);
  }
  else {
    return drupal_container()->get('plugin.manager.field.widget')->getDefinitions();
  }
}

function field_info_formatter_types($formatter_type = NULL) {
  if ($formatter_type) {
    return drupal_container()->get('plugin.manager.field.formatter')->getDefinition($formatter_type);
  }
  else {
    return drupal_container()->get('plugin.manager.field.formatter')->getDefinitions();
  }
}

Will continue working on the changes from comment #8 if the reroll applies.

Not sure if I did this right, but here's my try on the issue.

I found another instance of

\Drupal::service('plugin.manager.field.formatter')->getInstance($options)

as seen here:

diff --git a/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php
index ea74a94..82e153a 100644
--- a/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php
+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php
@@ -427,7 +439,7 @@ public function buildOptionsForm(&$form, &$form_state) {
     // Get the currently selected formatter.
     $format = $this->options['type'];
 
-    $settings = $this->options['settings'] + \Drupal::service('plugin.manager.field.formatter')->getDefaultSettings($format);
+    $settings = $this->options['settings'] + $this->formatterPluginManager('plugin.manager.field.formatter')->getDefaultSettings($format);

Another try, hopefully this time it'll be more like it's supposed to be.

#12: 2011082-field-drupal-service-12.patch queued for re-testing.

This should do it

Applied the patch and there's no usage of drupal_container() anymore in the field module, rtbc :)

Overall it looks good, just found some coding standards issues.

1. +++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php
   @@ -244,7 +255,7 @@ public function query($use_groupby = FALSE) {
            $langcode = str_replace(array('***CURRENT_LANGUAGE***', '***DEFAULT_LANGUAGE***'),
   -                                array(drupal_container()->get(Language::TYPE_CONTENT)->id, $default_langcode),
   +                                array($this->languageManager->getLanguage(Language::TYPE_CONTENT)),
                                    $this->view->display_handler->options['field_langcode']);
            $placeholder = $this->placeholder();
   

   This code block is badly indented, maybe fix this while we're at it?

2. +++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php
   @@ -848,7 +859,7 @@ function field_langcode(EntityInterface $entity) {
          $langcode = str_replace(array('***CURRENT_LANGUAGE***', '***DEFAULT_LANGUAGE***'),
   -                              array(drupal_container()->get(Language::TYPE_CONTENT)->id, $default_langcode),
   +                              array($this->languageManager->getLanguage(Language::TYPE_CONTENT)),
                                  $this->view->display_handler->options['field_language']);
    
   

   Same here.

3. +++ b/core/modules/field/lib/Drupal/field/Plugin/views/relationship/EntityReverse.php
   @@ -7,11 +7,12 @@
   +use Drupal\views\Plugin\ViewsHandlerManager;
    use Drupal\views\ViewExecutable;
    use Drupal\views\Plugin\views\display\DisplayPluginBase;
    use Drupal\views\Plugin\views\relationship\RelationshipPluginBase;
   

   Put this with the other plugins.

4. +++ b/core/modules/field/lib/Drupal/field/Plugin/views/relationship/EntityReverse.php
   @@ -23,6 +24,29 @@
   +   * Constructs an EntityReverse object.
   +   *
   +   * @param \Drupal\views\Plugin\ViewsHandlerManager $join_manager
   +   *   The views plugin join manager.
   +   */
   +    public function __construct(array $configuration, $plugin_id, array $plugin_definition, ViewsHandlerManager $join_manager) {
   +      parent::__construct($configuration, $plugin_id, $plugin_definition);
   +      $this->joinManager = $join_manager;
   +  }
   

   Indented too far.

Woops, crossposted with amateescu. I have quickly addressed the coding standards remarks I pointed out in my last comment. This has no changes except for whitespace fixes and ordering of namespaces, so setting this back to RTBC.

Is it me or the interdiff is backwards?

Ouch, you're right, it is backwards. I probably diffed from the older version and forgot to add the -R parameter.

No worries, the patch looks better and that's what matters :)

git ac https://drupal.org/files/2011082-field-drupal-service-20.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 10517  100 10517    0     0   6734      0  0:00:01  0:00:01 --:--:--  7665
error: patch failed: core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php:7
error: core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php: patch does not apply

Forgot the use statements.

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/relationship/EntityReverse.php
@@ -7,11 +7,12 @@
-use Drupal\Component\Annotation\PluginID;

Is this ok since the annotation uses @PluginID

Yep, because annotation classes are now loaded automatically, no need for use statements anymore.

Confirmed with @dawehener that the annotation reader will automatically load the classes.

Committed 00249db and pushed to 8.x. Thanks!

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php
@@ -243,9 +254,11 @@ public function query($use_groupby = FALSE) {
         $default_langcode = language_default()->id;
-        $langcode = str_replace(array('***CURRENT_LANGUAGE***', '***DEFAULT_LANGUAGE***'),
-                                array(drupal_container()->get(Language::TYPE_CONTENT)->id, $default_langcode),
-                                $this->view->display_handler->options['field_langcode']);
+        $langcode = str_replace(
+          array('***CURRENT_LANGUAGE***', '***DEFAULT_LANGUAGE***'),
+          array($this->languageManager->getLanguage(Language::TYPE_CONTENT)),
+          $this->view->display_handler->options['field_langcode']
+        );

I don't understand this change.
The second argument of the str_replace() call used to be an array with two values, and is now an array with one value.
This means ***DEFAULT_LANGUAGE*** gets now replaces by an empty string ?

Related, the new code leaves the former $default_langcode variable unused.

(two occurrences - query() and field_langcode() methods)

Mmm, you're right. So there's obviously no tests for this - tbh, not sure if I'm really ready for writing one here now too .. :/

Thanks!

33: 2011082-33.patch queued for re-testing.

33: 2011082-33.patch queued for re-testing.

Back to RTBC

33: 2011082-33.patch queued for re-testing.

Hm. Not crazy about committing this without test coverage TBH, but in the interest of closing this out, I'll go ahead and do it, but let's get a follow-up (can't tell if it's major or normal) for adding that test.

Committed and pushed to 8.x.