Running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig shows the following errors/warnings, which should be fixed.

FILE: /./opquast_checklist/opquast_checklist.module
---------------------------------------------------
FOUND 18 ERRORS AND 6 WARNINGS AFFECTING 20 LINES
---------------------------------------------------
   8 | WARNING | [x] Unused use statement
  18 | ERROR   | [x] Opening brace should be on the same line as the declaration
  24 | ERROR   | [x] Expected newline after closing brace
  46 | ERROR   | [x] Opening brace should be on the same line as the declaration
  61 | WARNING | [ ] Avoid backslash escaping in translatable strings when possible, use "" quotes instead
  71 | ERROR   | [x] Opening brace should be on the same line as the declaration
  76 | ERROR   | [x] TRUE, FALSE and NULL must be uppercase; expected "TRUE" but found "true"
  80 | WARNING | [ ] Only string literals should be passed to t() where possible
  89 | WARNING | [x] A comma should follow the last multiline array item. Found: ]
 103 | WARNING | [x] A comma should follow the last multiline array item. Found: )
 109 | ERROR   | [x] Expected 1 blank line after function; 2 found
 113 | 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."
 114 | ERROR   | [x] There must be exactly one blank line before the tags in a doc comment
 114 | ERROR   | [ ] Missing parameter comment
 114 | ERROR   | [ ] Missing parameter type
 115 | ERROR   | [ ] Missing parameter comment
 115 | ERROR   | [x] Data types in @param tags need to be fully namespaced
 116 | ERROR   | [ ] Missing parameter comment
 116 | ERROR   | [ ] Missing parameter type
 119 | ERROR   | [x] Opening brace should be on the same line as the declaration
 120 | ERROR   | [x] Data types in @var tags need to be fully namespaced
 138 | ERROR   | [x] Array indentation error, expected 6 spaces but found 8
 139 | ERROR   | [x] Array indentation error, expected 6 spaces but found 8
 140 | ERROR   | [x] Array closing indentation error, expected 4 spaces but found 6
---------------------------------------------------
PHPCBF CAN FIX THE 16 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------


FILE: /./opquast_checklist/src/Service/OpquastService.php
---------------------------------------------------------
FOUND 13 ERRORS AND 3 WARNINGS AFFECTING 14 LINES
---------------------------------------------------------
  6 | ERROR   | [x] Doc comment short description must end with a full stop
  7 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name
  9 | ERROR   | [x] Opening brace should be on the same line as the declaration
 12 | WARNING | [ ] Line exceeds 80 characters; contains 106 characters
 14 | ERROR   | [ ] Missing parameter comment
 15 | ERROR   | [x] Separate the @param and @return sections by a blank line.
 15 | ERROR   | [ ] Description for the @return value is missing
 18 | ERROR   | [x] Opening brace should be on the same line as the declaration
 23 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 35 | ERROR   | [ ] Description for the @return value is missing
 38 | ERROR   | [x] Opening brace should be on the same line as the declaration
 40 | ERROR   | [x] Expected 1 blank line after function; 2 found
 45 | ERROR   | [x] There must be exactly one blank line before the tags in a doc comment
 45 | ERROR   | [ ] Description for the @return value is missing
 48 | ERROR   | [x] Opening brace should be on the same line as the declaration
 50 | ERROR   | [x] Expected 1 blank line after function; 0 found
---------------------------------------------------------
PHPCBF CAN FIX THE 9 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------


FILE: /./opquast_checklist/README.md
------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
------------------------------------
 35 | WARNING | Line exceeds 80 characters; contains 117 characters
 41 | WARNING | Line exceeds 80 characters; contains 91 characters
 42 | WARNING | Line exceeds 80 characters; contains 93 characters
------------------------------------

Time: 109ms; Memory: 10MB

Comments

arpitk created an issue. See original summary.

arpitk’s picture

Assigned: arpitk » Unassigned
Status: Active » Needs review
StatusFileSize
new8.31 KB

Hi, Here is the patch. Please review.

Thanks!

Jaspreet-Kaur’s picture

Reviewed! After applying patch #2 all errors seems to be fixed.

raveen_thakur51’s picture

StatusFileSize
new181.7 KB
new204.38 KB
new103.82 KB

Hello arpitk,
I have reviewed your patch & Implemented it on my website having version 9.5.9 And PHP version - 8.1
I am adding some screenshots(before/after applying the patch) for your reference).
Please have a look once at these screenshots. And Thank You.

avpaderno’s picture

Title: Fix issues reported by phpcs. » Fix the issues reported by phpcs
Status: Needs review » Needs work

The report shows errors/warning for three files, but the patch changes four files.

ashutosh ahirwal’s picture

Status: Needs work » Needs review
StatusFileSize
new8.36 KB

Providing patch with fixes.

avpaderno’s picture

Priority: Normal » Minor
Status: Needs review » Needs work
-- Don't understand a rules or don't know how to implement it? No problem click on its link,
- you will be redirected to our where you will find all the information you need to set it up.
+- Don't understand a rules or don't know how to implement it? No problem click
+    on its link, you will be redirected to our where you will find all the
+    information you need to set it up.

The lines after the first one are indented two spaces more than needed.

-    '#help' => t('This checklist is based on the rules of Opquast\'s standards.') . $licence_link->toString(),
+    '#help' => t("This checklist is based on the rules of Opquast\'s standards.") . $licence_link->toString(),

Since the string delimiter has been changed, there is no need to escape the single quotes inside the string.

 /**
- * Class OpquastService
+ * OpquastService class to provide services.
  */
-class OpquastService
-{
+class OpquastService {

A class description must not repeat the class name. to provides services is a too broad description.

+  /**
+   * {@inheritdoc}
+   */
+  public function __construct(LanguageManagerInterface $languageManager) {
+    $this->languageManager = $languageManager;
+  }

The description for a constructor must start with Constructs a new followed by the class name (including its namespace), and end with object.
The documentation comment must also describe the constructor parameters.

+  /**
+   * Function detect current language and return base path of rules.
+   *
+   * If not exist return english language.

A method description must not start with Function or Method.
The used verbs must be declined to the third person singular.
The longer description is not necessary and it is not grammatically correct.

    * @param string $path
+   *   Path variable or url.

url is misspelled, since it is an acronym.
It is not necessary to say variable and Path or URL. is too broad; at least it should say which path is passed to the method.

   * @return string
+   *   Output will be string.

The return value type is already reported in the @return line; the description must describe what is returned.

    * Function return licence path of opquast website.
    *
    * @return string
+   *   Output will be string.
    */
-  public function getLicencePath(): string
-  {
+  public function getLicencePath(): string {
   /**
    * Function return endpoint url of opquast website.
+   *
    * @return string
+   *   Output will be string.
    */
-  public function getApiEndPoint(): string
-  {
+  public function getApiEndPoint(): string {

Since that documentation comment is changed, also the description must be changed.
The return value description is wrong for the same reason given before.

nitin_lama’s picture

Assigned: Unassigned » nitin_lama
nitin_lama’s picture

Status: Needs work » Needs review
StatusFileSize
new9.3 KB

Addressed #7.
Updated patch.

nitin_lama’s picture

Assigned: nitin_lama » Unassigned
nitin_lama’s picture

StatusFileSize
new4.58 KB

Providing interdiff file for #9

thakurnishant_06’s picture

StatusFileSize
new216.18 KB
new44.3 KB

Hello Folks !!
Tested the #2 patch for opquast_checklist 2.0.3 on Drupal 9.5.9 and PHP 8.2, Got some phcs errors .

Later applied the # 9 patch using composer version 2.5. Couldn't apply patch!
I will be adding both screenshot for references.

Thank you.

avpaderno’s picture

The patch must be used with the 2.0.x branch, not a tagged release. With that branch, the patch applies.

avpaderno’s picture

Anyway, running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig I get more errors. It seems the report shown in the issue summary is not complete.

avpaderno’s picture

Issue summary: View changes
avpaderno’s picture

Status: Needs review » Needs work
   /**
-   * Function detect current language and return base path of rules. If not exist return english language.
+   * Constructs a new OpquastService instance.
+   *

The class name is missing its namespace.

+  /**
+   * Detect current language and return base path of rules.
+   *
+   * If not exist return english language.
    *
    * @param string $path
+   *   The opquast checklist path.
+   *
    * @return string
+   *   The base URL of rules.

The verbs used in the description must be inflected in the third person singular.
base path of rules should be rules base path.
If not exist return english language. is not grammatically correct, and it must be changed to If it does not exist, returns the English language. It still not clear to what it does not exist is referring, though.
The return value is not the base URL, but the base path.

-   * Function return endpoint url of opquast website.
+   * Get endpoint url of opquast website.

url is misspelled, since it is an acronym.

nitin_lama’s picture

Assigned: Unassigned » nitin_lama
nitin_lama’s picture

StatusFileSize
new9.33 KB
new1.11 KB

Addressed #16. Updated patch.

nitin_lama’s picture

Assigned: nitin_lama » Unassigned
bindu r’s picture

Status: Needs work » Needs review
StatusFileSize
new6.08 KB

phpcs fixes.

  • Martygraphie committed 1e082eec on 3360936-fix-phpcs authored by bindu r
    Issue #3360936 by nitin_lama, arpitk, Ashutosh Ahirwal, bindu r, Raveen...

martygraphie’s picture

Version: 2.0.3 » 2.0.x-dev
Status: Needs review » Fixed

Hello everyone,
Thanks for the feedback and the associated patch.
I've gone back to patch 20 and fixed the latest PHPCS problems.
@aparderno I've also taken your feedback into account.

I've credited you all and I'm sorry for the lack of feedback over the last few months!
Marc

martygraphie’s picture

Title: Fix the issues reported by phpcs » [2.0.4] Fix the issues reported by phpcs
martygraphie’s picture

martygraphie’s picture

Status: Fixed » Closed (fixed)