Fix the issues reported by phpcs

Created on 9 March 2023, over 1 year ago
Updated 27 August 2023, about 1 year ago

Problem/Motivation

Getting following error/warnings.

FILE: /var/www/html/modules/contrib/entity_usage_light/entity_usage_light.install
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AND 2 WARNINGS AFFECTING 5 LINES
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
3 | ERROR | [ ] Missing short description in doc comment
10 | WARNING | [ ] Format should be "* Implements hook_foo().", "* Implements hook_foo_BAR_ID_bar() for xyz_bar().",, "* Implements hook_foo_BAR_ID_bar() for
| | xyz-bar.html.twig.", "* Implements hook_foo_BAR_ID_bar() for xyz-bar.tpl.php.", or "* Implements hook_foo_BAR_ID_bar() for block templates."
15 | WARNING | [ ] Unused variable $type.
31 | ERROR | [x] Equals sign not aligned with surrounding assignments; expected 5 spaces but found 1 space
32 | ERROR | [x] Equals sign not aligned with surrounding assignments; expected 4 spaces but found 1 space
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------

FILE: /var/www/html/modules/contrib/entity_usage_light/README.md
----------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
----------------------------------------------------------------------
34 | WARNING | Line exceeds 80 characters; contains 82 characters
35 | WARNING | Line exceeds 80 characters; contains 86 characters
41 | WARNING | Line exceeds 80 characters; contains 82 characters
----------------------------------------------------------------------

FILE: /var/www/html/modules/contrib/entity_usage_light/entity_usage_light.module
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
22 | ERROR | [x] Whitespace found at end of line
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: /var/www/html/modules/contrib/entity_usage_light/src/Form/SettingsForm.php
--------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AND 3 WARNINGS AFFECTING 7 LINES
--------------------------------------------------------------------------------------------------
86 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
87 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
108 | WARNING | [ ] Line exceeds 80 characters; contains 83 characters
109 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
110 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
114 | ERROR | [x] Expected 1 blank line after function; 0 found
115 | ERROR | [x] The closing brace for the class must have an empty line before it
--------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------

FILE: /var/www/html/modules/contrib/entity_usage_light/src/Controller/UsageController.php
--------------------------------------------------------------------------------------------------------------------------------------------
FOUND 14 ERRORS AND 2 WARNINGS AFFECTING 16 LINES
--------------------------------------------------------------------------------------------------------------------------------------------
26 | ERROR | [x] Whitespace found at end of line
34 | ERROR | [ ] Description for the @return value is missing
38 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
41 | ERROR | [x] Expected 1 blank line after function; 2 found
68 | ERROR | [x] The first index in a multi-value array must be on a new line
70 | ERROR | [x] Closing parenthesis of array declaration must be on a new line
92 | WARNING | [x] A comma should follow the last multiline array item. Found: ]
140 | ERROR | [ ] Parameter $entity_type_id is not described in comment
161 | ERROR | [ ] The array declaration extends to column 99 (the limit is 80). The array content should be split up over multiple lines
183 | ERROR | [x] Case breaking statements must be followed by a single blank line
190 | ERROR | [x] Case breaking statements must be followed by a single blank line
204 | ERROR | [ ] The array declaration extends to column 94 (the limit is 80). The array content should be split up over multiple lines
283 | ERROR | [x] Expected newline after closing brace
329 | ERROR | [ ] The array declaration extends to column 83 (the limit is 80). The array content should be split up over multiple lines
335 | ERROR | [x] Expected 1 blank line after function; 0 found
336 | ERROR | [x] The closing brace for the class must have an empty line before it
--------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 10 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------------------------------

FILE: /var/www/html/modules/contrib/entity_usage_light/src/EntityTypeInfo.php
-------------------------------------------------------------------------------------------------------------------------------
FOUND 12 ERRORS AND 2 WARNINGS AFFECTING 13 LINES
-------------------------------------------------------------------------------------------------------------------------------
126 | ERROR | [x] Whitespace found at end of line
129 | ERROR | [x] Whitespace found at end of line
130 | ERROR | [x] Expected "\|null" but found "\..|null" for function return type
142 | ERROR | [x] Whitespace found at end of line
148 | ERROR | [ ] Type hint "array" missing for $form
170 | WARNING | [x] A comma should follow the last multiline array item. Found: $bundle_of
173 | WARNING | [x] A comma should follow the last multiline array item. Found: ]
180 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
193 | ERROR | [x] Comma not allowed after last value in single-line array declaration
193 | ERROR | [x] Expected one space after the comma, 0 found
212 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
245 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
270 | ERROR | [x] Expected 1 blank line after function; 0 found
271 | ERROR | [x] The closing brace for the class must have an empty line before it
-------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 13 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------------

Time: 1.49 secs; Memory: 6MB

Steps to reproduce

Run following command

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml modules/contrib/entity_usage_light/

Proposed resolution

Above error/warnings need to be fixed.

📌 Task
Status

Fixed

Version

1.0

Component

Code

Created by

🇮🇳India samit.310@gmail.com

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 @samit.310@gmail.com
  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • 🇮🇳India samit.310@gmail.com

    Above error/warnings has been fixed.

  • Status changed to Needs work over 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
    +  $module_dir     = \Drupal::service('extension.list.module')->getPathname('entity_usage_light');
    +  $install_dir    = $module_dir . '/' . InstallStorage::CONFIG_INSTALL_DIRECTORY;

    Only a single space is necessary before and after the assignment operator. Since other dependencies have been injected, also that one should be.

    /**
    - * Returns the file url generator.
    + * Constructs an UsageController object.
    *
    - * @return \Drupal\Core\Extension\ModuleHandlerInterface
    + * @param \Drupal\Core\File\FileUrlGeneratorInterface $file_url_generator
    + * The file URL generator.
    */
    - protected function fileUrlGenerator() {
    - if (!$this->fileUrlGenerator) {
    - $this->fileUrlGenerator = \Drupal::service('file_url_generator');
    - }
    - return $this->fileUrlGenerator;
    + public function __construct(FileUrlGeneratorInterface $file_url_generator) {
    + $this->fileUrlGenerator = $file_url_generator;
    }

    The class name needs to be fully namespaced. Also, the article used for words starting with user is a.

    + * @param string $entity_type_id
    + * A entity type id.

    A is used for words that start with a consonant sound. id needs to be all uppercase letters.

    +   * @return \|null
        *   The bundle entity type or nothing.
    

    \ is not a correct type name.

  • Assigned to imustakim
  • 🇮🇳India imustakim Ahmedabad

    Working on this.

  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • 🇮🇳India imustakim Ahmedabad

    Patch updated.
    Please review.

  • Status changed to RTBC over 1 year ago
  • 🇮🇳India saket-001

    Applied the patch #5 its work fine remove all PHPCS error and changes are there as mentioned in #3

  • Status changed to Needs work over 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    The patch could apply, but it is wrong.

    I do not have time to make a full review, but at least the following change is not correct.

       /**
    -   * Returns the file url generator.
    +   * Constructs a UsageController object.
        *
    -   * @return \Drupal\Core\Extension\ModuleHandlerInterface
    +   * @param \Drupal\Core\File\FileUrlGeneratorInterface $file_url_generator
    +   *   The file URL generator.
        */
    -  protected function fileUrlGenerator() {
    -    if (!$this->fileUrlGenerator) {
    -      $this->fileUrlGenerator = \Drupal::service('file_url_generator');
    -    }
    -    return $this->fileUrlGenerator;
    +  public function __construct(FileUrlGeneratorInterface $file_url_generator) {
    +    $this->fileUrlGenerator = $file_url_generator;
       }

    That is not the short description used for class constructors. It is also wrong, since it returns an instance for a different class.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹
     /**
      * @file
    + * Install file for Entity Usage Light.
      */

    The comment for a .install file is usually Install, update, and uninstall hooks for the [module name] module. or Install, update, and uninstall hooks for [module name].

       /**
        * Get media IDs out of a given entity's fields.
        *
    +   * @param string $entity_type_id
    +   *   An entity type ID.
        * @param \Drupal\Core\Entity\EntityInterface $entity
        *   A given entity.
        *

    Since that comment is changed, the verb used in the short description should use the third person singular.
    The articles used in the parameter descriptions are wrong.

       /**
        * Check if the form is a Bundle configuration form.
    -   * 
    +   *
        * @param \Drupal\Core\Form\FormStateInterface $form_state
        *   The form state object.
    -   * 
    -   * @return \..|null
    +   *
    +   * @return null
        *   The bundle entity type or nothing.
        */

    Since that comment is changed, the verb used in the short description should use the third person singular.
    There should be just an empty line between the short description and the parameter descriptions or the return value description.
    The type hint for the return value is wrong, given the description. If the method can returns NULL, the description must be changed too.

    -    // Defaut values
    +    // Defaut values.
         $bundle = $form_state->getFormObject()->getEntity();

    That comment can be removed, since it does not say anything that is already clear reading the code.

           '#description' => '<p>' . $this->t('Select the display in the usage tab per entity type.') . '</p>' .
    -        '<p>' . $this->t('A default table will be used if there is no view available for a given entity type.') . '</p>',
    +      '<p>' . $this->t('A default table will be used if there is no view available for a given entity type.') . '</p>',

    The indentation is already correct.

    -    // Submit
    +    // Submit.
         $form['actions']['submit']['#validate'][] = [$this, 'setThirdPartySettings'];

    That comment can be removed too.

    +  /**
    +   * The plugin Cache Clearer.
    +   *
    +   * @var \Drupal\Core\Plugin\CachedDiscoveryClearer
    +   */
    +  protected $pluginCacheClearer;

    Words that are not at the beginning of a sentence are not written in capital case.

    +  /**
    +   * Private Route builder attribute.
    +   *
    +   * @var \Drupal\Core\Routing\RouteBuilderInterface
    +   */
    +  private $routeBuilder;

    Class properties are not described as attribute.
    Words that are not at the beginning of a sentence are not written in capital case.

    +    // Clear enough caches to enable/disable "Usage" tab for selected
    +    // entity types.

    That comment does not make sense. To be correct, enough should be removed, caches should be written the cache, and two articles should be added.

  • First commit to issue fork.
  • 🇺🇸United States mortona2k Seattle

    I added patch 5 to the issue fork. (The dev branch was an accident)

    I was getting an error on the settings form and had to change CachedDiscoveryClearer to CachedDiscoveryClearerInterface because the service returns Drupal\Core\ProxyClass\Plugin\CachedDiscoveryClearer. I don't know if this is the correct fix.

  • Status changed to Fixed about 1 year ago
  • Thank you very much for reporting and helping on this issue.

    All errors should have been fixed on 1.0.3 now - except 2 warning on line length.

    Marking as fixed.

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024