Fix the issues reported by phpcs

Created on 14 February 2023, over 1 year ago
Updated 25 May 2024, about 1 month ago

Problem/Motivation

FILE: /home/system/Documents/contribution/spambot-3341484/src/Controller/SpambotUserSpamPageController.php
----------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------
 23 | WARNING | Line exceeds 80 characters; contains 85 characters
----------------------------------------------------------------------------------------------------------

FILE: /home/system/Documents/contribution/spambot-3341484/src/Plugin/WebformHandler/SpamBotWebformHandler.php
-----------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AND 3 WARNINGS AFFECTING 6 LINES
-----------------------------------------------------------------------------------------------------------------------------------------
 113 | WARNING | Only string literals should be passed to t() where possible
 118 | WARNING | Only string literals should be passed to t() where possible
 123 | WARNING | Only string literals should be passed to t() where possible
 201 | ERROR   | The array declaration extends to column 132 (the limit is 80). The array content should be split up over multiple lines
 202 | ERROR   | The array declaration extends to column 152 (the limit is 80). The array content should be split up over multiple lines
 230 | ERROR   | The array declaration extends to column 152 (the limit is 80). The array content should be split up over multiple lines
-----------------------------------------------------------------------------------------------------------------------------------------

FILE: /home/system/Documents/contribution/spambot-3341484/spambot.module
------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------
 43 | WARNING | Code after the RETURN statement on line 41 cannot be executed
------------------------------------------------------------------------------

Time: 197ms; Memory: 12MB

FILE: /home/system/Documents/contribution/spambot-3341484/spambot.module
-------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------
 19 | WARNING | Global constants should not be used, move it to a class or interface
-------------------------------------------------------------------------------------

Time: 185ms; Memory: 12MB

Steps to reproduce

Run the following command :
phpcs --standard="Drupal,DrupalPractice" --extensions=php,module,inc,install,test,profile,theme,info,txt,md,yml /modules/contrib/spambot

Proposed resolution

Fix all the php coding standard issues and warnings.

📌 Task
Status

Needs work

Version

1.0

Component

Code

Created by

🇮🇳India Charchil Khandelwal

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

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Issue created by @Charchil Khandelwal
  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • Status changed to Needs work over 1 year ago
  • FILE: C:\xampp\htdocs\drupal\modules\spambot\spambot.module
    ----------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    ----------------------------------------------------------------------
    18 | WARNING | Global constants should not be used, move it to a
    | | class or interface
    ----------------------------------------------------------------------

    FILE: ...modules\spambot\src\Controller\SpambotUserSpamPageController.php
    ----------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    ----------------------------------------------------------------------
    24 | WARNING | \Drupal calls should be avoided in classes, use
    | | dependency injection instead
    ----------------------------------------------------------------------

    FILE: ...docs\drupal\modules\spambot\src\Form\SpambotSettingsForm.php
    ----------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    ----------------------------------------------------------------------
    272 | WARNING | \Drupal calls should be avoided in classes, use
    | | dependency injection instead
    ----------------------------------------------------------------------

    FILE: ...docs\drupal\modules\spambot\src\Form\SpambotUserspamForm.php
    ----------------------------------------------------------------------
    FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
    ----------------------------------------------------------------------
    289 | WARNING | Unused variable $comments_enabled.
    646 | WARNING | \Drupal calls should be avoided in classes, use
    | | dependency injection instead
    ----------------------------------------------------------------------

    Need to fix these errors and warnings.

  • Assigned to Indra patil
  • 🇮🇳India Indra patil Bangalore

    I will work on this.

  • Issue was unassigned.
  • Status changed to Needs review over 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
  • Assigned to imustakim
  • Status changed to Needs work over 1 year ago
  • 🇮🇳India imustakim Ahmedabad

    Working on this.

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

    There are still this 3 error showing up in phpcs, there is an open issue for these error -> https://www.drupal.org/project/coder/issues/3326197 REVERTED: Allow passing constants to t() translate function to prevent 'Only string literals should be passed to t() where possible' CS warning Fixed
    I've updated the changes in the patch. Please review.

  • 🇮🇳India sagarTiwari

    @imustakim Reviewed the patch you gave it works as expected following comment #13 it still has 3 warnings
    (Only string literals should be passed to t() where possible).

  • Status changed to Needs work over 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
    +  /**
    +   * The SpambotUserSpamPageController constructor.
    +   *
    +   * @param \Drupal\Core\Render\RendererInterface $renderer
    +   *   The renderer service.
    +   */
    +  public function __construct(RendererInterface $renderer) {
    +    $this->renderer = $renderer;
    +  }

    Short descriptions for methods and functions start with a verb.
    The class namespace is missing.

  • 🇮🇳India sahil.goyal

    Addressed the comment #15 However missing class namespace is not relevant in this case, as the namespace is already present in the code.

  • Assigned to nitin_lama
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 8
    last update 6 months ago
    4 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 8
    last update 6 months ago
    4 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 8
    last update 6 months ago
    4 pass
  • 🇮🇳India nitin_lama

    Rebased and pushed DI issues fix in the MR. Updating the IS for the remaining issues.

  • Issue was unassigned.
  • 🇮🇳India nitin_lama

    WARNING | Global constants should not be used, move it to a class or interface

    To address this warning, one possible solution is to create a distinct class for the constants and subsequently employ that class within the .module. I refrained from pushing the modifications as there is only a single constant variable. I am uncertain whether it is acceptable to disregard the warning or if it is necessary to establish a separate class to resolve it. please review the remaining issues.

  • Hi,
    Applied #22 patch. patch applied cleanly but still could see some errors.

    FILE: C:\spambot\spambot.module
    ----------------------------------------------------------------------------------------------------------------------------------------------
    FOUND 1 ERROR AND 2 WARNINGS AFFECTING 3 LINES
    ----------------------------------------------------------------------------------------------------------------------------------------------
      19 | WARNING | Global constants should not be used, move it to a class or interface
      43 | WARNING | Code after the RETURN statement on line 41 cannot be executed
     539 | ERROR   | unserialize() is insecure unless allowed classes are limited. Use a safe format like JSON or use the allowed_classes option.
    ----------------------------------------------------------------------------------------------------------------------------------------------
    
    
    FILE: C:\spambot\src\Controller\SpambotUserSpamPageController.php
    ---------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    ---------------------------------------------------------------------------------------------
     24 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    ---------------------------------------------------------------------------------------------
    
    
    FILE: C:\spambot\src\Form\SpambotSettingsForm.php
    ------------------------------------------------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 25 WARNINGS AFFECTING 25 LINES
    ------------------------------------------------------------------------------------------------------------------------------------------
     124 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     125 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     279 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
     362 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     363 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     368 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     369 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     371 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     372 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     373 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     374 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     375 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     376 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     377 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     378 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     384 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     385 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     387 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     388 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     389 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     390 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     391 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     392 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     393 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     394 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
    ------------------------------------------------------------------------------------------------------------------------------------------
    
    
    FILE: C:\spambot\src\Form\SpambotUserspamForm.php
    -------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
    -------------------------------------------------------------------------------------------------
     194 | WARNING | Node::load calls should be avoided in classes, use dependency injection instead
     283 | WARNING | Unused variable $comments_enabled.
     636 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
    -------------------------------------------------------------------------------------------------
    
    
    FILE: C:\spambot\src\Plugin\WebformHandler\SpamBotWebformHandler.php
    ------------------------------------------------------------------------------------------------------------------------------------------
    FOUND 0 ERRORS AND 6 WARNINGS AFFECTING 6 LINES
    ------------------------------------------------------------------------------------------------------------------------------------------
     113 | WARNING | Only string literals should be passed to t() where possible
     114 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     118 | WARNING | Only string literals should be passed to t() where possible
     119 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
     123 | WARNING | Only string literals should be passed to t() where possible
     124 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
    ------------------------------------------------------------------------------------------------------------------------------------------
    
  • First commit to issue fork.
  • 🇮🇳India sakthi_dev

    sakthi_dev changed the visibility of the branch 8.x-1.x-local to hidden.

  • 🇮🇳India sakthi_dev

    sakthi_dev changed the visibility of the branch 3341484-drupal-coding-standards to hidden.

  • Merge request !11Phpcs issue → (Open) created by sakthi_dev
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 4 months ago
    4 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update 4 months ago
    4 pass
  • Status changed to Needs review 4 months ago
  • 🇮🇳India sakthi_dev

    Fixed all the PHPCS issues. Please review.
    As per the new rules constructs description is not mandatory.

  • Status changed to RTBC about 1 month ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update about 1 month ago
    Patch Failed to Apply
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.1 & MySQL 8
    last update about 1 month ago
    4 pass
  • 🇵🇭Philippines cleavinjosh

    Hi @sakthi_dev,

    I applied MR!11, it applied smoothly and fixed the phpcs issues.

    ➜  spambot git:(0a6a6e6) curl https://git.drupalcode.org/project/spambot/-/merge_requests/11.diff | patch -p1
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
    100 28853    0 28853    0     0  61249      0 --:--:-- --:--:-- --:--:-- 61389
    patching file spambot.install
    patching file spambot.module
    patching file src/ConstantInterface.php
    patching file src/Controller/SpambotUserSpamPageController.php
    patching file src/Form/SpambotSettingsForm.php
    patching file src/Form/SpambotUserspamForm.php
    patching file src/Plugin/WebformHandler/SpamBotWebformHandler.php
    patching file tests/src/Kernel/SpambotUserspamTest.php
    ➜  spambot git:(0a6a6e6) ✗ ..
    ➜  contrib git:(main) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml spambot
    
    FILE: /Users/interns/Demo-site/drupal-orgissue/web/modules/contrib/spambot/src/Controller/SpambotUserSpamPageController.php
    ---------------------------------------------------------------------------------------------------------------------------
    FOUND 1 ERROR AFFECTING 1 LINE
    ---------------------------------------------------------------------------------------------------------------------------
     22 | ERROR | Missing short description in doc comment
    ---------------------------------------------------------------------------------------------------------------------------
    
    Time: 823ms; Memory: 14MB
    
    ➜  contrib git:(main) ✗

    I will move the status to Reviewed & tested by the community since it is stated in comment #28 that the it is not mandatory.

    Thank you.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    apaderno changed the visibility of the branch 8.x-1.x to hidden.

  • Status changed to Needs work about 1 month ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
Production build 0.69.0 2024