Fix the issues reported by phpcs

The report shows errors/warnings for 20 files, but the patch changes only four files.

Are you running PHPCS with PHP 8.1?

Thank you for helping out on this module,

@apaderno and @imustakim,
Can you elaborate on your PHP/PHPCS settings and versions used? I am using the PHPCS shipped with "drupal/core-dev": "^10.0"

www-data@bedfa839c0d8:/app$ cd web/modules/contrib/
www-data@bedfa839c0d8:/app/web/modules/contrib$ phpcs --version
PHP_CodeSniffer version 3.7.2 (stable) by Squiz (http://www.squiz.net)
www-data@bedfa839c0d8:/app/web/modules/contrib$ php --version
PHP 8.1.18 (cli) (built: May  3 2023 05:11:38) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.18, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.18, Copyright (c), by Zend Technologies
www-data@bedfa839c0d8:/app/web/modules/contrib$ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,js,info,txt,md,yml,twig graphql_compose/
www-data@bedfa839c0d8:/app/web/modules/contrib$ 

On dev, latest, I no longer see issues, and definitely cant see issues with 20 files.
I'm unsure what further requirements you have noticed

Raveen Thakur → made their first commit to this issue’s fork.

@AlMunnings I am referring to the phpcs report shown in the issue summary. It shows warnings/errors for 10 files, but the patch changes only four files.
Either the patch is not changing all the files that should be fixed, or the report is not for the branch for which the patch has been created.

@apaderno, the report shows issue in 7 files only and not for 10 files, those are:
1. graphql_compose.api.php
2. uuid.admin.js
3. contriEntityTypeWrapper.php
4. GraphQLFieldRow.php
5. CreateComment.php
6. Cursor.php
7. Edge.php

Out of which, patch fixed errors for 4 files and if you see remaining tasks section in issue summary it mentions the issues present in the remaining 3 files.

Hope this helps.

@AlMunnings I just ran phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,js,info,txt,md,yml,twig for the 2.0.x branch. I do not get any warning/error either.

Let's check some of the changes done from the patch.

diff --git a/modules/graphql_compose_comments/src/Plugin/GraphQL/DataProducer/CreateComment.php b/modules/graphql_compose_comments/src/Plugin/GraphQL/DataProducer/CreateComment.php
index 3529526..bf7b4c7 100644
--- a/modules/graphql_compose_comments/src/Plugin/GraphQL/DataProducer/CreateComment.php
+++ b/modules/graphql_compose_comments/src/Plugin/GraphQL/DataProducer/CreateComment.php
@@ -95,7 +95,7 @@ class CreateComment extends DataProducerPluginBase implements ContainerFactoryPl
    * @throws \GraphQL\Error\UserError
    *   First error violation returned from the comment entity validation.
    */
-  public function resolve(array $data, ?EntityInterface $entity, RefinableCacheableDependencyInterface $metadata): ?CommentInterface {
+  public function resolve(array $data, EntityInterface $entity, RefinableCacheableDependencyInterface $metadata): ?CommentInterface {

The src/Plugin/GraphQL/DataProducer/CreateComment.php file does not exist in the 2.0.x branch.

diff --git a/modules/graphql_compose_views/src/Plugin/views/row/GraphQLFieldRow.php b/modules/graphql_compose_views/src/Plugin/views/row/GraphQLFieldRow.php
index 08af8fa..fd8fe9e 100644
--- a/modules/graphql_compose_views/src/Plugin/views/row/GraphQLFieldRow.php
+++ b/modules/graphql_compose_views/src/Plugin/views/row/GraphQLFieldRow.php
@@ -144,7 +144,7 @@ class GraphQLFieldRow extends RowPluginBase {
    * @param \Drupal\Core\Form\FormStateInterface $form_state
    *   The form state.
    */
-  public function validateAliasName($element, FormStateInterface $form_state) {
+  public function validateAliasName(array $element, FormStateInterface $form_state) {
     if (preg_match('@[^A-Za-z0-9]+@', $element['#value'])) {
       $form_state->setError($element, $this->t('The machine-readable name must contain only letters and numbers.'));
     }

The src/Plugin/views/row/GraphQLFieldRow.php does not exist in the 2.0.x branch.

@apaderno, I'm unsure of the reviewing process at your end. In order to assist you, please find the path for the files mentioned below, and I can confirm that both files do indeed exist:
1. "modules/graphql_compose_comments/src/Plugin/GraphQL/DataProducer/CreateComment.php"

2. "modules/graphql_compose_views/src/Plugin/views/row/GraphQLFieldRow.php"

Hope this helps you.

Marking the status to "RTBC", because it seems that there has been a slight oversight in the review process before updating it to "needs work" and now, you are finding a empty report on phpcs because @AlMunnings already committed the changes done by me for those files.

Thanks for wanting to help.

@urvashi_vora Since you provided the patch, you cannot change the status to Reviewed & tested by the community.

I apologize: I was looking for the files in the wrong directory.
Still, replacing ?EntityInterface $entity with EntityInterface $entity is not correct, since a parameter declared as instance of a class implemented by a module or a parameter declared as implementing an interface can be declared as nullable.

Then, I do not get any error/warning when I run phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,js,info,txt,md,yml,twig for the 2.0.x branch.

Notice that AlMunnings has already done a commit, reported in comment #5. If there is anything left, the patch needs to be rerolled.

Thanks for the feedback @apaderno

Yeppers, I think we can mark this one as fixed with no further work required.

AlMunnings → closed merge request !47