Fix the issues reported by phpcs

Created on 21 March 2023, over 1 year ago
Updated 21 May 2024, 6 months ago

Problem/Motivation

Getting following error/warnings.

FILE: ...er/src/Plugin/Field/FieldFormatter/EncryptedFileDownloadLink.php
----------------------------------------------------------------------
FOUND 24 ERRORS AND 1 WARNING AFFECTING 20 LINES
----------------------------------------------------------------------
7 | WARNING | [x] Unused use statement
63 | ERROR | [ ] Missing short description in doc comment
66 | ERROR | [ ] Class property $route_match should use
| | lowerCamel naming without underscores
68 | ERROR | [ ] Missing short description in doc comment
69 | ERROR | [x] Data types in @var tags need to be fully
| | namespaced
69 | ERROR | [x] Do not append variable name "$link_crypter" to
| | the type declaration in a member variable
| | comment
71 | ERROR | [ ] Class property $link_crypter should use
| | lowerCamel naming without underscores
73 | ERROR | [ ] Parameter $route_match is not described in
| | comment
73 | ERROR | [ ] Parameter $link_crypter is not described in
| | comment
425 | ERROR | [x] There must be no blank lines after the function
| | comment
426 | ERROR | [x] Scope keyword "protected" must be followed by a
| | single space; found newline
427 | ERROR | [x] Visibility must be declared on method
| | "getExampleToken"
427 | ERROR | [x] Expected 1 blank line before function; 0 found
448 | ERROR | [x] There must be no blank lines after the function
| | comment
449 | ERROR | [x] Scope keyword "protected" must be followed by a
| | single space; found newline
450 | ERROR | [x] Visibility must be declared on method
| | "getTokenWarningMarkup"
450 | ERROR | [x] Expected 1 blank line before function; 0 found
465 | ERROR | [x] There must be no blank lines after the function
| | comment
466 | ERROR | [x] Scope keyword "protected" must be followed by a
| | single space; found newline
467 | ERROR | [x] Visibility must be declared on method
| | "getTokenExampleMarkup"
467 | ERROR | [x] Expected 1 blank line before function; 0 found
471 | ERROR | [x] Object operator not indented correctly; expected
| | 6 spaces but found 8
513 | ERROR | [x] Missing function doc comment
520 | ERROR | [x] Expected 1 blank line after function; 2 found
523 | ERROR | [x] The closing brace for the class must have an
| | empty line before it
----------------------------------------------------------------------
PHPCBF CAN FIX THE 19 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

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/src/Form/CryptSettingsForm.php
----------------------------------------------------------------------
FOUND 19 ERRORS AFFECTING 12 LINES
----------------------------------------------------------------------
18 | ERROR | [x] The open comment tag must be the only content on
| | the line
18 | ERROR | [ ] Missing short description in doc comment
18 | ERROR | [x] Expected "\Drupal\Core\File\FileSystemInterface"
| | but found "\Drupal\Core\File\FileSystemInterface "
| | for @var tag in member variable comment
19 | ERROR | [ ] Class property $file_system should use lowerCamel
| | naming without underscores
21 | ERROR | [x] The open comment tag must be the only content on
| | the line
21 | ERROR | [ ] Missing short description in doc comment
21 | ERROR | [x] Expected
| | "\Drupal\encrypted_link_formatter\LinkCrypter" but
| | found
| | "\Drupal\encrypted_link_formatter\LinkCrypter "
| | for @var tag in member variable comment
22 | ERROR | [ ] Class property $link_crypter should use lowerCamel
| | naming without underscores
24 | ERROR | [x] The open comment tag must be the only content on
| | the line
24 | ERROR | [ ] Missing short description in doc comment
24 | ERROR | [x] Expected
| | "\Drupal\Core\Cache\CacheTagsInvalidatorInterface"
| | but found
| | "\Drupal\Core\Cache\CacheTagsInvalidatorInterface
| | " for @var tag in member variable comment
25 | ERROR | [ ] Class property $cache_tags_invalidator should use
| | lowerCamel naming without underscores
27 | ERROR | [x] Missing function doc comment
34 | ERROR | [x] Missing function doc comment
35 | ERROR | [x] Space before opening parenthesis of function call
| | prohibited
137 | ERROR | [x] Missing function doc comment
137 | ERROR | [ ] Protected method name
| | "CryptSettingsForm::generateIV" is not in
| | lowerCamel format
148 | ERROR | [x] Expected 1 blank line after function; 2 found
151 | ERROR | [x] The closing brace for the class must have an empty
| | line before it
----------------------------------------------------------------------
PHPCBF CAN FIX THE 12 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: ...web/modules/contrib/encrypted_link_formatter/src/LinkCrypter.php
----------------------------------------------------------------------
FOUND 13 ERRORS AND 1 WARNING AFFECTING 11 LINES
----------------------------------------------------------------------
7 | WARNING | [x] Unused use statement
23 | ERROR | [x] The open comment tag must be the only content on
| | the line
23 | ERROR | [ ] Missing short description in doc comment
23 | ERROR | [x] Expected "\Drupal\Core\File\FileSystemInterface"
| | but found "\Drupal\Core\File\FileSystemInterface
| | " for @var tag in member variable comment
24 | ERROR | [ ] Class property $file_system should use lowerCamel
| | naming without underscores
26 | ERROR | [ ] Parameter $file_system is not described in
| | comment
48 | ERROR | [x] Use null coalesce operator instead of ternary
| | operator.
51 | ERROR | [x] Case breaking statements must be followed by a
| | single blank line
59 | ERROR | [x] Missing function doc comment
63 | ERROR | [x] Use null coalesce operator instead of ternary
| | operator.
66 | ERROR | [x] Case breaking statements must be followed by a
| | single blank line
74 | ERROR | [x] Missing function doc comment
94 | ERROR | [x] Missing function doc comment
94 | ERROR | [ ] Protected method name "LinkCrypter::loadIVfile"
| | is not in lowerCamel format
----------------------------------------------------------------------
PHPCBF CAN FIX THE 10 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.

📌 Task
Status

Fixed

Version

2.0

Component

Code

Created by

🇮🇳India akshaydalvi212

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

Merge Requests

Comments & Activities

  • Issue created by @akshaydalvi212
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • 🇮🇳India akshaydalvi212

    Fixed all the issues reported by PHPCS.
    kindly review.

  • Status changed to Needs work over 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
    -  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.

  • First commit to issue fork.
  • Assigned to sagarTiwari
  • Merge request !23349308: more phpcs fixes → (Merged) created by kkalashnikov
  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • 🇮🇳India kkalashnikov Ghaziabad, India
  • 🇮🇳India sagarTiwari

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

  • Status changed to Needs work over 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
    -   *   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.

  • Assigned to dcimorra
  • 🇪🇸Spain dcimorra 🇪🇸 Spain
  • Issue was unassigned.
  • 🇮🇳India zkhan.aamir

    Changing to unassigned since there is no update for a while

  • First commit to issue fork.
  • Assigned to nitin_lama
  • Issue was unassigned.
  • 🇪🇸Spain dcimorra 🇪🇸 Spain
  • Status changed to Fixed 7 months ago
  • 🇪🇸Spain dcimorra 🇪🇸 Spain
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024