[meta] Dependency injection in Table.php

Created on 13 October 2024, 6 months ago

Phpstan shows two warnings for Table.php (both in the 2.0.x and 2.1.x branch):

 ------ ---------------------------------------------------------------------- 
  Line   src/Plugin/views/style/Table.php                                      
 ------ ---------------------------------------------------------------------- 
  603    \Drupal calls should be avoided in classes, use dependency injection instead                                                               
  1266   \Drupal calls should be avoided in classes, use dependency injection instead

What I would like to do in this issue is examine the cause of these two static \Drupal:: calls and see if they can be eliminated. I specifically do NOT want to be injecting services from other contributed modules like Commerce, because I don't want to REQUIRE a dependency on those modules.

The first static call is here (see #3040814: Grouping breaks Bulk Operations β†’ ):

    // Used for Views Bulk Operations to rename the checkboxes.
    $field_handlers = $this->view->field;
    $moduleHandler = \Drupal::service('module_handler');
    foreach ($field_handlers as $field_name => $handler) {
      if ($handler instanceof BulkForm ||
        ($moduleHandler->moduleExists('views_bulk_operations') && is_a($handler,
            '\\Drupal\\views_bulk_operations\\Plugin\\views\\field\\ViewsBulkOperationsBulkForm'))) {
        foreach ($this->rendered_fields as $num => $row) {
          $this->rendered_fields[$num][$field_name] = ViewsRenderPipelineMarkup::create('<!--form-item-' . $field_name . '--' . $num . '-->');
        }
      }
    }

In this case, I don't think we need to be using the module_handler service at all. The Views Bulk Operation module does NOT need to be enabled in order to check if a $handler is an instance of the class string '\\Drupal\\views_bulk_operations\\Plugin\\views\\field\\ViewsBulkOperationsBulkForm', so I think we can just delete the $moduleHandler variable and the moduleExists() check without affecting the operation of this code block.

The second static call is here:

        if (isset($this->commerce_field_values)) {
          if (array_key_exists($field_name, $this->commerce_field_values)) {
            if (in_array($col_operation, $commerce_operations)) {
              if (count($this->commerce_field_values[$field_name]) > 1) {
                $currency_formatter = \Drupal::service('commerce_price.currency_formatter');

The Commerce-specific integration in this module goes back to Drupal 7 and 2015, and it's not clear to me that this module is even used by Commerce anymore in Drupal 8+. Does this code even still work with Commerce? I wish we had tests, ESPECIALLY if we are going to have special exceptions for things.

It's not immediately obvious to me how "commerce" fields are determined and whether we are identifying all of them or not, but what is obvious is that we never check explicitly for whatever Commerce module provides this service (commerce:commerce_price I think). Like I said above, I do NOT want to always inject this service because that would mean adding a dependency.

In this case, I think the immediate solution is to tell Phpstan to ignore this call to the service.

πŸ“Œ Task
Status

Active

Version

2.1

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States tr Cascadia

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Production build 0.71.5 2024