Coding standards

Created on 6 February 2023, over 1 year ago
Updated 31 August 2023, 10 months ago

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.

📌 Task
Status

Fixed

Version

3.0

Component

Code

Created by

🇯🇵Japan ptmkenny

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @ptmkenny
  • @ptmkenny opened merge request.
  • Status changed to Needs review over 1 year ago
  • 🇯🇵Japan ptmkenny

    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.

  • Status changed to Needs work over 1 year ago
  • First commit to issue fork.
  • @rashmisoni opened merge request.
  • Status changed to Needs review over 1 year ago
  • Assigned to Indra patil
  • 🇮🇳India Indra patil Bangalore

    I will work on it.

  • Status changed to Needs work over 1 year ago
  • 🇮🇳India Indra patil Bangalore

    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.

  • 🇯🇵Japan ptmkenny

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

  • Status changed to Needs review over 1 year ago
  • 🇮🇳India Indra patil Bangalore

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

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

    Time: 620ms; Memory: 18MB

  • Issue was unassigned.
  • Assigned to RohitRawat676
  • Status changed to Active about 1 year ago
  • 🇮🇳India RohitRawat676

    Fixing Some Coding Standard

  • Issue was unassigned.
  • Status changed to Needs review about 1 year ago
  • Open on Drupal.org →
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    Waiting for branch to pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & MySQL 5.7
    last update about 1 year ago
    Composer require-dev failure
  • Open in Jenkins → Open on Drupal.org →
    Core: 10.0.7 + Environment: PHP 8.1 & MySQL 5.7
    last update 10 months ago
    17 pass
  • 🇳🇱Netherlands bbrala Netherlands

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

    • bbrala committed e9222988 on 8.x-3.x
      Issue #3339527 by Rassoni, bbrala, ptmkenny, Indrapatil, RohitRawat676,...
  • Status changed to Fixed 10 months ago
  • 🇳🇱Netherlands bbrala Netherlands

    Thanks! A bit cleaner again :)

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

Production build 0.69.0 2024