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.

CommentFileSizeAuthor
#5 3346860-5.patch16.39 KBimustakim
#2 3346860-2.patch16.51 KBsamitk

Issue fork entity_usage_light-3346860

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

  • 1.0.x Comparecompare
  • dev Comparecompare
About issue forks

Comments

samit.310@gmail.com created an issue. See original summary.

samitk’s picture

samitk commented
Assigned: samitk » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3346860-2.patch16.51 KB

Above error/warnings has been fixed.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Drupal Coding Standards Issues | phpcs » Fix the issues reported by phpcs
Priority: Normal » Minor
Status: Needs review » Needs work
Issue tags: -, - 
+  $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 or usage 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.

imustakim’s picture

imustakim commented
Assigned: Unassigned » imustakim

Working on this.

imustakim’s picture

imustakim commented
Assigned: imustakim » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3346860-5.patch16.39 KB

Patch updated.
Please review.

skt-001’s picture

skt-001 commented
Status: Needs review » Reviewed & tested by the community

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

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Reviewed & tested by the community » Needs work

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.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented 
 /**
  * @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.

mortona2k made their first commit to this issue’s fork.

mortona2k’s picture

mortona2k commented

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.

matthieuscarset’s picture

matthieuscarset commented
Status: Needs work » Fixed

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.

Status: Fixed » Closed (fixed)

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