Fix the issues reported by phpcs

Created on 19 February 2023, almost 2 years ago
Updated 29 August 2023, about 1 year ago

Running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig shows the following warnings/errors, which should be fixed if they are not false positive and do not suggest wrong changes.

FILE: ./gifplayer/src/Form/SettingsForm.php
-------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------
 7 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Config\ConfigFactoryInterface.
-------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------

FILE: ./gifplayer/src/Plugin/Field/FieldType/GifPlayer.php
-------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------
 6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is
   |       |     Drupal\Core\Field\FieldStorageDefinitionInterface.
-------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------

FILE: ./gifplayer/src/Plugin/Field/FieldType/GifPlayerVideoMode.php
-------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------
 6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is
   |       |     Drupal\Core\Field\FieldStorageDefinitionInterface.
-------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------

FILE: ./gifplayer/src/Plugin/Field/FieldFormatter/GifPlayerVideoFormatter.php
-------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------
 31 | WARNING | #options values usually have to run through t() for translation
-------------------------------------------------------------------------

FILE: ./gifplayer/src/Plugin/Field/FieldFormatter/GifPlayerFormatter.php
-------------------------------------------------------------------------
FOUND 1 ERROR AND 7 WARNINGS AFFECTING 7 LINES
-------------------------------------------------------------------------
   7 | ERROR   | [x] Use statements should be sorted alphabetically. The first wrong one is
     |         |     Drupal\Core\Field\Plugin\Field\FieldFormatter\EntityReferenceFormatterBase.
  43 | WARNING | [ ] #options values usually have to run through t() for translation
  44 | WARNING | [ ] #options values usually have to run through t() for translation
 168 | WARNING | [ ] Line exceeds 80 characters; contains 106 characters
 178 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 181 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 181 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 195 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------

FILE: ./gifplayer/gifplayer.module
-------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------
 9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Link.
-------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------

Time: 188ms; Memory: 10MB
📌 Task
Status

Fixed

Version

1.0

Component

Code

Created by

🇮🇳India indrapatil Bangalore

Live updates comments and jobs are added and updated live.
  • Coding standards

    It involves compliance with, or the content of coding standards. Requires broad community agreement.

Sign in to follow issues

Comments & Activities

  • Issue created by @indrapatil
  • 🇮🇳India indrapatil Bangalore

    I will work on it.

  • Assigned to indrapatil
  • Status changed to Needs work almost 2 years ago
  • Issue was unassigned.
  • Status changed to Active almost 2 years ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
  • 🇮🇳India Akram Khan Cuttack, Odisha

    there are no Fixable error of phpcs

  • Status changed to Needs review almost 2 years ago
  • 🇮🇳India Akram Khan Cuttack, Odisha
  • 🇮🇳India Akram Khan Cuttack, Odisha

    OOps it's my mistake wrongly check so move to Active

  • Status changed to Active almost 2 years ago
  • 🇮🇳India Akram Khan Cuttack, Odisha
  • Status changed to Needs review almost 2 years ago
  • 🇮🇳India Akram Khan Cuttack, Odisha

    Added updated patch and remove all PHPCS issues

  • Status changed to Needs work almost 2 years ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
    -    // Load imagcover from settings if this is set.\Drupal::config('gifplayer.settings')->get('cover_png')
    +    // Load imagcover from settings if this is set.\Drupal::config
    +    // ('gifplayer.settings')->get('cover_png')
    

    That is the wrong way to split code.
    There is a missing space after the period.
    The code after the period does not make much sense.

         if ($cid !== 0 && $file = $file_storage->load($cid)) {
    -      $img_cover = \Drupal::service('file_url_generator')->generateAbsoluteString($file->getFileUri());
    +      $img_cover = $this->service('file_url_generator')->generateAbsoluteString($file->getFileUri());
         }
    

    There is no service() method.

  • 🇮🇳India kkalashnikov Ghaziabad, India
  • Status changed to Needs review over 1 year ago
  • 🇮🇳India kkalashnikov Ghaziabad, India
  • Assigned to danrod
  • 🇨🇦Canada danrod Ottawa

    I am having this issue after I applied the patch and tried to add a new "Gif Player" field:

    TypeError: Drupal\gifplayer\Plugin\Field\FieldFormatter\GifPlayerFormatter::__construct(): Argument #3 ($field_definition) must be of type Drupal\gifplayer\Plugin\Field\FieldFormatter\FieldDefinitionInterface, Drupal\field\Entity\FieldConfig given, called in /var/www/html/web/modules/contrib/gifplayer/src/Plugin/Field/FieldFormatter/GifPlayerFormatter.php on line 104 in Drupal\gifplayer\Plugin\Field\FieldFormatter\GifPlayerFormatter->__construct() (line 71 of /var/www/html/web/modules/contrib/gifplayer/src/Plugin/Field/FieldFormatter/GifPlayerFormatter.php)

  • Issue was unassigned.
  • Status changed to Needs work over 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
       /**
    +   * File url generator object.
    +   *
    +   * @param \Drupal\Core\File\FileUrlGenerator $fileUrlGenerator
    +   *   File url generator object.

    url in the description is misspelled. Since it is an acronym, it must be spelled in capital letters.
    The definite article is missing at the beginning of the description.

    +  /**
    +   * The list of available modules.
    +   *
    +   * @var \Drupal\Core\Extension\ModuleExtensionList
    +   */
    +  protected $extensionListModule;
    +   * @param \Drupal\Core\Extension\ModuleExtensionList $extension_list_module
    +   *   The list of available modules.

    The description Drupal core use for that property/parameter is The module extension list.

    +  /**
    +   * Configuration Factory.
    +   *
    +   * @var \Drupal\Core\Config\ConfigFactory
    +   * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
    +   *   Config Factory Interface.

    A definite article is missing from the descriptions.
    Only the first word must be capitalized.

    +  /**
    +   * Construct a MyFormatter object.
    +   *

    The description for a constructor must start with Constructs a new followed by the class name (including its namespace), and end with object.
    The class name is not correct, since that comment is for the GifPlayerFormatter class.

    +   * @param string $plugin_id
    +   *   The plugin_id for the plugin instance.

    plugin_id must be plugin ID.

    +   * @param \Drupal\Core\Field\FieldDefinitionInterface $field_definition
    +   *   Defines an interface for entity field definitions.

    That description is not correct: An object does not define an interface, but its class implements an interface.
    The correct description is The definition of the field to which the formatter is associated.

    +  public function __construct(
    +    $plugin_id,
    +    $plugin_definition,
    +    FieldDefinitionInterface $field_definition,
    +    array $settings,
    +    $label,
    +    $view_mode,
    +    array $third_party_settings,
    +    FileUrlGenerator $fileUrlGenerator,
    +    ModuleExtensionList $extension_list_module,
    +    ConfigFactoryInterface $configFactory
    +    ) {

    As per Drupal coding standards, a function/method declaration must be written on a single line.

    -    // Load imagcover from settings if this is set.\Drupal::config('gifplayer.settings')->get('cover_png')
    +    // Load imagcover from settings if this is set.
    +    // \Drupal::config('gifplayer.settings')->get('cover_png').
         // @codingStandardsIgnoreStart

    That comment is not necessary, as it describe what is already clear from the code. Instead of being edited, it should be removed.

    +    $cid = ($this->configFactory->get('gifplayer.settings')->get('cover_png') !== NULL)
    +      ? $this->configFactory->get('gifplayer.settings')->get('cover_png')
    +      : 0;

    Code lines are allowed to exceed 80 characters, if they are easier to understand. Writing that code in a single line makes it easier to understand.

  • First commit to issue fork.
  • @bharath-kondeti opened merge request.
  • Status changed to Needs review over 1 year ago
  • 🇮🇳India bharath-kondeti Hyderabad

    Addressed #15 and raised a MR. Please review.

  • Status changed to Needs work over 1 year ago
  • 🇵🇭Philippines roberttabigue

    Hi @bharath-kondeti,

    I applied the latest MR !2 to the Gif Player Field module against 8.x-1.x-dev on Drupal 9.5.10 and I'm still seeing 1 warning from GifPlayerFormatter.php file, please see below:

    I ran this command on the module:
    phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig gifplayer/

    Please see the attached files for reference.

    I'm moving this to Needs work for now.

    Thank you!

  • Assigned to nitin_lama
  • Status changed to Needs review over 1 year ago
  • 🇮🇳India nitin_lama India

    We can ignore that warning. @roberttabigue. Also, I've fixed the class name in the constructor description which was incorrect.

  • Issue was unassigned.
  • Status changed to Needs work over 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
  • Assigned to nitin_lama
  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • Assigned to danrod
  • 🇨🇦Canada danrod Ottawa

    Hi @nitin_lama @apaderno thanks for the work on this, could you provide a patch file instead, if possible? I am having an issue with the merge pipeline.

  • Status changed to RTBC about 1 year ago
  • Assigned to nitin_lama
  • 🇮🇳India nitin_lama India

    Addressing comment #27. Providing updated patch.

  • Issue was unassigned.
  • 🇨🇦Canada danrod Ottawa

    Thanks @nitin_lama , I applied the patch with no issues, committed to the branch 8.x-1.x: https://www.drupal.org/project/gifplayer/releases/8.x-1.x-dev

  • Status changed to Fixed about 1 year ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024