Fix the PHP_CodeSniffer warnings/errors shown in GitLab CI tests

The issue summary should always describe what the issue is trying to fix and, in the case, of coding standards issues, show which command has been used, which arguments have been used, and which report that command shown.

Providing patch with phpcs issue fixes and also issue summary updated.

The last submitted patch, 3: phpcs-issue-fixes-3357024-3.patch, failed testing. View results →

Providing patch with phpcs issue fixes .This patch fixed all the phpcs issues .Kindly check and review it.

There were still phpcs errors an warning. Providing the updated patch.

Hi @nitin_lama, I have applied your patch and run successfully .

These are the steps I followed:

1. Took clone from git version 3.x in drupal 10.1.x

2. Ran this command:

./vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig modules/contrib/subrequests/

3. Applied patch and again ran phpcs command.

found no errors.

Moving to RTBC.

+ * The BlueprintManager class handles the creation and management of
+ * blueprints used for generating Subrequests within the Subrequests module.

It is for generating sub-requests.

+  /**
+   * Class BlueprintManager.
+   *
+   * @package Drupal\subrequests\Blueprint
+   */
   public function __construct(Serializer $serializer) {

The description just repeats the class name, which is also wrong since that description is for the constructor, not the class. Either the constructor is not documented (as the updated Drupal coding standards say to do), or its documentation comment describes the method purpose and its parameters.
@package Drupal\subrequests\Blueprint is used for classes, not their methods.

  /**
+   * The Subrequests Manager service.

Only the first word must be capitalized.

+   * Constructs a new object with the provided logger.
+   *
+   * @param \Psr\Log\LoggerInterface $logger
+   *   The logger service used for logging.
+   */

Since the documentation comment is provided, it should include the name-spaced class name.
The logger service. is sufficient.

-  public $_resolved;
+  public $resolved;

Since that is a public property, changing its name is not a BC change.

+  /**
+   * Constructs a Subrequest object.
+   *

Since the documentation comment is provided, it must include the class namespace.

+  protected function processBatchesSequence(SubrequestsTree $tree, $_sequence = 0, array $_responses = []) {
+    $batch = $tree[$_sequence];
+    // Perform all the necessary replacements for the elements in the batch.
+    $batch = $this->replacer->replaceBatch($batch, $_responses);
+    $results = array_map(function (Subrequest $subrequest) use ($tree) {
+      $master_request = $tree->getMasterRequest();
+      // Create a Symfony Request object based on the Subrequest.
+      /** @var \Symfony\Component\HttpFoundation\Request $request */
+      $request = $this->serializer->denormalize(
+        $subrequest,
+        Request::class,
+        NULL,
+        ['master_request' => $master_request]
+      );
+      $response = $this->httpKernel
+        ->handle($request, HttpKernelInterface::MASTER_REQUEST);
+      // Set the Content-ID header in the response.
+      $content_id = sprintf('<%s>', $subrequest->requestId);
+      $response->headers->set('Content-ID', $content_id);
+      return $response;
+    }, $batch);
+    // Accumulate the responses for the current batch.
+    $_responses = array_merge($_responses, $results);

Parameter names do not start with underscores. Since its a parameter, its name can be corrected.

+   * The System Under Test (SUT) - BlueprintManager instance.
+   *
    * @var \Drupal\subrequests\Blueprint\BlueprintManager

The System Under Test (SUT) - is not necessary. The property must be described as other properties are.

+   * The JsonBlueprintDenormalizer instance for testing.
+   *
    * @var \Drupal\subrequests\Normalizer\JsonBlueprintDenormalizer

The class name is already given in the @var line; there is no need to repeat it.

Phpcs issues are fixed.

I did not have time to review all the patch, but I would like to point out this wrong change.

+  /**
+   * Constructs an instance of MyClass.
+   *

The documentation comment for constructors is not mandatory anymore, If it is given, the description must be Constructs a new [class name] object. where [class name] includes the class namespace.
The class is BlueprintManager, not MyClass.

Addressed the comment #14. Please review.

+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The HTTP request object associated with the input.

*The active request.* is probably sufficient.

    * @return \Drupal\subrequests\SubrequestsTree
+   *   A subrequest tree prepared for execution based on the provided input.
    */

based on the provided input is not necessary.

   /**
+   * Blueprint Manager instance for handling blueprints.
+   *
+   * This variable holds an instance of the BlueprintManager class,
+   * responsible for handling and managing blueprints within the system.
+   *
    * @var \Drupal\subrequests\Blueprint\BlueprintManager
    */
   protected $blueprintManager;

The Blueprint manager. is sufficient. There is no need to use a long description which essentially repeats what the short description says.

+   * Subrequests Manager instance for handling subrequests.
+   *
+   * This variable holds an instance of the SubrequestsManager class,
+   * responsible for managing and handling subrequests within the system.
+   *
    * @var \Drupal\subrequests\SubrequestsManager
    */

The subrequest manager. is sufficient.

+/**
+ * Handles replacing JSON paths within data structures.
+ */

Replaces JSON paths with data structures. is sufficient.

-   * Searches for JSONPath tokens in the request and replaces it with the values
-   * from previous responses.
+   * Searches for JSONPath tokens in the request and replaces it with values.
+   *
+   * From previous responses.

Splitting a short description in a short description and a long description like that is wrong. If the problem is the length of the short description, the short description must be rewritten. Replaces JSONPath tokens in the request. should be shorter than 80 characters.

   /**
+   * Serializer instance for data serialization and deserialization.
+   *
    * @var \Symfony\Component\Serializer\Serializer

The serializer. is sufficient.

+  /**
+   * Constructor for initializing the object with a logger.
+   *
+   * @param \Psr\Log\LoggerInterface $logger
+   *   The logger instance used for logging purposes.
+   */
   public function __construct(LoggerInterface $logger) {

The documentation comment for constructors is not mandatory anymore, If it is given, the description must be Constructs a new [class name] object. where [class name] includes the class namespace.
The logger. is sufficient to describe $logger.

-   * @return SubrequestsTree
+   * @return \Drupal\subrequests\SubrequestsTree
    *   The sequence of IDs grouped by execution order.

Since the return value is an instance of \Drupal\subrequests\SubrequestsTree, it cannot be a sequence of IDs.

-  public $_resolved;
+  public $resolved;

Since it is changing the name of a public property, that is not a BC change. It should be better to open a new issue for that type of change.

+  /**
+   * Convert the object to its string representation.
+   *
+   * This method is invoked when an object is used in a string context.
+   *
+   * @return string
+   *   The string representation of the object.
+   */
   public function __toString() {

Since that is a magic method used by PHP, the long description is not necessary.

+  /**
+   * Set up the test environment before each test method runs.
+   *
+   * This method is called before each test method to prepare the test
+   * environment or perform any necessary initialization steps.
+   */
   protected function setUp(): void {
     parent::setUp();

Methods inherited from a parent class or defined in an interface have a simpler documentation comment.

Addressed comment #17. Providing updated patch.

Please review. Thanks.

There are merge conflicts that must be manually fixed.

Hi,

I applied patch #27,

Patch not applying properly, getting rejected, Please check

Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules/contrib/subrequests (3.x)
$ curl https://www.drupal.org/files/issues/2024-02-08/subrequests.patch | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 22331  100 22331    0     0  60697      0 --:--:-- --:--:-- --:--:-- 61013
(Stripping trailing CRs from patch; use --binary to disable.)
patching file src/Blueprint/BlueprintManager.php
(Stripping trailing CRs from patch; use --binary to disable.)
patching file src/Controller/FrontController.php
Hunk #2 FAILED at 14.
1 out of 2 hunks FAILED -- saving rejects to file src/Controller/FrontController.php.rej
(Stripping trailing CRs from patch; use --binary to disable.)
patching file src/JsonPathReplacer.php
Hunk #4 FAILED at 259.
1 out of 5 hunks FAILED -- saving rejects to file src/JsonPathReplacer.php.rej
(Stripping trailing CRs from patch; use --binary to disable.)
patching file src/Normalizer/JsonBlueprintDenormalizer.php
Hunk #4 succeeded at 175 (offset 14 lines).
Hunk #5 succeeded at 228 (offset 14 lines).
(Stripping trailing CRs from patch; use --binary to disable.)
patching file src/Normalizer/JsonSubrequestDenormalizer.php
Hunk #2 FAILED at 131.
1 out of 2 hunks FAILED -- saving rejects to file src/Normalizer/JsonSubrequestDenormalizer.php.rej
(Stripping trailing CRs from patch; use --binary to disable.)
patching file src/Subrequest.php
(Stripping trailing CRs from patch; use --binary to disable.)
patching file src/SubrequestsManager.php
(Stripping trailing CRs from patch; use --binary to disable.)
patching file src/SubrequestsTree.php
(Stripping trailing CRs from patch; use --binary to disable.)
patching file tests/src/Unit/Blueprint/BlueprintManagerTest.php
Hunk #1 FAILED at 2.
Hunk #2 FAILED at 10.
Hunk #3 succeeded at 27 with fuzz 1.
2 out of 3 hunks FAILED -- saving rejects to file tests/src/Unit/Blueprint/BlueprintManagerTest.php.rej
(Stripping trailing CRs from patch; use --binary to disable.)
patching file tests/src/Unit/JsonPathReplacerTest.php
(Stripping trailing CRs from patch; use --binary to disable.)
patching file tests/src/Unit/Normalizer/JsonBlueprintDenormalizerTest.php
(Stripping trailing CRs from patch; use --binary to disable.)
patching file tests/src/Unit/Normalizer/JsonSubrequestDenormalizerTest.php
(Stripping trailing CRs from patch; use --binary to disable.)
patching file tests/src/Unit/Normalizer/MultiresponseJsonNormalizerTest.php
Hunk #2 succeeded at 35 with fuzz 1.
(Stripping trailing CRs from patch; use --binary to disable.)
patching file tests/src/Unit/Normalizer/MultiresponseNormalizerTest.php
(Stripping trailing CRs from patch; use --binary to disable.)
patching file tests/src/Unit/SubrequestsManagerTest.php
Hunk #2 succeeded at 9 with fuzz 1.
Hunk #3 succeeded at 29 (offset 1 line).
(Stripping trailing CRs from patch; use --binary to disable.)
patching file tests/src/Unit/SubrequestsTreeTest.php

sakthi_dev → changed the visibility of the branch 3.x to hidden.

Hi,

MR #29 applied successfully.

Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules/subrequests (3.x)
$ curl https://git.drupalcode.org/project/subrequests/-/merge_requests/27.diff | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 34625    0 34625    0     0  77044      0 --:--:-- --:--:-- --:--:-- 77287
patching file SPECIFICATION.md
patching file src/Blueprint/BlueprintManager.php
patching file src/Controller/FrontController.php
patching file src/JsonPathReplacer.php
patching file src/Normalizer/JsonBlueprintDenormalizer.php
patching file src/Normalizer/JsonSubrequestDenormalizer.php
patching file src/Normalizer/MultiresponseJsonNormalizer.php
patching file src/Subrequest.php
patching file src/SubrequestsManager.php
patching file src/SubrequestsTree.php
patching file tests/src/Unit/Blueprint/BlueprintManagerTest.php
patching file tests/src/Unit/JsonPathReplacerTest.php
patching file tests/src/Unit/Normalizer/JsonBlueprintDenormalizerTest.php
patching file tests/src/Unit/Normalizer/JsonSubrequestDenormalizerTest.php
patching file tests/src/Unit/Normalizer/MultiresponseJsonNormalizerTest.php
patching file tests/src/Unit/Normalizer/MultiresponseNormalizerTest.php
patching file tests/src/Unit/SubrequestsManagerTest.php
patching file tests/src/Unit/SubrequestsTreeTest.php

Still some issues remaining

Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules
$ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,js,yml subrequests/

FILE: C:\Users\Admin\Desktop\projects\drupal\web\modules\subrequests\src\Normalizer\JsonBlueprintDenormalizer.php
-----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------
 47 | WARNING | Line exceeds 80 characters; contains 86 characters
-----------------------------------------------------------------------------------------------------------------


FILE: C:\Users\Admin\Desktop\projects\drupal\web\modules\subrequests\src\Subrequest.php
------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------
 43 | WARNING | Property name "$_resolved" should not be prefixed with an underscore to indicate visibility
 43 | ERROR   | Class property $_resolved should use lowerCamel naming without underscores
------------------------------------------------------------------------------------------------------------


FILE: C:\Users\Admin\Desktop\projects\drupal\web\modules\subrequests\tests\modules\subrequests_test\src\TestPolicy.php
----------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------
 24 | ERROR | Missing parameter comment
----------------------------------------------------------------------------------------------------------------------


FILE: C:\Users\Admin\Desktop\projects\drupal\web\modules\subrequests\tests\modules\subrequests_test\subrequests_test.routing.yml
--------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------------------------------------
  6 | WARNING | Open page callback found, please add a comment before the line why there is no access restriction
 13 | WARNING | Open page callback found, please add a comment before the line why there is no access restriction
--------------------------------------------------------------------------------------------------------------------------------


FILE: C:\Users\Admin\Desktop\projects\drupal\web\modules\subrequests\tests\src\Unit\Blueprint\BlueprintManagerTest.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------
 63 | ERROR | The array declaration extends to column 97 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------

Time: 576ms; Memory: 12MB

.

Jasjeet Kaur Brar → made their first commit to this issue’s fork.

Since this module uses GitLab CI, it is better to see what GitLab CI complains about, and fix its report.

ankitv18 → changed the visibility of the branch 3357024-phpcs-ci-job-fix to hidden.

Closing this in favour of https://git.drupalcode.org/issue/subrequests-3434821/-/jobs/1762582