Drupal Coding Standards Issues | phpcs

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.

I will work on this.

Working on this.

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.

@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).

+  /**
+   * 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.

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

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

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

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

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

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

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.

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

Closing this as outdated. Current 8.x-1.x-dev is phpcs passing now. If further improvement in strings is desired by anyone, please open a new ticket