Fix the issues reported by phpcs [#3349308]

Problem/Motivation

Getting following error/warnings.

FILE: ...ink_formatter/src/Controller/EncryptedFileDownloadController.php
----------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 6 LINES
----------------------------------------------------------------------
26 | ERROR | [ ] Missing short description in doc comment
29 | ERROR | [ ] Class property $link_crypter should use lowerCamel
| | naming without underscores
31 | ERROR | [ ] Parameter $link_crypter is not described in
| | comment
64 | ERROR | [ ] Parameter tags must be defined first in a doc
| | comment
92 | ERROR | [x] list(...) is forbidden, use [...] instead.
111 | ERROR | [ ] The array declaration extends to column 99 (the
| | limit is 80). The array content should be split up
| | over multiple lines
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: ...contrib/encrypted_link_formatter/encrypted_link_formatter.module
----------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
----------------------------------------------------------------------
8 | WARNING | [x] Unused use statement
44 | ERROR | [x] Expected 1 newline at end of file; 2 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

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/printjs/

Proposed resolution

Above errors/warnings need to be fixed.

Comment	File	Size	Author
#10	interdiff-3-8.txt	1.41 KB	sagartiwari
#3	3349308-3.patch	15.33 KB	akshaydalvi212

Issue fork encrypted_link_formatter-3349308

Show commands

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

Add & fetch this issue fork’s repository

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

Add & fetch this issue fork’s repository

3349308-phpcs changes, plain diff MR !2
Check out this branch for the first time

Check out existing branch, if you already have it locally
3349308-fix-the-issues compare
Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

21 March 2023 at 12:11

akshaydalvi212 created an issue. See original summary.

Comment #2

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 21 March 2023 at 12:21

Version:	2.0.0-beta2	» 2.x-dev
Issue tags:		+Coding standards

Comment #3

akshaydalvi212 commented 21 March 2023 at 15:00

Assigned:	akshaydalvi212	» Unassigned
Status:	Active	» Needs review

Status	File	Size
new	3349308-3.patch	15.33 KB

Fixed all the issues reported by PHPCS.
kindly review.

Comment #4

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 21 March 2023 at 16:09

Status:

Needs review

» Needs work

-  public function __construct(StreamWrapperManagerInterface $streamWrapperManager, LinkCrypter $link_crypter) {
+  public function __construct(StreamWrapperManagerInterface $streamWrapperManager, LinkCrypter $linkCrypter) {

It is not necessary to change the parameter name, since $link_crypter is still correct, as local variable name. Drupal coding standards say that class properties should not use snake case names, but local variables are still allowed to use that name style.

+  /**
+   * Constructs for CryptSettingsForm.
+   *
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   *   The config_factory service.
+   * @param \Drupal\Core\File\FileSystemInterface $fileSystem
+   *   The fileSystem service.
+   * @param \Drupal\encrypted_link_formatter\LinkCrypter $linkCrypter
+   *   LinkCrypter service.
+   * @param \Drupal\Core\Cache\CacheTagsInvalidatorInterface $cacheTagsInvalidator
+   *   The cacheTagsInvalidator service.
+   */
+  public function __construct(ConfigFactoryInterface $config_factory, FileSystemInterface $fileSystem, LinkCrypter $linkCrypter, CacheTagsInvalidatorInterface $cacheTagsInvalidator) {

Grammatically, construct needs a direct object; constructs for is wrong.
The class name does not have its namespace.

+  /**
+   * A helping function which generates invalidate tags.
+   */
+  protected function generateIv($type): bool {
+    $private_dir = $this->fileSystem->realpath('private://');
+    if (!$this->linkCrypter->ensureDirExists("$private_dir/iv")) {
       return FALSE;
     }

The short description should start with a verb.

+  /**
+   * A helper function for checking field defination.
+   */
   public static function isApplicable(FieldDefinitionInterface $field_definition) {
     $uri_scheme = $field_definition->getItemDefinition()->getSetting('uri_scheme');
     if ($uri_scheme === 'private') {
       return TRUE;
     }
-
     return FALSE;
   }

defination is a typo.
The short description does not make clear what the method does.
The parameter description is missing, as well as the return value description.

Comment #5

22 March 2023 at 06:12

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

Comment #6

sagartiwari commented 22 March 2023 at 06:34

Assigned:

Unassigned

» sagartiwari

Comment #7

22 March 2023 at 06:56

kkalashnikov opened merge request !2

Comment #8

sagartiwari commented 22 March 2023 at 07:10

Assigned:

sagartiwari

» Unassigned

Comment #9

kunalgautam commented 22 March 2023 at 07:26

Status:

Needs work

» Needs review

Comment #10

sagartiwari commented 22 March 2023 at 09:32

Status	File	Size
new	interdiff-3-8.txt	1.41 KB

Intradiff file for phpcs issues and suggestions on comment #4.

Comment #11

avpaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

commented 22 March 2023 at 16:06

Status:

Needs review

» Needs work

-   *   The stream wrapper manager.
+   *   The streamWrapper manager.
+   * @param \Drupal\encrypted_link_formatter\LinkCrypter $link_crypter
+   *   The linkCrypter service.

Both the descriptions are wrong, since they merge into a word what are two words.

-      $this->moduleHandler()->invokeAll('alter_file_uri', [&$uri, $queryParams, &$headers, $scheme]);
+      $this->moduleHandler()->invokeAll('alter_file_uri', [&$uri, $queryParams,
+        &$headers, $scheme,
+      ]
+        );

The last two lines are wrongly indented.

+  /**
+   * The Crypt settings form constructor.
+   *
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   *   The config_factory service.
+   * @param \Drupal\Core\File\FileSystemInterface $fileSystem
+   *   The fileSystem service.
+   * @param \Drupal\encrypted_link_formatter\LinkCrypter $linkCrypter
+   *   LinkCrypter service.
+   * @param \Drupal\Core\Cache\CacheTagsInvalidatorInterface $cacheTagsInvalidator
+   *   The cacheTagsInvalidator service.

The description for a method starts with a verb.

+  public function __construct(ConfigFactoryInterface $config_factory, FileSystemInterface $fileSystem, LinkCrypter $linkCrypter, CacheTagsInvalidatorInterface $cacheTagsInvalidator) {
     parent::__construct($config_factory);
-    $this->file_system = $file_system;
-    $this->link_crypter = $link_crypter;
-    $this->cache_tags_invalidator = $cache_tags_invalidator;
+    $this->fileSystem = $fileSystem;
+    $this->linkCrypter = $linkCrypter;
+    $this->cacheTagsInvalidator = $cacheTagsInvalidator;

There is no need to change the scheme used for the local variables. Snake case is allowed for them.

+  /**
+   * Helping function which generates invalidate tags.
+   */

It should be helper function, but Helping function which is not necessary. A method description starts with a verb.
The description of parameters and return value is still missing.

   /**
    * Constructs an Example object.
    *
    * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
    *   The config factory.
+   * @param \Drupal\Core\File\FileSystemInterface $fileSystem
+   *   The fileSystem service.
    */

Since that comment is changed, the class name should also include its namespace.

+  /**
+   * A helper function to ensure Directory Exists.
+   */

Directory and Exists are not at the beginning of a sentence. In English, those words are not written in capital case.
The description is still wrong.

+   * @param \Drupal\Core\Routing\RouteMatchInterface $routeMatch
+   *   RouteMatch service.
+   * @param \Drupal\encrypted_link_formatter\LinkCrypter $link_crypter
+   *   LinkCrypter service.
    */

There are missing articles and words merged in a single word.