Coding standards [#3339527]

Problem/Motivation

Coder identifies a number of coding standards issues:

FILE: /var/www/html/web/modules/contrib/jsonapi_extras/modules/jsonapi_defaults/tests/src/Functional/JsonApiDefaultsFunctionalTest.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 22 ERRORS AND 1 WARNING AFFECTING 22 LINES
--------------------------------------------------------------------------------------------------------------------------------------
 10 | WARNING | [x] Unused use statement
 31 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 34 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 35 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 36 | ERROR   | [x] Array indentation error, expected 10 spaces but found 12
 37 | ERROR   | [x] Array indentation error, expected 10 spaces but found 12
 40 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 42 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 43 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 45 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 46 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 48 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 49 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 51 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 52 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 53 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 55 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 56 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 58 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 59 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 60 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 8
 62 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 4
 62 | ERROR   | [x] Expected 1 blank line after function; 2 found
--------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 23 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/modules/jsonapi_defaults/src/Controller/EntityResource.php
-----------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------
 174 | ERROR   | [x] Use null coalesce operator instead of ternary operator.
 191 | WARNING | [ ] Redeclaration of function parameter $arr as variable.
-----------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/tests/src/Functional/JsonApiExtrasFunctionalTestBase.php
-------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
-------------------------------------------------------------------------------------------------------------------------------------------
  9 | WARNING | [x] Unused use statement
 66 | ERROR   | [ ] The array declaration extends to column 88 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/tests/src/Functional/JsonApiExtrasFunctionalTest.php
--------------------------------------------------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AND 1 WARNING AFFECTING 6 LINES
--------------------------------------------------------------------------------------------------------------------------------------------
  16 | WARNING | [x] Unused use statement
 128 | ERROR   | [ ] The array declaration extends to column 88 (the limit is 80). The array content should be split up over multiple lines
 143 | ERROR   | [ ] Doc comment is empty
 160 | ERROR   | [x] Missing function doc comment
 190 | ERROR   | [ ] The @see reference should not contain any additional text
 678 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
--------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/tests/src/Kernel/EntityToJsonApiTest.php
-----------------------------------------------------------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 6 LINES
-----------------------------------------------------------------------------------------------------------------------
  55 | ERROR | [ ] Missing short description in doc comment
  60 | ERROR | [ ] Missing short description in doc comment
  65 | ERROR | [ ] Missing short description in doc comment
 105 | ERROR | [ ] Missing short description in doc comment
 246 | ERROR | [x] list(...) is forbidden, use [...] instead.
 254 | ERROR | [ ] Doc comment short description must be on a single line, further text should be a separate paragraph
-----------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/tests/src/Kernel/Controller/EntityResourceTest.php
---------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------
 115 | WARNING | Unused variable $parsed_node_type.
---------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/src/ResourceType/ConfigurableResourceTypeRepository.php
--------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
--------------------------------------------------------------------------------------------------------------
 178 | ERROR | [x] Use null coalesce operator instead of ternary operator.
 199 | ERROR | [x] list(...) is forbidden, use [...] instead.
 250 | ERROR | [x] list(...) is forbidden, use [...] instead.
--------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/src/EventSubscriber/ConfigSubscriber.php
-----------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------
 53 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-----------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/src/EventSubscriber/JsonApiBuildSubscriber.php
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 3 WARNINGS AFFECTING 3 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 19 | ERROR   | Missing short description in doc comment
 43 | WARNING | The deprecation-version 'jsonapi_extras:8.x-3.x' does not match the lower-case machine-name standard: drupal:n.n.n or project:n.x-n.n or project:n.x-n.n-label[n] or
    |         | project:n.n.n or project:n.n.n-label[n]
 43 | WARNING | The removal-version 'jsonapi_extras:8.x-4.x' does not match the lower-case machine-name standard: drupal:n.n.n or project:n.x-n.n or project:n.x-n.n-label[n] or
    |         | project:n.n.n or project:n.n.n-label[n]
 44 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/src/Form/JsonapiResourceConfigForm.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
----------------------------------------------------------------------------------------------------------------------------------------------
 154 | ERROR   | [x] list(...) is forbidden, use [...] instead.
 417 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/src/Normalizer/SchemataSchemaNormalizer.php
----------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
----------------------------------------------------------------------------------------------------------------------------------------
 52 | WARNING | Unused variable $root.
 65 | ERROR   | The array declaration extends to column 116 (the limit is 80). The array content should be split up over multiple lines
----------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/src/JsonapiResourceConfigListBuilder.php
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AND 5 WARNINGS AFFECTING 7 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  12 | WARNING | [x] Unused use statement
  50 | ERROR   | [x] Data types in @param tags need to be fully namespaced
  63 | WARNING | [ ] The deprecation-version 'jsonapi_extras:8.x-3.x' does not match the lower-case machine-name standard: drupal:n.n.n or project:n.x-n.n or
     |         |     project:n.x-n.n-label[n] or project:n.n.n or project:n.n.n-label[n]
  63 | WARNING | [ ] The removal-version 'jsonapi_extras:8.x-4.x' does not match the lower-case machine-name standard: drupal:n.n.n or project:n.x-n.n or project:n.x-n.n-label[n]
     |         |     or project:n.n.n or project:n.n.n-label[n]
 100 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 101 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 188 | ERROR   | [x] Use "elseif" in place of "else if"
 224 | ERROR   | [x] Expected 1 space before ":"; 0 found
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Adding an MR to fix as many as possible.

Comment	File	Size	Author
#15	interfiff-14-15.txt	7.19 KB	bbrala
#15	3339527-15.diff	17.22 KB	bbrala
#14	Fixing_Coding_Standard-3339527-14.patch	10.22 KB	rohit.rawat619

Issue fork jsonapi_extras-3339527

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

coding_standards changes, plain diff MR !26
Check out this branch for the first time

Check out existing branch, if you already have it locally
3339527- changes, plain diff MR !27
Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

6 February 2023 at 09:20

ptmkenny created an issue. See original summary.

Comment #2

6 February 2023 at 09:23

ptmkenny opened merge request !26

Comment #3

ptmkenny commented 6 February 2023 at 09:25

Status:

Active

» Needs review

This MR applies the automated fixes and I also manually fixed the low-hanging fruit (breaking up arrays onto multiple lines, removing unused variables, etc.)

Here is a scan after applying the MR showing the remaining errors:

FILE: /var/www/html/web/modules/contrib/jsonapi_extras/modules/jsonapi_defaults/src/Controller/EntityResource.php
-----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------
 191 | WARNING | Redeclaration of function parameter $arr as variable.
-----------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/tests/src/Functional/JsonApiExtrasFunctionalTest.php
-----------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
-----------------------------------------------------------------------------------------------------------
 145 | ERROR | Doc comment is empty
 162 | ERROR | Doc comment is empty
 195 | ERROR | The @see reference should not contain any additional text
-----------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/tests/src/Kernel/EntityToJsonApiTest.php
-----------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
-----------------------------------------------------------------------------------------------
  55 | ERROR | Missing short description in doc comment
  60 | ERROR | Missing short description in doc comment
  65 | ERROR | Missing short description in doc comment
 105 | ERROR | Missing short description in doc comment
-----------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/src/EventSubscriber/ConfigSubscriber.php
-----------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------
 53 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-----------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/src/EventSubscriber/JsonApiBuildSubscriber.php
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 3 WARNINGS AFFECTING 3 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 19 | ERROR   | Missing short description in doc comment
 43 | WARNING | The deprecation-version 'jsonapi_extras:8.x-3.x' does not match the lower-case machine-name standard: drupal:n.n.n or project:n.x-n.n or project:n.x-n.n-label[n] or
    |         | project:n.n.n or project:n.n.n-label[n]
 43 | WARNING | The removal-version 'jsonapi_extras:8.x-4.x' does not match the lower-case machine-name standard: drupal:n.n.n or project:n.x-n.n or project:n.x-n.n-label[n] or
    |         | project:n.n.n or project:n.n.n-label[n]
 44 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/src/Form/JsonapiResourceConfigForm.php
------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------------
 417 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
------------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/web/modules/contrib/jsonapi_extras/src/JsonapiResourceConfigListBuilder.php
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 3 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  62 | WARNING | The deprecation-version 'jsonapi_extras:8.x-3.x' does not match the lower-case machine-name standard: drupal:n.n.n or project:n.x-n.n or project:n.x-n.n-label[n]
     |         | or project:n.n.n or project:n.n.n-label[n]
  62 | WARNING | The removal-version 'jsonapi_extras:8.x-4.x' does not match the lower-case machine-name standard: drupal:n.n.n or project:n.x-n.n or project:n.x-n.n-label[n] or
     |         | project:n.n.n or project:n.n.n-label[n]
  99 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 100 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Time: 641ms; Memory: 14MB

I do not feel confident fixing the remaining errors so I am submitting the MR in this state.

Comment #4

nayana_mvr commented 9 February 2023 at 07:19

Status:

Needs review

» Needs work

Comment #5

14 February 2023 at 12:08

Rashmisoni made their first commit to this issue’s fork.

Comment #6

14 February 2023 at 12:09

Rashmisoni opened merge request !27

Comment #7

rassoni commented 14 February 2023 at 12:17

Status:

Needs work

» Needs review

Comment #8

indrapatil commented 17 February 2023 at 05:35

Assigned:

Unassigned

» indrapatil

I will review it.

Comment #9

indrapatil commented 17 February 2023 at 09:27

Status:

Needs review

» Needs work

I reviewed the patch, still i can see errors

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml jsonapi_extras-3339527/
PHP Warning: Module "gd" is already loaded in Unknown on line 0

FILE: /var/www/html/drupal8/web/modules/jsonapi_extras-3339527/src/EventSubscriber/JsonApiBuildSubscriber.php
-------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------
46 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------------------------------------

FILE: /var/www/html/drupal8/web/modules/jsonapi_extras-3339527/src/EventSubscriber/ConfigSubscriber.php
-------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------
53 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------------------------------

FILE: /var/www/html/drupal8/web/modules/jsonapi_extras-3339527/src/Form/JsonapiResourceConfigForm.php
------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------------
417 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
------------------------------------------------------------------------------------------------------------------------------------------

FILE: /var/www/html/drupal8/web/modules/jsonapi_extras-3339527/src/JsonapiResourceConfigListBuilder.php
------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------------------------------------------
99 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
100 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
------------------------------------------------------------------------------------------------------------------------------------------

FILE: /var/www/html/drupal8/web/modules/jsonapi_extras-3339527/modules/jsonapi_defaults/src/Controller/EntityResource.php
-------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------
191 | WARNING | Redeclaration of function parameter $arr as variable.
-------------------------------------------------------------------------------------------------------------------------

Time: 1.89 secs; Memory: 18MB

I will work on it.

Comment #10

ptmkenny commented 17 February 2023 at 10:04

@Indra patil The patch does not need to fix all coding standards errors; if the patch fixes errors in a valid way, I think the patch can pass review with a note that "this does not fix all errors."

In particular, changing code to use dependency injection in a coding standards patch increases the chance that something might get broken, so I think it is better to handle DI in a separate patch in a separate issue.

Comment #11

indrapatil commented 17 February 2023 at 10:54

Status:

Needs work

» Needs review

@ptmkenny thank you
I totally agree I fixed some of the issues and still, this below error is pending this Need to create a new ticket.
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml jsonapi_extras-3339527/

Time: 620ms; Memory: 18MB

Comment #12

indrapatil commented 20 February 2023 at 06:14

Assigned:

indrapatil

» Unassigned

Comment #13

rohit.rawat619 commented 27 June 2023 at 07:37

Assigned:	Unassigned	» rohit.rawat619
Status:	Needs review	» Active

Fixing Some Coding Standard

Comment #14

rohit.rawat619 commented 27 June 2023 at 07:39

Assigned:	rohit.rawat619	» Unassigned
Status:	Active	» Needs review

Status	File	Size
new	Fixing_Coding_Standard-3339527-14.patch	10.22 KB

Kindly, Review

Comment #15

bbrala

Netherlands

commented 31 August 2023 at 13:40

Status	File	Size
new	3339527-15.diff	17.22 KB
new	interfiff-14-15.txt	7.19 KB

Thanks! I've updated a few more parts of the patch to make it complete. Lets see if ci doesnt fail.

Comment #16

31 August 2023 at 13:49

bbrala committed e9222988 on 8.x-3.x

Issue #3339527 by Rassoni, bbrala, ptmkenny, Indrapatil, RohitRawat676,...

Comment #17

bbrala

Netherlands

commented 31 August 2023 at 13:49

Status:

Needs review

» Fixed

Thanks! A bit cleaner again :)

Comment #18

14 September 2023 at 13:49

Status:

Fixed

» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Coding standards

Problem/Motivation

Issue fork jsonapi_extras-3339527

Comments

Comment #1

Comment #2

Comment #3

Comment #4

Comment #5

Comment #6

Comment #7

Comment #8

Comment #9

Comment #10

Comment #11

Comment #12

Comment #13

Comment #14

Comment #15

Comment #16

Comment #17

Comment #18

News items

Our community

Documentation

Drupal code base

Governance of community