Fix the issues reported by phpcs

Created on 10 March 2023, almost 2 years ago
Updated 29 May 2023, over 1 year ago

Problem/Motivation

FILE: /Users/projects/drupal-d10/modules/contrib/crazyegg/crazyegg.module
----------------------------------------------------------------------------
FOUND 24 ERRORS AND 10 WARNINGS AFFECTING 23 LINES
----------------------------------------------------------------------------
   1 | ERROR   | [x] Missing file doc comment
  20 | 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."
  21 | ERROR   | [ ] Doc comment short description must end with a full stop
  21 | ERROR   | [ ] Doc comment short description must be on a single line, further text should be a separate paragraph
  26 | ERROR   | [x] Language constructs must be followed by a single space; expected 1 space but found "\n"
  28 | ERROR   | [ ] Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
  36 | WARNING | [x] A comma should follow the last multiline array item. Found:
     |         |     'https://ceblog.s3.amazonaws.com/wp-content/uploads/2015/06/Crazy-Egg-logo-small.png'
  43 | 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."
  44 | WARNING | [ ] Line exceeds 80 characters; contains 90 characters
  44 | ERROR   | [ ] Doc comment short description must be on a single line, further text should be a separate paragraph
  65 | WARNING | [x] A comma should follow the last multiline array item. Found: ]
  74 | 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."
  75 | WARNING | [ ] Line exceeds 80 characters; contains 93 characters
  75 | ERROR   | [ ] Doc comment short description must end with a full stop
  75 | ERROR   | [ ] Doc comment short description must be on a single line, further text should be a separate paragraph
  81 | WARNING | [ ] Line exceeds 80 characters; contains 119 characters
  81 | ERROR   | [x] Inline comments must start with a capital letter
  81 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
  89 | ERROR   | [ ] Doc comment short description must end with a full stop
  89 | ERROR   | [ ] Doc comment short description must be on a single line, further text should be a separate paragraph
  90 | ERROR   | [x] There must be exactly one blank line before the tags in a doc comment
  90 | ERROR   | [ ] Description for the @return value is missing
 105 | WARNING | [ ] Line exceeds 80 characters; contains 120 characters
 106 | ERROR   | [x] There must be exactly one blank line before the tags in a doc comment
 106 | ERROR   | [ ] Description for the @return value is missing
 109 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 110 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 112 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 116 | WARNING | [ ] Line exceeds 80 characters; contains 83 characters
 116 | ERROR   | [x] Doc comment short description must end with a full stop
 117 | ERROR   | [x] There must be exactly one blank line before the tags in a doc comment
 117 | ERROR   | [ ] Description for the @return value is missing
 130 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 131 | ERROR   | [x] Functions must not contain multiple empty lines in a row; found 2 empty lines
----------------------------------------------------------------------------
PHPCBF CAN FIX THE 15 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------


FILE: /Users/projects/drupal-d10/modules/contrib/crazyegg/config/install/crazyegg.settings.yml
----------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------
 5 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------

FILE: /Users/projects/drupal-d10/modules/contrib/crazyegg/README.md
----------------------------------------------------------------------------
FOUND 0 ERRORS AND 57 WARNINGS AFFECTING 57 LINES
----------------------------------------------------------------------------
   9 | WARNING | Line exceeds 80 characters; contains 97 characters
  15 | WARNING | Line exceeds 80 characters; contains 99 characters
  16 | WARNING | Line exceeds 80 characters; contains 92 characters
  18 | WARNING | Line exceeds 80 characters; contains 116 characters
  19 | WARNING | Line exceeds 80 characters; contains 110 characters
  22 | WARNING | Line exceeds 80 characters; contains 107 characters
  28 | WARNING | Line exceeds 80 characters; contains 94 characters
  36 | WARNING | Line exceeds 80 characters; contains 96 characters
  37 | WARNING | Line exceeds 80 characters; contains 135 characters
  43 | WARNING | Line exceeds 80 characters; contains 85 characters
  44 | WARNING | Line exceeds 80 characters; contains 133 characters
  45 | WARNING | Line exceeds 80 characters; contains 93 characters
  47 | WARNING | Line exceeds 80 characters; contains 131 characters
  48 | WARNING | Line exceeds 80 characters; contains 103 characters
  49 | WARNING | Line exceeds 80 characters; contains 107 characters
  50 | WARNING | Line exceeds 80 characters; contains 97 characters
  52 | WARNING | Line exceeds 80 characters; contains 102 characters
  53 | WARNING | Line exceeds 80 characters; contains 129 characters
  54 | WARNING | Line exceeds 80 characters; contains 147 characters
  55 | WARNING | Line exceeds 80 characters; contains 123 characters
  58 | WARNING | Line exceeds 80 characters; contains 127 characters
  59 | WARNING | Line exceeds 80 characters; contains 134 characters
  60 | WARNING | Line exceeds 80 characters; contains 118 characters
  65 | WARNING | Line exceeds 80 characters; contains 119 characters
  71 | WARNING | Line exceeds 80 characters; contains 125 characters
  73 | WARNING | Line exceeds 80 characters; contains 105 characters
  81 | WARNING | Line exceeds 80 characters; contains 93 characters
  85 | WARNING | Line exceeds 80 characters; contains 89 characters
  97 | WARNING | Line exceeds 80 characters; contains 81 characters
 101 | WARNING | Line exceeds 80 characters; contains 129 characters
 103 | WARNING | Line exceeds 80 characters; contains 123 characters
 104 | WARNING | Line exceeds 80 characters; contains 89 characters
 109 | WARNING | Line exceeds 80 characters; contains 114 characters
 112 | WARNING | Line exceeds 80 characters; contains 117 characters
 113 | WARNING | Line exceeds 80 characters; contains 112 characters
 121 | WARNING | Line exceeds 80 characters; contains 112 characters
 123 | WARNING | Line exceeds 80 characters; contains 97 characters
 129 | WARNING | Line exceeds 80 characters; contains 114 characters
 132 | WARNING | Line exceeds 80 characters; contains 127 characters
 138 | WARNING | Line exceeds 80 characters; contains 118 characters
 143 | WARNING | Line exceeds 80 characters; contains 117 characters
 146 | WARNING | Line exceeds 80 characters; contains 127 characters
 154 | WARNING | Line exceeds 80 characters; contains 108 characters
 168 | WARNING | Line exceeds 80 characters; contains 92 characters
 170 | WARNING | Line exceeds 80 characters; contains 148 characters
 172 | WARNING | Line exceeds 80 characters; contains 121 characters
 173 | WARNING | Line exceeds 80 characters; contains 133 characters
 175 | WARNING | Line exceeds 80 characters; contains 89 characters
 180 | WARNING | Line exceeds 80 characters; contains 123 characters
 185 | WARNING | Line exceeds 80 characters; contains 121 characters
 188 | WARNING | Line exceeds 80 characters; contains 88 characters
 189 | WARNING | Line exceeds 80 characters; contains 125 characters
 194 | WARNING | Line exceeds 80 characters; contains 132 characters
 196 | WARNING | Line exceeds 80 characters; contains 125 characters
 197 | WARNING | Line exceeds 80 characters; contains 123 characters
 199 | WARNING | Line exceeds 80 characters; contains 122 characters
 200 | WARNING | Line exceeds 80 characters; contains 108 characters
----------------------------------------------------------------------------

FILE: /Users/projects/drupal-d10/modules/contrib/crazyegg/src/Form/CrazyeggSettingsForm.php
----------------------------------------------------------------------------
FOUND 20 ERRORS AND 9 WARNINGS AFFECTING 29 LINES
----------------------------------------------------------------------------
   3 | ERROR   | [x] Namespaced classes, interfaces and traits should not begin with a file doc comment
  12 | WARNING | [x] Unused use statement
  13 | WARNING | [x] Unused use statement
  32 | ERROR   | [x] Object operator not indented correctly; expected 6 spaces but found 8
  56 | ERROR   | [x] Namespaced classes/interfaces/traits should be referenced with use statements
  59 | ERROR   | [x] Short array syntax must be used to define arrays
  62 | ERROR   | [ ] Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
  63 | ERROR   | [x] Short array syntax must be used to define arrays
  65 | WARNING | [x] A comma should follow the last multiline array item. Found:
     |         |     'https://ceblog.s3.amazonaws.com/wp-content/uploads/2015/06/Crazy-Egg-logo-small.png'
  70 | ERROR   | [x] Short array syntax must be used to define arrays
  73 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
  74 | ERROR   | [x] Short array syntax must be used to define arrays
  80 | ERROR   | [x] Short array syntax must be used to define arrays
  83 | ERROR   | [x] Short array syntax must be used to define arrays
  86 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
  88 | ERROR   | [ ] Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
  92 | ERROR   | [x] Short array syntax must be used to define arrays
  98 | ERROR   | [x] Short array syntax must be used to define arrays
 102 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 103 | ERROR   | [x] Short array syntax must be used to define arrays
 109 | ERROR   | [x] Short array syntax must be used to define arrays
 113 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 115 | ERROR   | [ ] Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
 120 | ERROR   | [x] Short array syntax must be used to define arrays
 123 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 125 | ERROR   | [ ] Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
 135 | ERROR   | [x] Short array syntax must be used to define arrays
 138 | WARNING | [ ] Avoid backslash escaping in translatable strings when possible, use "" quotes instead
 139 | ERROR   | [ ] Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
----------------------------------------------------------------------------
PHPCBF CAN FIX THE 18 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------

Time: 85ms; Memory: 10MB

Steps to reproduce

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

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

📌 Task
Status

Fixed

Version

1.0

Component

Code

Created by

🇮🇳India rassoni 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 @rassoni
  • Assigned to imustakim
  • 🇨🇦Canada imustakim Canada

    Working on this.

  • Issue was unassigned.
  • Status changed to Needs review almost 2 years ago
  • 🇨🇦Canada imustakim Canada

    Added patch for the changes.
    Please review.

  • Status changed to Needs work almost 2 years ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    +/**
    + * @file
    + * The crazyegg module file.
    + */
    +

    That is not the comment used for a module.

    -  // if module is enabled, there is an account ID, user role is allowed and current page matches page targeting setting
    +  /*
    +   * If module is enabled,
    +   * there is an account ID, user role is allowed and
    +   * current page matches page targeting setting.
    +   */
    

    Comments used inside functions/methods use the other delimiter.

     /**
    - * Build account path from account number/id, e.g:
    - *  "1234567" => "0123/4567"
    + * Build account path from account number/id.
    + *
    + * E.g:"1234567" => "0123/4567".
    + *
    

    The existing sentence was already correct. A period is not used before e.g.. It is also better to use for example, instead of e.g..

    + * If actual page URL matches patterns listed in
    + * `crazyegg_paths` variable, If the variable is empty,
    + * return true.
    + *
      * @return bool
    + *   Returns a boolean value.

    The part about the return value should go after @return. Returns a boolean value. is not much helpful, apart non spelling correctly Boolean.

  • 🇨🇦Canada imustakim Canada

    Updated patch and interdiff.

     /**
    - * Build account path from account number/id, e.g:
    - *  "1234567" => "0123/4567"
    + * Build account path from account number/id.
    + *
    + * E.g:"1234567" => "0123/4567".
    + *
    

    In this section first line exceeds the number of character and gives error in phpcs, if we break this into two lines, phpcs does not allow it, and suggests to move the second line as a separate paragraph. Did the changes as suggested.
    Please review.

  • Status changed to Needs review over 1 year ago
  • 🇨🇦Canada imustakim Canada
  • Status changed to Needs work over 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
    +/**
    + * @file
    + * Primary module hooks for Crazy egg integration module.
    + */
    +

    Drupal does not have primary nor secondary hooks. The correct comment is Hook implementations for the Crazy Egg Integration module.

     /**
    - * Implements hook_help().
    - * Shows help info on `/admin/help/crazyegg` page
    + * Implements hook_help() for crazyegg module.
      */

    The existing first line is already correct. It is the second line that should not be there.

    -  // if module is enabled, there is an account ID, user role is allowed and current page matches page targeting setting
    +  // If module is enabled,
    +  // there is an account ID, user role is allowed and
    +  // current page matches page targeting setting.

    Coding standards say to keep lines shorter than 80 characters, not to split a comment at the first comma. // If module is enabled, is a way too short.

     /**
    - * Build account path from account number/id, e.g:
    - *  "1234567" => "0123/4567"
    + * Build account path from account number/id.
    + *
    + * For example, "1234567" => "0123/4567".
    + *
      * @return bool|string
    + *   Returns account path or FALSE.
      */

    It is sufficient to use Build account path from account number/ID. as short description. The example is not necessary.

    + *
      * @return bool
    + *   Returns true if the variable is empty.

    Return value description must not start with Returns.

    + *
      * @return bool
    + *   Returns a boolean value.

    The return type is already reported from the @return line. The description must say when TRUE is returned from the function.

  • Assigned to imustakim
  • 🇨🇦Canada imustakim Canada

    Working on this.

  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • 🇨🇦Canada imustakim Canada

    Patch updated.
    Please review.

  • 🇨🇦Canada imustakim Canada

    Please ignore the last patch.
    Here is the updated patch.

  • Status changed to Needs work over 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
    -  // if module is enabled, there is an account ID, user role is allowed and current page matches page targeting setting
    +  // If module is enabled, there is an account ID,
    +  // user role is allowed and current page
    +  // matches page targeting setting.
    
    That line has been truncated too early. It does not contain 240 characters; then, why was it split in three lines?
    
    <code>  * @return bool|string
    + *   Returns account path or FALSE.

    The description must not start with Returns.
    It also needs to explain when FALSE is returned.
    Also, if TRUE is not returned, the correct return value type is false|string.

    + * Checks URL for matching pattern.
    + *
    + * Checks if actual page URL matches patterns listed in
    + * `crazyegg_paths` variable.

    Both the short description and the long descriptions are missing articles.
    ` characters are not used in PHP comments.

  • Status changed to Fixed over 1 year ago
  • Status changed to Fixed over 1 year ago
Production build 0.71.5 2024